Skip to content

Simplify internal analyzer api #78070

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 9, 2025
Merged

Simplify internal analyzer api #78070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,13 +575,13 @@ public MockDocumentDiagnosticAnalyzer()

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument document, SyntaxTree? tree, CancellationToken cancellationToken)
{
ReceivedCallback = true;
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument document, SyntaxTree? tree, CancellationToken cancellationToken)
{
ReceivedCallback = true;
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
Expand Down
13 changes: 3 additions & 10 deletions src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,10 +891,6 @@ protected PriorityTestDocumentDiagnosticAnalyzer(int priority)

public override int Priority { get; }
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [];
public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
=> Task.FromResult(ImmutableArray<Diagnostic>.Empty);
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
=> Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

private sealed class LeakDocumentAnalyzer : DocumentDiagnosticAnalyzer
Expand All @@ -903,14 +899,11 @@ private sealed class LeakDocumentAnalyzer : DocumentDiagnosticAnalyzer

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [s_syntaxRule];

public override async Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument document, SyntaxTree tree, CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
return [Diagnostic.Create(s_syntaxRule, root.GetLocation())];
return Task.FromResult<ImmutableArray<Diagnostic>>(
[Diagnostic.Create(s_syntaxRule, tree.GetRoot(cancellationToken).GetLocation())]);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
=> SpecializedTasks.Default<ImmutableArray<Diagnostic>>();
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
Expand Down
6 changes: 1 addition & 5 deletions src/EditorFeatures/TestUtilities/Workspaces/NoCompilationDocumentDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.UnitTests;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;

Expand All @@ -21,10 +20,7 @@ internal sealed class NoCompilationDocumentDiagnosticAnalyzer : DocumentDiagnost

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Descriptor];

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
=> SpecializedTasks.EmptyImmutableArray<Diagnostic>();

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument document, SyntaxTree tree, CancellationToken cancellationToken)
{
return Task.FromResult(ImmutableArray.Create(
Diagnostic.Create(Descriptor, Location.Create(document.FilePath, default, default))));
Expand Down
5 changes: 1 addition & 4 deletions src/LanguageServer/ProtocolUnitTests/Diagnostics/AbstractPullDiagnosticTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,7 @@ private sealed class MockTypescriptDiagnosticAnalyzer : DocumentDiagnosticAnalyz

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Descriptor];

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
=> SpecializedTasks.EmptyImmutableArray<Diagnostic>();

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument document, SyntaxTree? tree, CancellationToken cancellationToken)
Copy link
Contributor

@ToddGrun ToddGrun Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TextDocument

no need to check for it being a Document?

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. This is just text code

{
return Task.FromResult(ImmutableArray.Create(
Diagnostic.Create(Descriptor, Location.Create(document.FilePath!, default, default))));
Expand Down
26 changes: 10 additions & 16 deletions ...sualStudio/ExternalAccess/FSharp/Internal/Diagnostics/FSharpDocumentDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,26 +67,20 @@ public static ImmutableArray<DiagnosticDescriptor> CreateSupportedDiagnostics()

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => _supportedDiagnostics;

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree tree, CancellationToken cancellationToken)
{
var analyzer = document.Project.Services.GetService<FSharpDocumentDiagnosticAnalyzerService>();
if (analyzer == null)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

return analyzer.AnalyzeSemanticsAsync(document, cancellationToken);
var analyzer = textDocument.Project.Services.GetService<FSharpDocumentDiagnosticAnalyzerService>();
return analyzer is null || textDocument is not Document document
? []
: await analyzer.AnalyzeSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument textDocument, SyntaxTree tree, CancellationToken cancellationToken)
{
var analyzer = document.Project.Services.GetService<FSharpDocumentDiagnosticAnalyzerService>();
if (analyzer == null)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

return analyzer.AnalyzeSyntaxAsync(document, cancellationToken);
var analyzer = textDocument.Project.Services.GetService<FSharpDocumentDiagnosticAnalyzerService>();
return analyzer is null || textDocument is not Document document
? []
: await analyzer.AnalyzeSyntaxAsync(document, cancellationToken).ConfigureAwait(false);
}

public DiagnosticAnalyzerCategory GetAnalyzerCategory()
Expand Down
18 changes: 5 additions & 13 deletions ...Studio/ExternalAccess/FSharp/Internal/Diagnostics/FSharpSimplifyNameDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,12 @@ internal class FSharpSimplifyNameDiagnosticAnalyzer : DocumentDiagnosticAnalyzer

public override int Priority => 100; // Default = 50

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree tree, CancellationToken cancellationToken)
{
var analyzer = document.Project.Services.GetService<FSharpSimplifyNameDiagnosticAnalyzerService>();
if (analyzer == null)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

return analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
var analyzer = textDocument.Project.Services.GetService<FSharpSimplifyNameDiagnosticAnalyzerService>();
return analyzer is null || textDocument is not Document document
? []
: await analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken).ConfigureAwait(false);
}

