SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
791 stars 228 forks source link

Fix AD0001: NullReferenceException is thrown in SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation #9637

Closed hugoqribeiro closed 2 months ago

hugoqribeiro commented 3 months ago

Description

Compiling a solution raises the following warning:

17>    CSC : warning AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
17>    Exception occurred with following context:
17>    Compilation: Cegid.ID.Server.Host
17>    SyntaxTree: C:\Git\lithium-product-core.ids\src\server\Host\WebSite\ViewModels\VerificationCodeViewModelBase.cs
17>    SyntaxNode: if (!int.TryParse(this.VerificationCode ... [IfStatementSyntax]@[1988..2382) (54,17)-(60,13)
17>    System.NullReferenceException: Object reference not set to an instance of an object.
17>    at SonarAnalyzer.Extensions.InvocationExpressionSyntaxExtensions.IsEqualTo(InvocationExpressionSyntax first, InvocationExpressionSyntax second, SemanticModel model)
17>    at SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation.HaveTheSameInvocations(InvocationExpressionSyntax[] referenceInvocations, InvocationExpressionSyntax[] candidateInvocations, SemanticModel model)
17>    at SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation.HaveTheSameInvocations(SyntaxNode first, SyntaxNode second, SemanticModel model)
17>    at SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation.AreEquivalentStatements(SyntaxNode node, SyntaxNode otherNode, SemanticModel model)
17>    at System.Linq.Enumerable.All[TSource](IEnumerable`1 source, Func`2 predicate)
17>    at SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation.CheckStatement(SonarSyntaxNodeReportingContext context, SyntaxNode node, IReadOnlyList`1 precedingBranches, SemanticModel model, Boolean hasElse, String discriminator)
17>    at SonarAnalyzer.Rules.CSharp.ConditionalStructureSameImplementation.<>c.<Initialize>b__5_0(SonarSyntaxNodeReportingContext c)
17>    at SonarAnalyzer.AnalysisContext.SonarCompilationStartAnalysisContext.<>c__DisplayClass11_0`1.<RegisterNodeAction>b__0(SyntaxNodeAnalysisContext x)
17>    at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__52`1.<ExecuteSyntaxNodeAction>b__52_0(ValueTuple`2 data)
17>    at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken)

Repro steps

I think the method being analyzed is this one:

    public IEnumerable<System.ComponentModel.DataAnnotations.ValidationResult> Validate(System.ComponentModel.DataAnnotations.ValidationContext validationContext)
    {
        if (!string.IsNullOrWhiteSpace(this.VerificationCode))
        {
            if (this.VerificationCode.Length != Constants.Codes.VerificationCodeLength)
            {
                yield return new System.ComponentModel.DataAnnotations.ValidationResult(
                    ValidationResources2.RES_Error_VerificationCodeChars_Invalid
                        .Format(Constants.Codes.VerificationCodeLength),
                    new string[] { nameof(this.VerificationCode) });
            }
            else if (!int.TryParse(this.VerificationCode, out _))
            {
                yield return new System.ComponentModel.DataAnnotations.ValidationResult(
                    ValidationResources2.RES_Error_VerificationCodeChars_Invalid
                        .Format(Constants.Codes.VerificationCodeLength),
                    new string[] { nameof(this.VerificationCode) });
            }
        }
    }

Known workarounds

N/A

Related information

Visual Studio 17.10.5 SonarAnalyzer.CSharp 9.30.0.95878

zsolt-kolbay-sonarsource commented 2 months ago

Thank you for reporting this. Confirmed as a bug. The analyzer fails when it reaches the nameof(this.VerificationCode) and tries to resolve it as a symbol. But Roslyn treats the nameof expression as a compile-time constant, and it returns null for a symbol, which isn't handled correctly by the analyzer. A shorter reproducer looks like this:

public string NameOfWithSameVariable(string str)
{
    if (str.Length == 42)
    {
        var ret = nameof(str);  // <--- AD0001
        return ret;
    }
    else if (str.Length == 43)
    {
        var ret = nameof(str);
        return ret;
    }
    return "";
}