# \.NET 7: suspicious places and errors in the source code

\.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\! 

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

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\!

```cpp
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();
    }
}
```

<details>
   <summary>Answer</summary>

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:

```cpp
DiagnosticLocation.Equals(DiagnosticLocation)
```

Correct comparison:

```cpp
DiagnosticLocation.Equals(other.DiagnosticLocation)
```


</details>


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**

```cpp
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**

```cpp
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:

```cpp
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:

```cpp
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:

```cpp
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**

```cpp
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**

```cpp
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:

```cpp
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:

```cpp
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:

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

If there's no exception, then the _year_ value is within \[0; _int\.MaxValue_\]\. 

Then, another _if_ statement:

```cpp
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**

```cpp
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:

```cpp
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:

```cpp
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_:

```cpp
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:

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

Let's look at the _LastLocationInput_ property:

```cpp
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: 

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

The second one:

```cpp
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**

```cpp
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:

```cpp
string strOp = pOperand1.ErrorString;

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

After:

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

string strOp = pOperand1.ErrorString;
```

**Issue 10**

```cpp
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_:

```cpp
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:

```cpp
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**

```cpp
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:

```cpp
....
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:

```cpp
....
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:

```cpp
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:

```cpp
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: 

```cpp
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:

```cpp
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\. 

```cpp
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:

```cpp
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:

```cpp
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/)\. 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\.