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

Roslyn Analyzer not clearing last diagnostic #53192

Closed
joncoello opened this issue May 5, 2021 · 9 comments · Fixed by #58479
Closed

Roslyn Analyzer not clearing last diagnostic #53192

joncoello opened this issue May 5, 2021 · 9 comments · Fixed by #58479
Assignees
Milestone

Comments

@joncoello
Copy link

I have created a Roslyn Analyzer that validates text files against a set of rules and have noticed that when I fix the last issue that diagnostic does not disappear. I have created a simple example to replicate the problem below.

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class Analyzer1Analyzer : DiagnosticAnalyzer
{
    public const string DiagnosticId = "TestCompilationAnalyzer";

    private static readonly LocalizableString Title = "TestAnalyzer";
    private static readonly LocalizableString MessageFormat = "This message should always appear {0}";
    private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, "Test", DiagnosticSeverity.Warning, isEnabledByDefault: true);
    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Rule); } }

    public override void Initialize(AnalysisContext context)
    {
        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
        context.EnableConcurrentExecution();
        context.RegisterAdditionalFileAction(AdditionalFileAction);
    }

    private void AdditionalFileAction(AdditionalFileAnalysisContext cxt)
    {
        var text = File.ReadAllText(cxt.AdditionalFile.Path);

        if (text.Contains("help"))
        {
            cxt.ReportDiagnostic(Diagnostic.Create(Rule, Location.None, $"help"));
        }

        if (text.Contains("test"))
        {
            cxt.ReportDiagnostic(Diagnostic.Create(Rule, Location.None, $"test"));
        }
    }
}

if I add the words help and test to my additional file both diagnostics appear but if I remove both whatever was showing last won't disappear.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels May 5, 2021
@sharwell
Copy link
Member

sharwell commented May 5, 2021

Location.None

This issue is likely related to the use of no-location diagnostics. Updating the result to report the diagnostic within the context of a specific additional file should allow the diagnostic to be cleared when that file is closed.

@joncoello
Copy link
Author

joncoello commented May 7, 2021

To update I have found a workaround using code generators (expand below for example)

⚠ Note from @sharwell: Don't do this unless you want the IDE to be very slow with no workaround
    [Generator]
    public class SchemaTestAnalyzer : ISourceGenerator
    {

        public const string DiagnosticId = "TestCompilationAnalyzer";

        private static readonly LocalizableString Title = "TestAnalyzer";
        private static readonly LocalizableString MessageFormat = "This message should always appear {0}";
        private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, "Test", DiagnosticSeverity.Warning, isEnabledByDefault: true);

        public void Execute(GeneratorExecutionContext context)
        {
            var additionalFile = context.AdditionalFiles.FirstOrDefault();
            if (additionalFile != null)
            {
                var text = File.ReadAllText(additionalFile.Path);
                if (text.Contains("help"))
                {
                    context.ReportDiagnostic(Diagnostic.Create(Rule, Location.None, "help"));
                }

                if (text.Contains("test"))
                {
                    context.ReportDiagnostic(Diagnostic.Create(Rule, Location.None, "test"));
                }
            }
        }

        public void Initialize(GeneratorInitializationContext context) { }
    }

These clear as expected when the last issue is addressed

@sharwell
Copy link
Member

sharwell commented May 7, 2021

@joncoello That approach comes with a non-trivial performance penalty. Is there a reason why the reported diagnostics cannot be updated to set the location within the additional file?

@sharwell
Copy link
Member

sharwell commented May 7, 2021

@joncoello The sample is very interesting, but due to the magnitude of the performance impact incurred I've added a warning to it so users don't think this is something worth trying. It will have really, really bad impacts.

@sharwell
Copy link
Member

sharwell commented May 7, 2021

@mavasani I think the solution here is to internally associate no-location diagnostics in live analysis with the source getting analyzed at the time the diagnostic was reported, so if that source is analyzed again later it knows to clear the diagnostic.

@joncoello
Copy link
Author

@sharwell fair enough - I'd be happy to go back to using CompilationAction but the last issue I clear doesn't go and Locations don't work in non-code files

@sharwell
Copy link
Member

sharwell commented May 7, 2021

Locations don't work in non-code files

Locations should work fine in non-code files. Public API analyzer reports diagnostics in additional files.
https://github.com/dotnet/roslyn-analyzers/blob/2f9d19d930e402485147e74f5f089f92219680ed/src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs#L719-L721

@joncoello
Copy link
Author

I have updated my original example with a location but the diagnostics are now not appearing

[DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class Analyzer1Analyzer : DiagnosticAnalyzer
    {
        public const string DiagnosticId = "TestCompilationAnalyzer";

        private static readonly LocalizableString Title = "TestAnalyzer";
        private static readonly LocalizableString MessageFormat = "This message should always appear {0}";
        private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, "Test", DiagnosticSeverity.Warning, isEnabledByDefault: true);
        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Rule); } }

        public override void Initialize(AnalysisContext context)
        {
            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
            context.EnableConcurrentExecution();
            context.RegisterAdditionalFileAction(AdditionalFileAction);
        }

        private void AdditionalFileAction(AdditionalFileAnalysisContext cxt)
        {
            var fileText = cxt.AdditionalFile.GetText();

            foreach (var line in fileText.Lines)
            {

                var linePositionSpan = fileText.Lines.GetLinePositionSpan(line.Span);
                var location = Location.Create(cxt.AdditionalFile.Path, line.Span, linePositionSpan);

                if (line.Text.ToString().Contains("help"))
                {
                    cxt.ReportDiagnostic(Diagnostic.Create(Rule, location, $"help"));
                }

                if (line.Text.ToString().Contains("test"))
                {
                    cxt.ReportDiagnostic(Diagnostic.Create(Rule, location, $"test"));
                }
            }

        }
    }

Passing in Location.None makes them appear again but back to the original problem of the last diagnostic not disappearing

@jinujoseph jinujoseph added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels May 12, 2021
@jinujoseph jinujoseph modified the milestones: 17.0, 16.11 May 12, 2021
@jinujoseph jinujoseph modified the milestones: 16.11, 17.0 Jul 16, 2021
@jinujoseph jinujoseph modified the milestones: 17.0, 17.1 Dec 10, 2021
@mavasani
Copy link
Contributor

Investigated a bit more and identified that AdditionalFileActions are not being consistently invoked in the IDE during intellisense builds. These actions are consistently invoked during explicit builds and the diagnostics show up fine. I'll continue investigating the IDE issue.

mavasani added a commit to mavasani/roslyn that referenced this issue Dec 24, 2021
Fixes dotnet#53192
Currently, work coordinator only enqueues source document IDs for project analysis work items. This means we don't analyze the additional documents, which can now report standalone diagnostics through RegisterAdditionalFileAction. This PR fixes this by enqueuing project's additional document IDs and analyzer config document IDs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants