Add support for reporting analyzer diagnostics in editorconfig files

[mavasani]

@mgoertz-msft FYI that your original feature request has now been implemented for 16.8 P1 with https://github.com/dotnet/roslyn/pull/45076. I will keep this issue open to track exposing a similar API for analyzer config files.

See open question in https://github.com/dotnet/roslyn/issues/44131#issuecomment-628722999 Originally posted by @mavasani in https://github.com/dotnet/roslyn/issues/44131#issuecomment-658887799

[mavasani]

Proposal

Feature motivation

This would help us address the open question mentioned in https://github.com/dotnet/roslyn/issues/44131#issuecomment-628722999 and finally help us notify users about mistakes in editorconfig files, both in CI and in the IDE:

Should we expose a similar API for analyzer config files? This will allow us to write analyzers for mistakes in editorconfig files, which show up on opening these files. For example, we can finally address https://github.com/dotnet/roslyn/issues/19055 if RegisterAnalyzerConfigFileAction were to be added.

We have repeatedly received github issues from users for incorrect entries added to editorconfig, so this will alleviate this pain point. We also have a story board for potential future items that can be implemented if this feature is implemented: .editorconfig Validation During Compilation

Design

Public API surface for analyzer config file actions would be analogous to AdditionalFile actions and SyntaxTree actions. Approved API for AdditionalFile actions is in this comment: https://github.com/dotnet/roslyn/issues/44131#issuecomment-628722999.

1. Add a new AnalyzerConfigFileAnalysisContext, analogous to AdditionalFileAnalysisContext:

// Existing API for AdditionalFile analysis context
public readonly struct AdditionalFileAnalysisContext
{
    public AdditionalText AdditionalFile { get; }
    public AnalyzerOptions Options { get; }
    public CancellationToken CancellationToken { get; }
    public Compilation Compilation { get; }

    public void ReportDiagnostic(Diagnostic diagnostic);
}

// New API for AnalyzerConfigFile analysis context
public readonly struct AnalyzerConfigFileAnalysisContext
{
    public AdditionalText AnalyzerConfigFile { get; }
    public AnalyzerOptions Options { get; }
    public CancellationToken CancellationToken { get; }
    public Compilation Compilation { get; }

    public void ReportDiagnostic(Diagnostic diagnostic);
}

2. Add a new RegisterAnalyzerConfigFileAction API to AnalysisContext and CompilationStartAnalysisContext:

// Existing API for registering callbacks for additional files
public void RegisterAdditionalFileAction(Action<AdditionalFileAnalysisContext> action);

// New API for registering callbacks for editorconfig files
public void RegisterAnalyzerConfigFileAction(Action<AnalyzerConfigFileAnalysisContext> action);

3. Expose AnalyzerConfigFiles as text files from AnalyzerOptions. Users can fetch the raw SourceText and file path of the editorconfig files from exposed AdditionalText. We also add new public constructors to AnalyzerOptions to specify analyzer config files:

public class AnalyzerOptions
{
    // Existing API
    public ImmutableArray<AdditionalText> AdditionalFiles { get; }

    // New API
    public ImmutableArray<AdditionalText> AnalyzerConfigFiles { get; }

    // Existing constructors
    public AnalyzerOptions(ImmutableArray<AdditionalText> additionalFiles);
    public AnalyzerOptions(ImmutableArray<AdditionalText> additionalFiles, AnalyzerConfigOptionsProvider optionsProvider);

    // New constructors
    public AnalyzerOptions(ImmutableArray<AdditionalText> additionalFiles, ImmutableArray<AdditionalText> analyzerConfigFiles);
    public AnalyzerOptions(ImmutableArray<AdditionalText> additionalFiles, ImmutableArray<AdditionalText> analyzerConfigFiles, AnalyzerConfigOptionsProvider optionsProvider);

    // Existing WithXXX methods
    public AnalyzerOptions WithAdditionalFiles(ImmutableArray<AdditionalText> additionalFiles);

