dotnet/roslyn-sdk

Obsoletion diagnostic IDs (SYSLIB0xxx) get filtered out in CodeFixTest leading to Exception

mpidash opened this issue · 7 comments

I am writing a fixer that replaces an obsolete call (for dotnet/runtime#27997) that fixes a diagnostic ID starting with SYSLIB0 generated by the compiler by specifying ObsoleteAttribute with a custom DiagnosticId.

I used Microsoft.CodeAnalysis.Testing.EmptyDiagnosticAnalyzer for testing, as I only provide a fixer, but when running my tests an exception is thrown:

ThrowHelper.ThrowNoElementsException()
Enumerable.First[TSource](IEnumerable`1 source)
FixAllContextExtensions.GetDefaultFixAllTitle(FixAllContext context)
BatchFixAllProvider.GetFixAsync(FixAllContext fixAllContext)
CodeFixTest`1.FixAllAnalyerDiagnosticsInScopeAsync(FixAllScope scope, ImmutableArray`1 analyzers, ImmutableArray`1 codeFixProviders, Nullable`1 codeFixIndex, String codeFixEquivalenceKey, Action`2 codeActionVerifier, Project project, Int32 numberOfIterations, IVerifier verifier, CancellationToken cancellationToken) line 836
CodeFixTest`1.VerifyFixAsync(String language, ImmutableArray`1 analyzers, ImmutableArray`1 codeFixProviders, SolutionState oldState, SolutionState newState, Int32 numberOfIterations, Func`10 getFixedProject, IVerifier verifier, CancellationToken cancellationToken) line 504
CodeFixTest`1.VerifyFixAsync(SolutionState testState, SolutionState fixedState, SolutionState batchFixedState, IVerifier verifier, CancellationToken cancellationToken) line 472
CodeFixTest`1.RunImplAsync(CancellationToken cancellationToken) line 310
AnalyzerTest`1.RunAsync(CancellationToken cancellationToken) line 188
DoNotUseThreadVolatileReadWriteTests.Test() line 89
--- End of stack trace from previous location ---

I looked into it a bit and found out that only compiler diagnostic IDs starting with CS or BC are passed to CreateFixAllContext resulting in relevantIds being empty because the compiler generated SYSLIB0xxx is filtered out and Microsoft.CodeAnalysis.Testing.EmptyDiagnosticAnalyzer does not declare supported diagnostics:

var analyzerDiagnosticIds = analyzers.SelectMany(x => x.SupportedDiagnostics).Select(x => x.Id);
var compilerDiagnosticIds = codeFixProviders.SelectMany(codeFixProvider => codeFixProvider.FixableDiagnosticIds).Where(x => x.StartsWith("CS", StringComparison.Ordinal) || x.StartsWith("BC", StringComparison.Ordinal));
var disabledDiagnosticIds = project.CompilationOptions.SpecificDiagnosticOptions.Where(x => x.Value == ReportDiagnostic.Suppress).Select(x => x.Key);
var relevantIds = analyzerDiagnosticIds.Concat(compilerDiagnosticIds).Except(disabledDiagnosticIds).Distinct();
var fixAllContext = CreateFixAllContext(fixableDocument, fixableDocument.Project, effectiveCodeFixProvider!, scope, equivalenceKey, relevantIds, fixAllDiagnosticProvider, cancellationToken);

I was wondering if also adding diagnostic IDs starting with SYSLIB0 (or SYSLIB) is possible (and does not affect existing analyzer tests). The workaround is to create a dummy analyzer that declares the diagnostic ID, something like this:

[SuppressMessage("MicrosoftCodeAnalysisCorrectness", "RS1001:Missing diagnostic analyzer attribute", Justification = "This analyzer is only used in tests.")]
internal sealed class ObsoletionDiagnosticAnalyzer : DiagnosticAnalyzer
{
    // Only used for the diagnostic ID
    internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
        DoNotUseThreadVolatileReadWriteFixer.ObsoleteThreadVolatileReadWriteDiagnosticId,
        MicrosoftNetCoreAnalyzersResources.CreateLocalizableResourceString(MicrosoftNetCoreAnalyzersResources.RemoveRedundantCall),
        MicrosoftNetCoreAnalyzersResources.CreateLocalizableResourceString(MicrosoftNetCoreAnalyzersResources.RemoveRedundantCall),
        DiagnosticCategory.Usage,
        RuleLevel.IdeSuggestion,
        MicrosoftNetCoreAnalyzersResources.CreateLocalizableResourceString(MicrosoftNetCoreAnalyzersResources.RemoveRedundantCall),
        isPortedFxCopRule: false,
        isDataflowRule: false);

    public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

    public override void Initialize(AnalysisContext context)
    {
        context.EnableConcurrentExecution();
        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
    }
}

@mpidash this should now be fixed!

@sharwell Thanks, works like a charm, thanks for fixing this! Do you know when this will be added to dotnet/roslyn-analyzers?

@mpidash As soon as someone sends a PR to update this line:
https://github.com/dotnet/roslyn-analyzers/blob/8b1675a20d889de99684bfced2e0cbc1296c7853/eng/Versions.props#L93

It would need to use version 1.1.2-beta1.23509.1 or newer.

@sharwell I tried to set the version locally to 1.1.2-beta1.23509.1, but it seems that it is not published yet (1.1.2-beta1.23476.1 is the latest version).

Looks like there is some outage in the publication pipeline. Hoping it's published soon.

I am writing a fixer that replaces an obsolete call (for dotnet/runtime#27997) that fixes a diagnostic ID starting with SYSLIB0 generated by the compiler by specifying ObsoleteAttribute with a custom DiagnosticId.

Hey @mpidash thank you for filing this issue, great to see it is fixed. How is it going with the fixer implementation? Have you seen this PR dotnet/roslyn-analyzers#7043? Would you like to give any feedback there?

@buyaa-n I have a fixer that uses the new SDK (so no additional analyzer needed) with basic tests. I stopped working on it when I saw the PR from @CollinAlpert. I will have a look at it 👍