dotnet / roslyn-analyzers

MIT License
1.59k stars 466 forks source link

NullReferenceException from CSharpPreferGenericOverloadsAnalyzer in .NET 9 RC1 #7421

Open bkoelman opened 1 month ago

bkoelman commented 1 month ago

Analyzer

Diagnostic ID: CA2263: Prefer generic overload when type is known

Describe the bug

After updating our codebase to .NET 9 RC1, the following warning is logged during build. This doesn't show when building against .NET 8.

CSC : warning AD0001: Analyzer 'Microsoft.NetCore.CSharp.Analyzers.Usage.CSharpPreferGenericOverloadsAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.

I'm not aware of an easy way to determine which line of code this originates from. This can be reproduced by cloning the JsonApiDotNetCore repository and building the dotnet9-preview branch, after commenting out the following in .editorconfig:

# Turn off crashing analyzer, https://github.com/dotnet/roslyn-analyzers/issues/7421
dotnet_diagnostic.CA2263.severity = none

I've tried installing the latest prerelease version, which doesn't help:

<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="9.0.0-preview.24454.1" PrivateAssets="All" />
mpidash commented 1 month ago

Thanks for reporting this issue, I've reproduced the problem and got the following stack trace:

error AD0001: Analyzer 'Microsoft.NetCore.CSharp.Analyzers.Usage.CSharpPreferGenericOverloadsAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.
Exception occurred with following context:
Compilation: JsonApiDotNetCoreTests
IOperation: Invocation
SyntaxTree: W:\JsonApiDotNetCore\test\JsonApiDotNetCoreTests\IntegrationTests\ResourceInheritance\ResourceTypeCaptureStore.cs
SyntaxNode: .LeftType.ClrType.Should().Be(typeof ... [InvocationExpressionSyntax]@[1023..1067) (31,29)-(31,73)

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.CodeAnalysis.CSharp.SyntaxFactory.FindConditionalAccessNodeForBinding(CSharpSyntaxNode node) in W:\roslyn\src\Compilers\CSharp\Portable\Syntax\SyntaxFactory.cs:line 2255
   at Microsoft.CodeAnalysis.CSharp.Binder.GetReceiverForConditionalBinding(ExpressionSyntax binding, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 10990
   at Microsoft.CodeAnalysis.CSharp.Binder.BindMemberBindingExpression(MemberBindingExpressionSyntax node, Boolean invoked, Boolean indexed, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 10959
   at Microsoft.CodeAnalysis.CSharp.Binder.<BindExpressionInternal>g__bindExpressionInternal|332_0(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 657
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpressionInternal(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 579
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 536
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 531
   at Microsoft.CodeAnalysis.CSharp.Binder.BindLeftOfPotentialColorColorMemberAccess(ExpressionSyntax left, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 7385
   at Microsoft.CodeAnalysis.CSharp.Binder.BindMemberAccess(MemberAccessExpressionSyntax node, Boolean invoked, Boolean indexed, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 7339
   at Microsoft.CodeAnalysis.CSharp.Binder.<BindExpressionInternal>g__bindExpressionInternal|332_0(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 622
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpressionInternal(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 579
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 536
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 531
   at Microsoft.CodeAnalysis.CSharp.Binder.BindLeftOfPotentialColorColorMemberAccess(ExpressionSyntax left, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 7385
   at Microsoft.CodeAnalysis.CSharp.Binder.BindMemberAccess(MemberAccessExpressionSyntax node, Boolean invoked, Boolean indexed, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 7339
   at Microsoft.CodeAnalysis.CSharp.Binder.BindMethodGroup(ExpressionSyntax node, Boolean invoked, Boolean indexed, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Invocation.cs:line 33
   at Microsoft.CodeAnalysis.CSharp.Binder.BindInvocationExpression(InvocationExpressionSyntax node, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Invocation.cs:line 211
   at Microsoft.CodeAnalysis.CSharp.Binder.<BindExpressionInternal>g__bindExpressionInternal|332_0(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 602
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpressionInternal(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 579
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics, Boolean invoked, Boolean indexed) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 536
   at Microsoft.CodeAnalysis.CSharp.Binder.BindExpression(ExpressionSyntax node, BindingDiagnosticBag diagnostics) in W:\roslyn\src\Compilers\CSharp\Portable\Binder\Binder_Expressions.cs:line 531
   at Microsoft.CodeAnalysis.CSharp.MemberSemanticModel.GetSpeculativelyBoundExpression(Int32 position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, Binder& binder, ImmutableArray`1& crefSymbols) in W:\roslyn\src\Compilers\CSharp\Portable\Compilation\MemberSemanticModel.cs:line 204
   at Microsoft.CodeAnalysis.CSharp.SyntaxTreeSemanticModel.GetSpeculativelyBoundExpression(Int32 position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, Binder& binder, ImmutableArray`1& crefSymbols) in W:\roslyn\src\Compilers\CSharp\Portable\Compilation\SyntaxTreeSemanticModel.cs:line 753
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSpeculativeSymbolInfo(Int32 position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption) in W:\roslyn\src\Compilers\CSharp\Portable\Compilation\CSharpSemanticModel.cs:line 722
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSpeculativeSymbolInfoCore(Int32 position, SyntaxNode node, SpeculativeBindingOption bindingOption) in W:\roslyn\src\Compilers\CSharp\Portable\Compilation\CSharpSemanticModel.cs:line 5037
   at Microsoft.CodeAnalysis.SemanticModel.GetSpeculativeSymbolInfo(Int32 position, SyntaxNode expression, SpeculativeBindingOption bindingOption) in W:\roslyn\src\Compilers\Core\Portable\Compilation\SemanticModel.cs:line 133
   at Microsoft.CodeAnalysis.ModelExtensions.GetSpeculativeSymbolInfo(SemanticModel semanticModel, Int32 position, SyntaxNode expression, SpeculativeBindingOption bindingOption) in W:\roslyn\src\Compilers\Core\Portable\Compilation\Extensions.cs:line 45
   at Microsoft.NetCore.Analyzers.Usage.PreferGenericOverloadsAnalyzer.<>c__DisplayClass6_0.<AnalyzeInvocation>g__IsApplicableGenericOverload|0(IMethodSymbol method) in W:\roslyn-analyzers\src\NetAnalyzers\Core\Microsoft.NetCore.Analyzers\Usage\PreferGenericOverloads.cs:line 101
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Boolean& found)
   at System.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable`1 source)
   at Microsoft.NetCore.Analyzers.Usage.PreferGenericOverloadsAnalyzer.AnalyzeInvocation(OperationAnalysisContext context) in W:\roslyn-analyzers\src\NetAnalyzers\Core\Microsoft.NetCore.Analyzers\Usage\PreferGenericOverloads.cs:line 51
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c.<ExecuteOperationAction>b__53_0(ValueTuple`2 data) in W:\roslyn\src\Compilers\Core\Portable\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 682
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info, CancellationToken cancellationToken) in W:\roslyn\src\Compilers\Core\Portable\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 1185
-----

You can change the line below as workaround to no longer get a NPE during build. The build then completes without errors.

-        Request.Relationship?.LeftType.ClrType.Should().Be(typeof(TLeft));
+        Request.Relationship?.LeftType.ClrType.Should().Be<TLeft>();

As for why a NPE exception is thrown is not completely clear to me. It happens during speculative binding of the new generic invocation:

https://github.com/dotnet/roslyn-analyzers/blob/8d4f432d69c3c996163e022189ee8c1af4f640e0/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Usage/PreferGenericOverloads.cs#L101-L104

I can reproduce the NPE with the following test case:

[Fact, WorkItem(7421, "https://github.com/dotnet/roslyn-analyzers/issues/7421")]
public async Task Issue7421_OffersFixer_CS()
{
    string source = """
        #nullable enable

        class A
        {
            public B? B { get; set; }
            public C? C { get; set; }
        }

        class B
        {
            public C C { get; set; } = new();
        }

        class C
        {
            public void M(System.Type type) { }
            public void M<T>() { }

            void Test()
            {
                new A().B?.C.M(typeof(C));
            }
        }
        """;

    var test = new VerifyCS.Test
    {
        TestCode = source,
        FixedCode = source,
        LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp9
    };

    await test.RunAsync();
}

This seems to have something to do with the use of a null-conditional operator and not directly calling the method on the result. In other words, if I change the line to either

// Call method 'M' directly after '?.'
new A().C?.M(typeof(C));

or

// Do not use '?.'
new A().B.C.M(typeof(C));

I no longer get a crash. Stepping through shows the following syntax before calling invocationContext.SemanticModel?.GetSpeculativeSymbolInfo:

invocationContext.Syntax is InvocationExpression .C.M(typeof(C)) and modifiedInvocationSyntax is InvocationExpression .C.M<C>()

Based on https://github.com/dotnet/roslyn/issues/72076#issuecomment-1942727001, this could be the same problem as https://github.com/dotnet/roslyn/issues/72076.

cc @CyrusNajmabadi

bkoelman commented 1 month ago

@mpidash Thanks for investigating. I can confirm the suggested change fixes the NRE during build.

bkoelman commented 1 month ago

On a side note, I'm getting the gold bar with crashing analyzers all the time in VS 17.12.0 Preview 2.1. Both during personal development and at work in a very different codebase. Hopefully, telemetry is being collected so this can be fixed before RTM.

{A4D68BAD-DED9-4F44-9362-8980CB17ED17}