    // New WithXXX method
    public AnalyzerOptions WithAnalyzerConfigFiles(ImmutableArray<AdditionalText> analyzerConfigFiles);
}

4. Expose AnalyzerConfigFileDiagnostics map from AnalysisResult:

public class AnalysisResult
{
    // Existing API for map of diagnostics reported in each AdditionaFile by analyzers.
    public ImmutableDictionary<AdditionalText, ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>>> AdditionalFileDiagnostics { get; }

    // New API for map of diagnostics reported in each AnalyzerConfigFile by analyzers.
    public ImmutableDictionary<AdditionalText, ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>>> AnalyzerConfigFileDiagnostics { get; }
}

[mgoertz-msft]

Awesome, thank you @mavasani! This will come in really handy when we start looking at improving the .NET Upgrade Assistant for Maui.

[333fred]

API Review

• Analyzer config files are a hierarchy, but this operates on a single file. Is that good enough for analyzers?
  • Analysis might be "not enough keys set", which could be non-observable from an individual file.
• The analysis context gets plain text, can we expose the stuff we've already parsed?
  • Is the user just going to have to reparse the file?
  • Our parser doesn't keep track of location today, so keeping ourselves and users who are attempting to give diagnostics in sync for understanding the editorconfig file will be challenging with this API.
• If we don't want to provide any more structure, can we just add them to the things RegisterAdditionalFileAction is called back for?

Conclusion: Needs more work to address the above questions.

[mavasani]

Analyzer config files are a hierarchy, but this operates on a single file. Is that good enough for analyzers? Analysis might be "not enough keys set", which could be non-observable from an individual file.

AFAIK, "not enough keys set" is not going to be a mainline scenario, if at all. The primary analysis scenarios would be for individual analyzer config files:

1. Invalid option value(s) for well-known option keys.
2. Invalid option key suffix for well-known option key prefixes
3. Invalid syntax, such as invalid headers, invalid globalconfig format, etc.
4. Duplicate entries with same key in the file
5. Hidden diagnostics to enable code fixes for cleaning up or enhancing the content of each editorconfig file

We can always revisit this and add a separate callback with all analyzer config files provided at once, if we get such a request.

The analysis context gets plain text, can we expose the stuff we've already parsed? Is the user just going to have to reparse the file? Our parser doesn't keep track of location today, so keeping ourselves and users who are attempting to give diagnostics in sync for understanding the editorconfig file will be challenging with this API.

Yes, that is going to be the core issue. If we want to expose parsed, structured data, then we need to expose locations of the parsed data for reporting diagnostics. Additionally, it adds a constraint on the structure of the internal parsed data as soon as we expose it publicly. IMO, exposing raw text that user parses is the most flexible.

If we don't want to provide any more structure, can we just add them to the things RegisterAdditionalFileAction is called back for?

I see couple of problems with this approach:

1. Globalconfig files, unlike editorconfig files, can have any name/extension. So, it won't be possible to differentiate all the analyzer config files from the true additional files in the AdditionalFileAction callbacks. User would have to parse each file, see if the file begins with the required globalconfig header to identify globalconfig files.
2. This deviates from the IDE Workspace API shape, where Project exposes AnalyzerConfigDocuments and AdditionalDocuments as separate types, and we also have separate TextDocumentKind.AnalyzerConfigDocument and TextDocumentKind.AdditionalDocument. I think keeping the analyzer config files and additional files separate even in the analyzer/compiler layer is more logical.

[Sergio0694]

This would be really nice to have. I also hit this same limitation in PolySharp the other day, where I wanted to emit some diagnostics for incorrect MSBuild properties being passed to the analyzer. Ended up having to write a source generator that only gets properties from the analyzer config and emits diagnostics (see PR here). Works just fine, but it's not ideal since it now can't run OOP (as it's not an analyzer), and the diagnostics don't show up nicely in solution explorer when expanding the analyzer node. Having a way to just emit diagnostics from RegisterCompilationStartAction would be great, since all the info needed is there already, there's just no way to emit diagnostics from there at all right now 😅