Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mpidash opened this issue Sep 3, 2023 · 7 comments · Fixed by #1118
Closed
Assignees
Labels
Area-MS.CA.Testing Microsoft.CodeAnalysis.Testing bug fixed

Comments

@mpidash
Copy link
Contributor

mpidash commented Sep 3, 2023

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);
    }
}
@sharwell
Copy link
Member

sharwell commented Oct 9, 2023

@mpidash this should now be fixed!

@mpidash
Copy link
Contributor Author

mpidash commented Oct 9, 2023

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

@sharwell
Copy link
Member

sharwell commented Oct 9, 2023

@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.

@mpidash
Copy link
Contributor Author

mpidash commented Oct 9, 2023

@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).

@sharwell
Copy link
Member

sharwell commented Oct 9, 2023

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

@buyaa-n
Copy link

buyaa-n commented Mar 2, 2024

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?

@mpidash
Copy link
Contributor Author

mpidash commented Mar 2, 2024

@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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-MS.CA.Testing Microsoft.CodeAnalysis.Testing bug fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants