dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.95k stars 4.02k forks source link

Internal NullReferenceException in CSharpValidateFormatStringDiagnosticAnalyzer when using custom workspace without WorkspaceAnalyzerOptions #21886

Open ashmind opened 7 years ago

ashmind commented 7 years ago

Version Used: master branch, 1a36abc673dcba88c79f6b769c9722f56f5c9f63

Steps to Reproduce:

Unfortunately I can't reproduce it with NuGet versions, and a I don't have time to build a test case on top of master. However I'm sure example at #16644 should be able to reproduce it if run against master (with SourceText updated as per step 2).

  1. Create an AdHocWorkspace or another custom workspace
  2. Create compilation based on valid code using Console.WriteLine():
    using System;
    class C {
    void M() {
        Console.WriteLine();
    }
    }
  3. Run compilation.WithAnalyzers().GetAllDiagnosticsAsync()

Expected Behavior: There are no diagnostics produced as the code is valid.

Actual Behavior: There is one diagnostic:

warning AD0001: Analyzer 'Microsoft.CodeAnalysis.CSharp.ValidateFormatString.CSharpValidateFormatStringDiagnosticAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.

Technical Notes: Looking at the code in https://github.com/dotnet/roslyn/blob/5632597039206c667e2cfb145988739acf7a5379/src/Features/Core/Portable/ValidateFormatString/AbstractValidateFormatStringDiagnosticAnalyzer.cs#L82-L85 it seems similar to the problem in #16644: https://github.com/dpoeschl/roslyn/commit/6375c4d2ff159d56edca0563ea21aec7221cdc89#diff-203dcb8c3cb1796c2a260bc8d40f03b4R21.

ashmind commented 7 years ago

Submitted this by mistake before filling in details, so closed at first -- then realized I can reopen after filling.

ashmind commented 7 years ago

Removed link to SharpLab demo: it now uses reflection to create WorkspaceAnalyzerOptions so the bug doesn't apply anymore.

dpoeschl commented 7 years ago

@chborl For now, we should probably bail if we can't get the WorkspaceAnalyzerOptions. @ashmind Is this the only one you're encountering problems with?

ashmind commented 7 years ago

@dpoeschl Yes, that's the only one. Though I needed CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer from #16644, so I created WorkspaceAnalyzerOptions with reflection and now I don't have this problem either.

Would be great if it didn't require reflection though.