.NET 7: suspicious places and errors in the source code
Tags: csharp, dotnet, opensource, programming
Published: 2022-12-14 09:43:35

![1015_NET7_Errors/image1.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image1.png)

\.NET 7 has been released\! It's time for us to dig into its source code and start looking for errors and strange code fragments\. In this article, you'll see comments on our findings from the \.NET developers\. After all, they know the platform code better than anyone else\. Buckle up\! 

I analyzed the release code of \.NET 7\. You can find it on GitHub: [link](https://github.com/dotnet/runtime/tree/v7.0.0)\. 

There were two release candidates \(RC\) prior to the main release, so most of the bugs must have been fixed\. It's more interesting that way — we can investigate whether some of them have gotten into production\. 

I created an issue on GitHub for each suspicious code fragment\. This helped me understand which ones are redundant, which ones are incorrect, and what was fixed by developers\.

**Issue 1**

Can you spot an error here? Let's check\!

```csharp
internal sealed record IncrementalStubGenerationContext(
  StubEnvironment Environment,
  SignatureContext SignatureContext,
  ContainingSyntaxContext ContainingSyntaxContext,
  ContainingSyntax StubMethodSyntaxTemplate,
  MethodSignatureDiagnosticLocations DiagnosticLocation,
  ImmutableArray<AttributeSyntax> ForwardedAttributes,
  LibraryImportData LibraryImportData,
  MarshallingGeneratorFactoryKey<
    (TargetFramework, Version, LibraryImportGeneratorOptions)
  > GeneratorFactoryKey,
  ImmutableArray<Diagnostic> Diagnostics)
{
  public bool Equals(IncrementalStubGenerationContext? other)
  {
    return    other is not null
           && StubEnvironment.AreCompilationSettingsEqual(Environment, 
                                                          other.Environment)
           && SignatureContext.Equals(other.SignatureContext)
           && ContainingSyntaxContext.Equals(other.ContainingSyntaxContext)
           && StubMethodSyntaxTemplate.Equals(other.StubMethodSyntaxTemplate)
           && LibraryImportData.Equals(other.LibraryImportData)
           && DiagnosticLocation.Equals(DiagnosticLocation)
           && ForwardedAttributes.SequenceEqual(other.ForwardedAttributes, 
                (IEqualityComparer<AttributeSyntax>)
                  SyntaxEquivalentComparer.Instance)
          && GeneratorFactoryKey.Equals(other.GeneratorFactoryKey)
          && Diagnostics.SequenceEqual(other.Diagnostics);
    }

    public override int GetHashCode()
    {
      throw new UnreachableException();
    }
}
```

This code fragment checks whether the *this* and *other* objects are equivalent\. However, the developer made a mistake and compared the *DiagnosticLocation* property with itself\.

Incorrect comparison:

```csharp
DiagnosticLocation.Equals(DiagnosticLocation)
```

Correct comparison:

```csharp
DiagnosticLocation.Equals(other.DiagnosticLocation)
```

I found this error in the *LibraryImportGenerator* class \([link to GitHub](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs#L43)\)\. A bit later I found two more fragments — the same error, but in different classes:

* the *JSImportGenerator* class, [link to GitHub](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSImportGenerator.cs#L42);
* the *JSExportGenerator* class, [link to GitHub](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSExportGenerator.cs#L37)\. 

Fun fact: \.NET 7 has a test for this feature\. However, the test is also incorrect, that's why it doesn't detect this error\. 

![1015_NET7_Errors/image2.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image2.png)

In \.NET 8 the code is heavily rewritten\. However, the developers haven't fixed the \.NET 7 code yet — they decided to wait for the feedback\. You can read more about it in the [issue on GitHub](https://github.com/dotnet/runtime/issues/78145)\.  

**Issue 2**

```csharp
internal static void CheckNullable(JSMarshalerType underlyingSig)
{
    MarshalerType underlying = underlyingSig._signatureType.Type;
    if (underlying == MarshalerType.Boolean
        || underlying == MarshalerType.Byte
        || underlying == MarshalerType.Int16
        || underlying == MarshalerType.Int32
        || underlying == MarshalerType.BigInt64
        || underlying == MarshalerType.Int52
        || underlying == MarshalerType.IntPtr
        || underlying == MarshalerType.Double
        || underlying == MarshalerType.Single // <=
        || underlying == MarshalerType.Single // <=
        || underlying == MarshalerType.Char
        || underlying == MarshalerType.DateTime
        || underlying == MarshalerType.DateTimeOffset
        ) return;
    throw new ArgumentException("Bad nullable value type");
}
```

Location: JSMarshalerType\.cs, 387 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSMarshalerType.cs#L387)\)

Here the developer double\-checks if the *underlying* variable equals to *MarshalerType\.Single*\. Sometimes such checks hide errors: for example, the *left* and *right* variables should have been checked, but instead the *left* variable is checked twice\. Here's a [list of similar errors](https://pvs-studio.com/en/blog/examples/v3001/) found in open\-source projects\. 

I created an issue on GitHub: [link](https://github.com/dotnet/runtime/issues/78682)\. Luckily, this code fragment wasn't erroneous — it was just a redundant check\.

**Issue 3**

```csharp
public static bool TryParse(string text, out MetricSpec spec)
{
  int slashIdx = text.IndexOf(MeterInstrumentSeparator);
  if (slashIdx == -1)
  {
    spec = new MetricSpec(text.Trim(), null);
    return true;
  }
  else
  {
    string meterName = text.Substring(0, slashIdx).Trim();
    string? instrumentName = text.Substring(slashIdx + 1).Trim();
    spec = new MetricSpec(meterName, instrumentName);
    return true;
  }
}
```

Location: MetricsEventSource\.cs, 453 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs#L453)\)

The *TryParse* method always returns *true*\. This is weird\. Let's see where this method is used:

```csharp
private void ParseSpecs(string? metricsSpecs)
{
  ....
  string[] specStrings = ....
  foreach (string specString in specStrings)
  {
    if (!MetricSpec.TryParse(specString, out MetricSpec spec))
    {
      Log.Message($"Failed to parse metric spec: {specString}");
    }
    else
    {
      Log.Message($"Parsed metric: {spec}");
      ....
    }
  }
}
```

Location: MetricsEventSource\.cs, 375 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs#L375)\) 

The return value of the *TryParse* method is used as the condition of the *if* statement\. If *specString* cannot be parsed, the original value should be logged\. Otherwise, the received representation \(*spec*\) is logged, and some operations are performed on it\. 

The problem is, *TryParse* always returns *true\.* Thus, the *then* branch of the *if* statement is never executed — the parsing is always successful\.

Issue on GitHub: [link](https://github.com/dotnet/runtime/issues/78625)\.

As a result of the fix, *TryParse* became *Parse*, and the caller method lost the *if* statement\. The developers also changed *Substring* to *AsSpan* in *TryParse*\.  

![1015_NET7_Errors/image3.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image3.png)

By the way, this is the same code fragment that I noted when digging in the \.NET 6 source code\. But back then, the interpolation character was missing in the logging method:

```csharp
if (!MetricSpec.TryParse(specString, out MetricSpec spec))
{
  Log.Message("Failed to parse metric spec: {specString}");
}
else
{
  Log.Message("Parsed metric: {spec}");
  ....
}
```

You can read more about this issue in the [article about \.NET 6 check](https://pvs-studio.com/en/blog/posts/csharp/0903/) \(issue 14\)\. 

**Issue 4**

Since we mentioned methods with strange return values, let's look at another one:

```csharp
public virtual bool TryAdd(XmlDictionaryString value, out int key)
{
  ArgumentNullException.ThrowIfNull(value);

  IntArray? keys;

  if (_maps.TryGetValue(value.Dictionary, out keys))
  {
    key = (keys[value.Key] - 1);

    if (key != -1)
    {
      // If the key is already set, then something is wrong
      throw System.Runtime
                  .Serialization
                  .DiagnosticUtility
                  .ExceptionUtility
                  .ThrowHelperError(
      new InvalidOperationException(SR.XmlKeyAlreadyExists));
     }

     key = Add(value.Value);
     keys[value.Key] = (key + 1);
     return true;               // <=
  }

  key = Add(value.Value);
  keys = AddKeys(value.Dictionary, value.Key + 1);
  keys[value.Key] = (key + 1);
  return true;                  // <=
}
```

Location: XmlBinaryWriterSession\.cs, 28 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Private.DataContractSerialization/src/System/Xml/XmlBinaryWriterSession.cs#L28)\)

The method either returns *true* or throws an exception — it never returns *false*\. This is a public API, so there's more demand for quality\. 

Let's look at the description on [learn\.microsoft\.com](https://learn.microsoft.com/en-us/dotnet/api/system.xml.xmlbinarywritersession.tryadd?view=net-7.0):

![1015_NET7_Errors/image4.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image4.png)

Oopsie\. I created an issue on GitHub for it as well \([link](https://github.com/dotnet/dotnet-api-docs/issues/8656)\), but at the moment of writing this article there was no news on it\.

**Issue 5**

```csharp
public static Attribute? GetCustomAttribute(ParameterInfo element, 
                                            Type attributeType, 
                                            bool inherit)
{
  // ....
  Attribute[] attrib = GetCustomAttributes(element, attributeType, inherit);

  if (attrib == null || attrib.Length == 0)
    return null;

  if (attrib.Length == 0)
    return null;

  if (attrib.Length == 1)
    return attrib[0];

  throw new AmbiguousMatchException(SR.RFLCT_AmbigCust);
}
```

Location: Attribute\.CoreCLR\.cs, 617 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs#L617)\)

In this code fragment, the same expression — *attrib\.Length == 0* — is checked twice: first as a right operand of the '\|\|' operator, then as a condition of the *if* statement\. 

Sometimes this may be an error — developers want to check one thing but instead check another\. We were lucky here: the second check was just redundant and the developers removed it\.

Issue on GitHub: [link](https://github.com/dotnet/runtime/issues/78683)\. 

![1015_NET7_Errors/image5.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image5.png)

**Issue 6**

```csharp
protected virtual XmlSchema? GetSchema()
{
  if (GetType() == typeof(DataTable))
  {
    return null;
  }
  MemoryStream stream = new MemoryStream();

  XmlWriter writer = new XmlTextWriter(stream, null);
  if (writer != null)
  {
    (new XmlTreeGen(SchemaFormat.WebService)).Save(this, writer);
  }
  stream.Position = 0;
  return XmlSchema.Read(new XmlTextReader(stream), null);
}
```

Location: DataTable\.cs, 6678 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Data.Common/src/System/Data/DataTable.cs#L6678)\)

The developer created an instance of the *XmlTextWriter* type\. Then a reference to this instance is assigned to the *writer* variable\. However, in the next line the developer checked *writer* for *null*\. The check always returns *true*, which means the condition is redundant here\. 

It's not horrific, but it's better to remove the check\. The developers did that, actually \([issue on GitHub](https://github.com/dotnet/runtime/issues/78684)\)\.

![1015_NET7_Errors/image6.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image6.png)

**Issue 7**

Redundant code again, but this time it's less obvious:

```csharp
public int ToFourDigitYear(int year, int twoDigitYearMax)
{
  if (year < 0)
  {
    throw new ArgumentOutOfRangeException(nameof(year), 
                                          SR.ArgumentOutOfRange_NeedPosNum);
  }

  if (year < 100)
  {
    int y = year % 100;
    return (twoDigitYearMax / 100 - (y > twoDigitYearMax % 100 ? 1 : 0)) 
             * 100 + y;
  }
  ....
}
```

Location: GregorianCalendarHelper\.cs, 526 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Private.CoreLib/src/System/Globalization/GregorianCalendarHelper.cs#L526)\)

Let's look at how the range of the *year* variable are checked throughout the code execution:

```csharp
ToFourDigitYear(int year, int twoDigitYearMax)
```

*year* is a parameter of the *int* type*\.* Which means its value is within the \[*int\.MinValue*; *int\.MaxValue*\] range\. 

When the code is executed, the *if* statement is met first; in this statement, an exception is thrown:

```csharp
if (year < 0)
{
  throw ....;
}
```

If there's no exception, then the *year* value is within \[0; *int\.MaxValue*\]\. 

Then, another *if* statement:

```csharp
if (year < 100)
{
  int y = year % 100;
  ....
}
```

If the code execution is in the *then* branch of *if*, then the *year* value is within the \[0; 99\] range\. This leads to an interesting result — to the operation of taking the remainder of the division:

```cpp
int y = year % 100;
```

The *year* value is always less than 100 \(i\.e\., the value is between 0\-99\)\. Therefore, the result of the *year % 100* operation is always equal to the left operand — *year*\. Thus, *y* is always equal to *year*\. 

Either the code is redundant or it's an error\. After I opened the [issue on GitHub](https://github.com/dotnet/runtime/issues/78627), the code was fixed and the *y* variable was removed\. 

**Issue 8**

```csharp
internal ConfigurationSection
FindImmediateParentSection(ConfigurationSection section)
{
  ....
  SectionRecord sectionRecord = ....
  if (sectionRecord.HasLocationInputs)
  {
    SectionInput input = sectionRecord.LastLocationInput;
    Debug.Assert(input.HasResult, "input.HasResult");
    result = (ConfigurationSection)input.Result;
  }
  else
  {
    if (sectionRecord.HasIndirectLocationInputs)
    {
      Debug.Assert(IsLocationConfig, 
                   "Indirect location inputs exist 
                    only in location config record");
      SectionInput input = sectionRecord.LastIndirectLocationInput;
      Debug.Assert(input != null);
      Debug.Assert(input.HasResult, "input.HasResult");
      result = (ConfigurationSection)input.Result;
    }
    ....
  ....
}
```

Location: MgmtConfigurationRecord\.cs, 341 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/MgmtConfigurationRecord.cs#L341)\)

We need to dig a bit deeper here\. First, let's look at the second *if* statement:

```csharp
if (sectionRecord.HasIndirectLocationInputs)
{
  Debug.Assert(IsLocationConfig, 
               "Indirect location inputs exist 
                only in location config record");
  SectionInput input = sectionRecord.LastIndirectLocationInput;
  Debug.Assert(input != null);
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}
```

The value of the *LastIndirectLocationInput* property is written to the *input* variable*\.* After that, *input* is checked in two asserts: it's checked for *null* \(*input \!= null*\) and for the presence of result \(*input\.HasResult*\)\. 

Let's look at the *LastIndirectLocationInput* property's body to understand which value can be written to the *input* variable:

```csharp
internal SectionInput LastIndirectLocationInput
  =>   HasIndirectLocationInputs 
     ? IndirectLocationInputs[IndirectLocationInputs.Count - 1] 
     : null;
```

On the one hand, the property may return *null*\. On the other hand, if *HasIndirectLocationInputs* is *true*, then *IndirectLocationInputs\[IndirectLocationInputs\.Count \- 1\]* is returned instead of explicit *null*\. 

The question is, can the value from the *IndirectLocationInputs* collection be *null*? Probably yes, although it's not clear from the code\. By the way, nullable annotations could help here, but they are not enabled in all \.NET projects\. 

Let's go back to *if*:

```csharp
if (sectionRecord.HasIndirectLocationInputs)
{
  Debug.Assert(IsLocationConfig, 
               "Indirect location inputs exist 
                only in location config record");
  SectionInput input = sectionRecord.LastIndirectLocationInput;
  Debug.Assert(input != null);
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}
```

The conditional expression is *sectionRecord\.HasIndirectLocationInputs\. *It's the same property that's checked in *LastIndirectLocationInput*\. Which means *LastIndirectLocationInput* definitely doesn't return explicit *null*\. However, it's unclear which value will be received from *IndirectLocationInputs* and written to *input*\.

The developer first checks that *input \!= null* and only then checks for the presence of the result — *input\.HasResult*\. Looks okay\. 

Now let's go back to the first *if* statement:

```csharp
if (sectionRecord.HasLocationInputs)
{
  SectionInput input = sectionRecord.LastLocationInput;
  Debug.Assert(input.HasResult, "input.HasResult");
  result = (ConfigurationSection)input.Result;
}
```

Let's look at the *LastLocationInput* property:

```csharp
internal SectionInput LastLocationInput 
  =>  HasLocationInputs 
    ? LocationInputs[LocationInputs.Count - 1] 
    : null;
```

It's written the same way as *LastIndirectLocationInput*\. Just like in the previous case, depending on the flag \(*HasLocationInputs*\), either *null* or a value from the *LocationInputs* collection is returned\. 

Now return to the *if* statement\. Its conditional expression is the *HasLocationInputs* property, which is checked within *LastLocationInput*\. If the code is executed in the *then* branch of the *if* statement, this means *LastLocationInput* cannot return explicit *null*\. Can the value from the *LocationInputs* collection be *null*? The question remains unanswered\. If it can, then *null* will be written to *input* too\.  

As in the case of the first inspected *if*, *input\.HasResult* is checked but there's no *input \!= null* this time\. 

Once again\. The first inspected code fragment: 

```csharp
SectionInput input = sectionRecord.LastIndirectLocationInput;
Debug.Assert(input != null);
Debug.Assert(input.HasResult, "input.HasResult");
result = (ConfigurationSection)input.Result;
```

The second one:

```csharp
SectionInput input = sectionRecord.LastLocationInput;
Debug.Assert(input.HasResult, "input.HasResult");
result = (ConfigurationSection)input.Result;
```

Looks like the *Debug\.Assert\(input \!= null\)* expression is missing\.

I opened an [issue on GitHub](https://github.com/dotnet/runtime/issues/78634) where I described this and other suspicious places related to *null* checks \(you'll see them below\)\. 

The developers decided not to fix this fragment and left it as is:

![1015_NET7_Errors/image7.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image7.png)

**Issues with null checks**

I came across several places in code where a reference is dereferenced and only then it's checked for *null\.* I created [one issue](https://github.com/dotnet/runtime/issues/78634) for all similar code fragments on GitHub\.

Let's inspect\. 

**Issue 9**

```csharp
private static RuntimeBinderException BadOperatorTypesError(Expr pOperand1, 
                                                            Expr pOperand2)
{
  // ....
  string strOp = pOperand1.ErrorString;

  Debug.Assert(pOperand1 != null);
  Debug.Assert(pOperand1.Type != null);

  if (pOperand2 != null)
  {
    Debug.Assert(pOperand2.Type != null);
    return ErrorHandling.Error(ErrorCode.ERR_BadBinaryOps,
                               strOp, 
                               pOperand1.Type, 
                               pOperand2.Type);
  }

  return ErrorHandling.Error(ErrorCode.ERR_BadUnaryOp, strOp, pOperand1.Type);
}
```

Location: ExpressionBinder\.cs, 798 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/ExpressionBinder.cs#L798)\)

First, *pOperand1* is dereferenced \(*pOperand1\.ErrorString*\) and is checked for *null* in *Debug\.Assert *in the next code line\. If *pOperand1* is *null*, then the assert is not triggered, but an exception of the *NullReferenceException* type is thrown instead\. 

The code was fixed — *pOperand1* is checked before use\. 

Before:

```csharp
string strOp = pOperand1.ErrorString;

Debug.Assert(pOperand1 != null);
Debug.Assert(pOperand1.Type != null);
```

After:

```csharp
Debug.Assert(pOperand1 != null);
Debug.Assert(pOperand1.Type != null);

string strOp = pOperand1.ErrorString;
```

**Issue 10**

```csharp
public void Execute()
{
  var count = _callbacks.Count;
  if (count == 0)
  {
    return;
  }

  List<Exception>? exceptions = null;

  if (_callbacks != null)
  {
    for (int i = 0; i < count; i++)
    {
      var callback = _callbacks[i];
      Execute(callback, ref exceptions);
    }
  }

  if (exceptions != null)
  {
    throw new AggregateException(exceptions);
  }
}
```

Location: PipeCompletionCallbacks\.cs, 20 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletionCallbacks.cs#L20)\)

The *\_callbacks* variable is used first and only then it's checked for *null*:

```csharp
public void Execute()
{
  var count = _callbacks.Count;
  ....
  if (_callbacks != null)
  ....
}
```

At the time of writing this article, the developers removed checking *\_callbacks* for *null*\. 

By the way, *\_callbacks* is a *readonly* field that's initialized in a constructor:

```csharp
internal sealed class PipeCompletionCallbacks
{
  private readonly List<PipeCompletionCallback> _callbacks;
  private readonly Exception? _exception;
  public PipeCompletionCallbacks(List<PipeCompletionCallback> callbacks, 
                                 ExceptionDispatchInfo? edi)
  {
    _callbacks = callbacks;
    _exception = edi?.SourceException;
  }
  ....
}
```

In the thread with the fix, the developers discussed whether it was worth adding *Debug\.Assert* and checking *\_callbacks* for *null* into a constructor\. In the end, they decided it wasn't\.

![1015_NET7_Errors/image8.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image8.png)

**Issue 11**

```csharp
private void ValidateAttributes(XmlElement elementNode)
{
  ....
  XmlSchemaAttribute schemaAttribute 
    = (_defaultAttributes[i] as XmlSchemaAttribute)!;
  attrQName = schemaAttribute.QualifiedName;
  Debug.Assert(schemaAttribute != null);
  ....
}
```

Location: DocumentSchemaValidator\.cs, 421 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Private.Xml/src/System/Xml/Dom/DocumentSchemaValidator.cs#L421)\)

The controversial code:

1. The result of the *as* operator is written to *schemaAttribute*\. If *\_defaultAttributes\[i\]* – *null* or the cast failed, the result will be *null*\. 
1. The null\-forgiving operator \('\!'\) implies that the result of casting cannot be *null*\. Therefore, *schemaAttribute* cannot be *null*\. 
1. In the next code line, *schemaAttribute* is dereferenced\. Then in a line below, the reference is checked for *null*\.

Here's the question\. Can *schemaAttribute* be *null*? It's not very clear from the code\. 

The code was fixed like that:

```csharp
....
XmlSchemaAttribute schemaAttribute 
  = (XmlSchemaAttribute)_defaultAttributes[i]!;
attrQName = schemaAttribute.QualifiedName;
....
```

During the discussion of the fix, the developer proposed moving the *Debug\.Assert* call in the line above instead of removing it\. The code would look like that:

```csharp
....
XmlSchemaAttribute schemaAttribute = (XmlSchemaAttribute)_defaultAttributes[i]!;
Debug.Assert(schemaAttribute != null);
attrQName = schemaAttribute.QualifiedName;
....
```

In the end, they decided not to return *Assert*\.

![1015_NET7_Errors/image9.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image9.png)

**Issue 12**

Let's look at the constructor of the* XmlConfigurationElementTextContent* type:

```csharp
public XmlConfigurationElementTextContent(string textContent, 
                                          int? linePosition, 
                                          int? lineNumber)
{ .... }
```

Location: XmlConfigurationElementTextContent\.cs, 10 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.Extensions.Configuration.Xml/src/XmlConfigurationElementTextContent.cs#L10)\)

Now let's see where it's used:

```csharp
public static IDictionary<string, string?> Read(....)
{
  ....
  case XmlNodeType.EndElement:
    ....
    var lineInfo = reader as IXmlLineInfo;
    var lineNumber = lineInfo?.LineNumber;
    var linePosition = lineInfo?.LinePosition;
    parent.TextContent = new XmlConfigurationElementTextContent(string.Empty, 
                                                                lineNumber,
                                                                linePosition);
    ....
    break;
  ....
  case XmlNodeType.Text:
    ....
    var lineInfo = reader as IXmlLineInfo;
    var lineNumber = lineInfo?.LineNumber;
    var linePosition = lineInfo?.LinePosition;

    XmlConfigurationElement parent = currentPath.Peek();

    parent.TextContent = new XmlConfigurationElementTextContent(reader.Value,
                                                                lineNumber, 
                                                                linePosition);
    ....
    break;
  ....
}
```

Locations:

* XmlStreamConfigurationProvider\.cs, 133 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.Extensions.Configuration.Xml/src/XmlStreamConfigurationProvider.cs#L133)\)
* XmlStreamConfigurationProvider\.cs, 148 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.Extensions.Configuration.Xml/src/XmlStreamConfigurationProvider.cs#L148)\) 

Have you noticed anything strange in code?

Pay attention to the order of arguments and parameters:

* arguments: \.\.\., *lineNumber*, *linePosition*; 
* parameters: \.\.\., *linePosition*, *lineNumber*\.

I created an issue on GitHub \([link](https://github.com/dotnet/runtime/issues/78212)\), the code was fixed: the developers put arguments in the correct order and added a test\. 

**Issue 13**

Another suspicious case: 

```csharp
public virtual bool Nested
{
  get {....}
  set 
  {
    ....
    ForeignKeyConstraint? constraint 
      = ChildTable.Constraints
                  .FindForeignKeyConstraint(ChildKey.ColumnsReference, 
                                            ParentKey.ColumnsReference); 
    ....
  }
}
```

Location: DataRelation\.cs, 486 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Data.Common/src/System/Data/DataRelation.cs#L486)\)

Look at the* FindForeignKeyConstraint* method:

```csharp
internal ForeignKeyConstraint? 
FindForeignKeyConstraint(DataColumn[] parentColumns, 
                         DataColumn[] childColumns)
{ .... }
```

Location: ConstraintCollection\.cs, 548 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/System.Data.Common/src/System/Data/ConstraintCollection.cs#L548)\)

Seems like the argument order is mixed up again:

* parameters: *parent*\.\.\., *child*\.\.\.
* arguments: *ChildKey*\.\.\., *ParentKey*\.\.\.

There's another method call: the argument order is correct there\. 

```csharp
ForeignKeyConstraint? foreignKey
  = relation.ChildTable
            .Constraints
            .FindForeignKeyConstraint(relation.ParentColumnsReference,
                                      relation.ChildColumnsReference);
```

I created an issue on GitHub: [link](https://github.com/dotnet/runtime/issues/78628)\. Unfortunately, I haven't received any comments on it at the moment of writing the article\. 

**Issue 14**

These are not all places where the argument order is mixed up — I found another one:

```csharp
void RecurseChildren(....)
{
  ....
  string? value 
    =  processValue != null
      ? processValue(new ConfigurationDebugViewContext(
                           child.Key, 
                           child.Path, 
                           valueAndProvider.Value, 
                           valueAndProvider.Provider))
      : valueAndProvider.Value;

  ....
}
```

Location: ConfigurationRootExtensions\.cs, 50 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/ConfigurationRootExtensions.cs#L50)\)

Look at the* ConfigurationDebugViewContext* constructor:

```csharp
public ConfigurationDebugViewContext(
  string path, 
  string key, 
  string? value, 
  IConfigurationProvider configurationProvider) 
{ .... }
```

Location: ConfigurationDebugViewContext\.cs, 11 \([link](https://github.com/dotnet/runtime/blob/d099f075e45d2aa6007a22b71b45a08758559f80/src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/ConfigurationDebugViewContext.cs#L11)\)

The order:

* parameters: *path*, *key*, \.\.\.
* arguments: *child\.Key*, *child\.Path*, \.\.\.

I created an issue on GitHub: [link](https://github.com/dotnet/runtime/issues/78306)\. According to the developers, this case doesn't have any issues despite the mistake\. 

![1015_NET7_Errors/image10.png](https://import.viva64.com/docx/blog/1015_NET7_Errors/image10.png)

However, they still fixed the order of arguments\.

**Conclusion**

The \.NET code is of high quality\. I believe this is achieved by an established development process — the developers know the exact release date\. Besides, release candidates help find the most serious errors and prepare the project for the release\.

Nevertheless, I still manage to find something intriguing in the code\. This time my favorites are arguments that were mixed up during method calls\. 

All code fragments described in this article were found by the PVS\-Studio analyzer\. Yes, now it can check projects on \.NET 7\. 

If you want to check your projects \(personal or commercial\), download the analyzer [here](https://pvs-studio.com/en/pvs-studio/try-free/?utm_source=website&utm_medium=our_site&utm_campaign=article&utm_content=1015&utm_term=link_try-free)\. There's also a link to the documentation on this page: we described how to enter the license and run the analysis\. If you have any questions or issues — [let us know](https://pvs-studio.com/en/about-feedback/) and we'll help\.