sonar-dotnet
sonar-dotnet copied to clipboard
Investigate why S2259 does not always trigger on C# 8 code
Description
C#8 introduces the concept of non-nullable reference types. By default, the reference types are considered non-nullable. See details in Nullable reference types.
I analyzed andrei-epure-sonarsource/CSharp8Demo on SonarCloud. Note that the csproj has the NullableContextOptions
enabled.
Rule S2259 doesn't behave normally with easy-to-find cases.
Repro steps
Look at this line on sonarcloud. It is clearly a NRE (and it does thrown at runtime).
Expected behavior
The analyzer should raise S2259 at line 17, 20, 25 and 42.
Actual behavior
Only line 42 has a raised issue.
Known workarounds
None.
Related information
- SonarC# Version 7.16
While digging into the reasons the issues are not raised for C#8, I found 2 causes:
-
CFG generation fails when it encounters a null-forgiving operator (ex:
str!.Length
), as code will go insideCSharpControlFlowGraphBuilder#BuildExpression(ExpressionSyntax expression, Block currentBlock)
, and the expression has typeSuppressNullableWarningExpression
, which is not known => method will throw aNotSupportedException
. To fix this, we should use an updated Shim layer that defines correctly theSuppressNullableWarningExpression
kind inside theSyntaxKindEx
class, as well as update the logic insideBuildExpression
to handle this type of syntax node correctly. -
Unrelated to C#8, but SE engine exploration stops on the first null dereference found (due to this line here). So for the analyzer to raise an issue on each of the dereference found in the example code referenced above, we should either put them in separate methods, or continue the SE after the first issue found.
We need to add test cases and then we'll see what the behavior of the new engine is
Our team is seeing this issue. I am unable to view the sources referenced in this thread.
This hides many potential issues in our codebase.
Any update on this?
See the following example.
public static class Example
{
public class Test
{
public string Uri { get; set; }
}
public static void Method()
{
// change just one of these member accesses on the lines commented w/ S2259
// to use null conditional operator and it suppresses all other occurrences of S2259
// warning in the method
int[] ia = null;
Console.WriteLine(ia.Length); // S2259
Test[] ta = Array.Empty<Test>();
var test1 = ta.FirstOrDefault();
Console.WriteLine(test1.Uri); // S2259
Test test2 = null;
Console.WriteLine(test2.Uri); // S2259
// A single null coalescing will suppress all S2259 errors in the method
// Uncomment the following
// var test3 = test2 ?? new ();
// Console.WriteLine(test3.Uri);
}
}
A single null coalescing will suppress all S2259 warnings in a method too.
We are dotnet 6.0 / C# 10.
We're in process of rewriting our Symbolic Execution engine. We'll revisit this once the effort is done. That should happen later this year.
Related issue #4250 for the new engine.