public DiagnosticAnalyzerCategory GetAnalyzerCategory()
Expand Down
18 changes: 5 additions & 13 deletions ...sualStudio/ExternalAccess/FSharp/Internal/Diagnostics/FSharpUnusedDeclarationsAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,12 @@ internal class FSharpUnusedDeclarationsDiagnosticAnalyzer : DocumentDiagnosticAn

public override int Priority => 80; // Default = 50

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree tree, CancellationToken cancellationToken)
{
var analyzer = document.Project.Services.GetService<FSharpUnusedDeclarationsDiagnosticAnalyzerService>();
if (analyzer == null)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

return analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
var analyzer = textDocument.Project.Services.GetService<FSharpUnusedDeclarationsDiagnosticAnalyzerService>();
return analyzer is null || textDocument is not Document document
? []
: await analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken).ConfigureAwait(false);
}

public DiagnosticAnalyzerCategory GetAnalyzerCategory()
Expand Down
18 changes: 5 additions & 13 deletions ...lStudio/ExternalAccess/FSharp/Internal/Diagnostics/FSharpUnusedOpensDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,12 @@ internal class FSharpUnusedOpensDeclarationsDiagnosticAnalyzer : DocumentDiagnos

public override int Priority => 90; // Default = 50

public override Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree tree, CancellationToken cancellationToken)
{
var analyzer = document.Project.Services.GetService<FSharpUnusedOpensDiagnosticAnalyzerService>();
if (analyzer == null)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
}

return analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken);
}

public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
{
return Task.FromResult(ImmutableArray<Diagnostic>.Empty);
var analyzer = textDocument.Project.Services.GetService<FSharpUnusedOpensDiagnosticAnalyzerService>();
return analyzer is null || textDocument is not Document document
? []
: await analyzer.AnalyzeSemanticsAsync(_descriptor, document, cancellationToken).ConfigureAwait(false);
}

public DiagnosticAnalyzerCategory GetAnalyzerCategory()
Expand Down
35 changes: 9 additions & 26 deletions src/VisualStudio/Xaml/Impl/Diagnostics/Analyzers/XamlDocumentDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -17,30 +15,15 @@ namespace Microsoft.CodeAnalysis.Xaml.Diagnostics.Analyzers;
internal sealed class XamlDocumentDiagnosticAnalyzer : DocumentDiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
get
{
return XamlProjectService.AnalyzerService?.SupportedDiagnostics ?? [];
}
}

public override async Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
{
if (XamlProjectService.AnalyzerService == null)
{
return [];
}

return await XamlProjectService.AnalyzerService.AnalyzeSyntaxAsync(document, cancellationToken).ConfigureAwait(false);
}
=> XamlProjectService.AnalyzerService?.SupportedDiagnostics ?? [];

public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
{
if (XamlProjectService.AnalyzerService == null)
{
return [];
}
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
=> XamlProjectService.AnalyzerService is null || textDocument is not Document document
? []
: await XamlProjectService.AnalyzerService.AnalyzeSyntaxAsync(document, cancellationToken).ConfigureAwait(false);
Copy link
Contributor

@ToddGrun ToddGrun Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await

Does this need to be awaited?

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. because on different paths we return different values.

Copy link
Contributor

@ToddGrun ToddGrun Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making sure we're thinking of the same change:

    public override Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
        => XamlProjectService.AnalyzerService is null || textDocument is not Document document
            ? SpecializedTasks.EmptyImmutableArray<Diagnostic>()
            : XamlProjectService.AnalyzerService.AnalyzeSyntaxAsync(document, cancellationToken);

Copy link
Contributor

@ToddGrun ToddGrun Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi -- not a deal breaker, just curious why the suggested code wouldn't work

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work... Or just the code already there :-)

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not huge on avoiding async/await personally.


return await XamlProjectService.AnalyzerService.AnalyzeSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
}
public override async Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
=> XamlProjectService.AnalyzerService is null || textDocument is not Document document
? []
: await XamlProjectService.AnalyzerService.AnalyzeSemanticsAsync(document, cancellationToken).ConfigureAwait(false);
}
16 changes: 1 addition & 15 deletions src/Workspaces/Core/Portable/Diagnostics/DocumentDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,9 @@ internal abstract class DocumentDiagnosticAnalyzer : DiagnosticAnalyzer
public const int DefaultPriority = 50;

public virtual Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
{
return textDocument is Document document
? AnalyzeSyntaxAsync(document, cancellationToken)
: SpecializedTasks.EmptyImmutableArray<Diagnostic>();
}

public virtual Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
{
return textDocument is Document document
? AnalyzeSemanticsAsync(document, cancellationToken)
: SpecializedTasks.EmptyImmutableArray<Diagnostic>();
}

public virtual Task<ImmutableArray<Diagnostic>> AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken)
=> SpecializedTasks.EmptyImmutableArray<Diagnostic>();

public virtual Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(Document document, CancellationToken cancellationToken)
public virtual Task<ImmutableArray<Diagnostic>> AnalyzeSemanticsAsync(TextDocument textDocument, SyntaxTree? tree, CancellationToken cancellationToken)
=> SpecializedTasks.EmptyImmutableArray<Diagnostic>();

/// <summary>
Expand Down
Loading