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
781 stars 226 forks source link

Fix AD0001: NRE in SE for declaration pattern #8957

Closed pavel-mikula-sonarsource closed 2 months ago

pavel-mikula-sonarsource commented 6 months ago

As reported here and observed inside ascx.cs file:

CSC : error AD0001: Analyzer 'SonarAnalyzer.Rules.CSharp.SymbolicExecutionRunner' threw an exception of type 'SonarAnalyzer.SymbolicExecution.SymbolicExecutionException' with message 'Error processing method: collectiveResultRepeater_ItemDataBound
 Inner exception: System.NullReferenceException: Object reference not set to an instance of an object.
     at SonarAnalyzer.Helpers.TypeHelper.<>c__DisplayClass20_0.<Implements>b__0(INamedTypeSymbol x)
     at System.Linq.ImmutableArrayExtensions.Any[T](ImmutableArray`1 immutableArray, Func`2 predicate)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors.IsPattern.BoolConstraintFromDeclarationPattern(ObjectConstraint valueConstraint, IDeclarationPatternOperationWrapper declaration)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors.IsPattern.BoolConstraintFromPattern(ProgramState state, SymbolicValue value, IPatternOperationWrapper pattern)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors.IsPattern.BoolConstraintFromOperation(ProgramState state, IIsPatternOperationWrapper operation, Boolean isLoopCondition, Int32 visitCount)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors.BranchingProcessor`1.Process(SymbolicContext context, T operation)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors.MultiProcessor`1.Process(SymbolicContext context)
     at SonarAnalyzer.SymbolicExecution.Roslyn.OperationDispatcher.Process(SymbolicContext context)
     at SonarAnalyzer.SymbolicExecution.Roslyn.RoslynSymbolicExecution.<ProcessOperation>d__17.MoveNext()
     at SonarAnalyzer.SymbolicExecution.Roslyn.RoslynSymbolicExecution.Execute()
     at SonarAnalyzer.Rules.SymbolicExecutionRunnerBase.AnalyzeRoslyn(SonarAnalysisContext analysisContext, SonarSyntaxNodeReportingContext nodeContext, ISymbol symbol)'. 

We don't have a reproducer (yet).

zsolt-kolbay-sonarsource commented 6 months ago

The only place where this can happen is this line, if declaration.InputType is null. Then, the null value ends up in the extension methods where an NRE is thrown. I couldn't yet replicate it. I tried with using object, dynamic or generic types in declaration patterns.

Tim-Pohlmann commented 6 months ago

The user shared the code. It looks harmless:

var collectiveEmployee = e.Item.DataItem as CollectiveBaseDto;
...
if (collectiveEmployee is CollectiveEmployeeEpldto CollectiveEPLDTOEmp)

Do you know where the call to .Any comes from in the stack trace?

pavel-mikula-sonarsource commented 6 months ago

In case InputType is null, then InputType.DerivesOrImplements returns false, because it's null-tolerant everywhere.

Tim-Pohlmann commented 6 months ago

I looked again at the stack trace: It is this line:

    private static bool Implements(this ITypeSymbol typeSymbol, ISymbol type) =>
        typeSymbol is { }
        && typeSymbol.AllInterfaces.Any(x => type.IsDefinition ? x.OriginalDefinition.Equals(type) : x.Equals(type));

It can throw an NRE if type, x or x.OriginalDefinition are null.

The first one seems the most likely to me, but I'm not sure.

pavel-mikula-sonarsource commented 6 months ago

This makes sense.

  • type is null if declaration.NarrowedType is null.

AllInterfaces returns ImmutableArray that is a struct and cannot be null OriginalDefinition cannot be null, it defaults to this.

mary-georgiou-sonarsource commented 2 months ago

I tried with VSBuild Tools 2019 and 2022. I could not reproduce it.