dotnet/roslyn-analyzers

Intermittent NullReferenceException from RS1022 (in a unit test project)

bradwilson opened this issue · 4 comments

bradwilson commented

Analyzer

Diagnostic ID: RS1022: Do not use types from Workspaces assembly in an analyzer

Analyzer source

NuGet Package: Microsoft.CodeAnalysis.Analyzers

Version: 3.3.4 (Latest)

Describe the bug

My project (xunit/xunit.analyzers) contains three projects:

  • xunit.analyzers.csproj (analyzers, netstandard2.0)
  • xunit.analyzers.fixes.csproj (fixers, netstandard2.0)
  • xunit.analyzers.tests.csproj (tests for both, net472 and net6.0)

The references are as follows:

  • xunit.analyzers.csproj references:
    • Microsoft.CodeAnalysis.Analyzers version 3.3.4
    • Microsoft.CodeAnalysis.CSharp version 4.6.0
  • xunit.analyzers.fixes.csproj references:
    • xunit.analyzers (as project reference)
    • Microsoft.CodeAnalysis.Analyzers version 3.3.4
    • Microsoft.CodeAnalysis.CSharp version 4.6.0
    • Microsoft.CodeAnalysis.CSharp.Workspaces version 4.6.0
  • xunit.analyzers.tests.csproj references:
    • xunit.analyzers (as project reference)
    • xunit.analyzers.fixes (as project reference)
    • Microsoft.CodeAnalysis.CSharp.CodeFix.Testing version 1.1.1
    • xunit.assert.source version 2.5.0-pre.21 (this is relevant)

From a clean environment (git clean -xdf then dotnet build) I will occasionally get a NullReferenceException from RS1022:

CSC error AD0001: Analyzer 'Microsoft.CodeAnalysis.CSharp.Analyzers.MetaAnalyzers.CSharpDiagnosticAnalyzerApiUsageAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [C:\Dev\xunit\analyzers\src\xunit.analyzers.tests\xunit.analyzers.tests.csproj]
Exception occurred with following context:
Compilation: xunit.analyzers.tests
ISymbol: CollectionTrackerExtensions (NamedType)
System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.DiagnosticAnalyzerApiUsageAnalyzer`1.AddUsedNamedTypeCore(ITypeSymbol typeOpt, Builder builder, Boolean& hasAccessToTypeFromWorkspaceAssemblies)
at Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.DiagnosticAnalyzerApiUsageAnalyzer`1.AddUsedNamedTypeCore(ITypeSymbol typeOpt, Builder builder, Boolean& hasAccessToTypeFromWorkspaceAssemblies)
at Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.DiagnosticAnalyzerApiUsageAnalyzer`1.GetUsedNamedTypes(INamedTypeSymbol namedType, Compilation compilation, CancellationToken cancellationToken, Boolean& hasAccessToTypeFromWorkspaceAssemblies)
at Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.DiagnosticAnalyzerApiUsageAnalyzer`1.<>c__DisplayClass11_0.<Initialize>b__1(SymbolAnalysisContext symbolContext)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c.<ExecuteSymbolActions>b__48_1(ValueTuple`2 data)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)

The reason I mention the reference to xunit.assert.source is that the type in question (CollectionTrackerExtensions) comes from that NuGet package as source.

Steps To Reproduce

Pre-requisites: Install .NET SDK 6.0.

  1. git clone --recurse-submodules https://github.com/xunit/xunit.analyzers
  2. cd xunit.analyzers
  3. git switch -d c6b8c4808acc70a3aa70a1cf4ef57cf4949b6547
  4. Edit src\xunit.analyzers.tests\xunit.analyzers.tests.csproj and remove the
  5. Remove or comment out the line with <NoWarn>
  6. Now, repeatedly run:
    a. git clean -xdf
    b. dotnet build -bl

You should eventually hit upon a failing build, and the binlog should show the exception pasted above.

Expected behavior

No exception.

Actual behavior

Occasional exception.

sharwell commented

This should have been fixed by #2015. Are you certain the build is using 3.3.4 for that project? Today when I hit this error in a different project, the binlog revealed that the compilation was using 2.6.1.

bradwilson commented

This is the package reference: https://github.com/xunit/xunit.analyzers/blob/895196119062c71564167aa2be55022d846bd717/src/Directory.Build.props#L45

Since that's an indirect reference, it's possible that the (now significantly out of date) reference from Microsoft.CodeAnalysis.CSharp.CodeFix.Testing (which is referencing 2.6.1) is somehow taking precedence. I don't want or care about the analyzers running against my tests (would prefer they didn't, honestly), so of course I never took a direct reference.

Taking a direct reference "fixes" the problem (it doesn't fix the problem that I don't want the analyzers to run, but at least it'll stop breaking my build).

bradwilson commented

Out of curiosity: why does Microsoft.CodeAnalysis.CSharp.CodeFix.Testing bring along a mandatory reference to Microsoft.CodeAnalysis.Analyzers? This code is intended to be used by unit tests. Analyzing unit tests with rules designed for production code doesn't make sense to me.

sharwell commented

why does Microsoft.CodeAnalysis.CSharp.CodeFix.Testing bring along a mandatory reference to Microsoft.CodeAnalysis.Analyzers

Microsoft.CodeAnalysis.Common has a public dependency on the analyzers package, so it's unavoidable as a transitive dependency.