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

WIP: Remove source generator diagnostic reporting from Workspace #76863

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 3 additions & 12 deletions src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,7 @@ public async Task TestGetFixesAsyncForDocumentDiagnosticAnalyzerAsync()
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67354")]
public async Task TestGetFixesAsyncForGeneratorDiagnosticAsync()
{
// We have a special GeneratorDiagnosticsPlaceholderAnalyzer that report 0 SupportedDiagnostics.
// We need to ensure that we don't skip this special analyzer
// when computing the diagnostics/code fixes for "Normal" priority bucket, which
// normally only execute those analyzers which report at least one fixable supported diagnostic.
// Note that this special placeholder analyzer instance is always included for the project,
// we do not need to include it in the passed in analyzers.
Assert.Empty(GeneratorDiagnosticsPlaceholderAnalyzer.Instance.SupportedDiagnostics);
// We do not get generator diagnostics, and so we should not have code fixes for them.

var analyzers = ImmutableArray<DiagnosticAnalyzer>.Empty;
var generator = new MockAnalyzerReference.MockGenerator();
Expand All @@ -208,11 +202,8 @@ public async Task TestGetFixesAsyncForGeneratorDiagnosticAsync()
var fixCollectionSet = await tuple.codeFixService.GetFixesAsync(document, TextSpan.FromBounds(0, 0),
priorityProvider: new DefaultCodeActionRequestPriorityProvider(CodeActionRequestPriority.Default),
cancellationToken: CancellationToken.None);
Assert.True(codeFix.Called);
var fixCollection = Assert.Single(fixCollectionSet);
Assert.Equal(MockFixer.Id, fixCollection.FirstDiagnostic.Id);
var fix = Assert.Single(fixCollection.Fixes);
Assert.Equal(fixTitle, fix.Action.Title);
Assert.False(codeFix.Called);
Assert.Empty(fixCollectionSet);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ public async Task TestHostAnalyzerOrderingAsync()
AssertEx.Equal(
[
typeof(FileContentLoadAnalyzer),
typeof(GeneratorDiagnosticsPlaceholderAnalyzer),
typeof(CSharpCompilerDiagnosticAnalyzer),
typeof(Analyzer),
typeof(Priority0Analyzer),
Expand Down Expand Up @@ -847,9 +846,8 @@ void M()
Assert.Equal(CancellationTestAnalyzer.DiagnosticId, diagnostic.Id);
}

[Theory, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1909806")]
[CombinatorialData]
internal async Task TestGeneratorProducedDiagnostics(bool fullSolutionAnalysis, bool analyzeProject, TestHost testHost)
[Theory, CombinatorialData, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1909806")]
internal async Task TestGeneratorDoesNotProduceDiagnostics(bool fullSolutionAnalysis, bool analyzeProject, TestHost testHost)
{
using var workspace = EditorTestWorkspace.CreateCSharp("// This file will get a diagnostic", composition: s_featuresCompositionWithMockDiagnosticUpdateSourceRegistrationService.WithTestHostParts(testHost));

Expand All @@ -875,7 +873,7 @@ internal async Task TestGeneratorProducedDiagnostics(bool fullSolutionAnalysis,
var incrementalAnalyzer = service.CreateIncrementalAnalyzer(workspace);
var diagnostics = await incrementalAnalyzer.ForceAnalyzeProjectAsync(project, CancellationToken.None);

Assert.NotEmpty(diagnostics);
Assert.Empty(diagnostics);
}

private static Document GetDocumentFromIncompleteProject(AdhocWorkspace workspace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Linq;
using System.Reflection;
using Microsoft.CodeAnalysis.Diagnostics.Telemetry;
using Microsoft.CodeAnalysis.Simplification;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics;
Expand All @@ -18,8 +17,7 @@ internal static class DiagnosticAnalyzerExtensions
public static bool IsWorkspaceDiagnosticAnalyzer(this DiagnosticAnalyzer analyzer)
=> analyzer is DocumentDiagnosticAnalyzer
|| analyzer is ProjectDiagnosticAnalyzer
|| analyzer == FileContentLoadAnalyzer.Instance
|| analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance;
|| analyzer == FileContentLoadAnalyzer.Instance;

public static bool IsBuiltInAnalyzer(this DiagnosticAnalyzer analyzer)
=> analyzer is IBuiltInAnalyzer || analyzer.IsWorkspaceDiagnosticAnalyzer() || analyzer.IsCompilerAnalyzer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ namespace Microsoft.CodeAnalysis.Diagnostics;
internal interface IRemoteDiagnosticAnalyzerService
{
ValueTask<SerializableDiagnosticAnalysisResults> CalculateDiagnosticsAsync(Checksum solutionChecksum, DiagnosticArguments arguments, CancellationToken cancellationToken);
ValueTask<ImmutableArray<DiagnosticData>> GetSourceGeneratorDiagnosticsAsync(Checksum solutionChecksum, ProjectId projectId, CancellationToken cancellationToken);
ValueTask ReportAnalyzerPerformanceAsync(ImmutableArray<AnalyzerPerformanceInfo> snapshot, int unitCount, bool forSpanAnalysis, CancellationToken cancellationToken);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Portable/EditAndContinue/EditSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,8 @@ internal static async Task PopulateChangedAndAddedDocumentsAsync(Project oldProj

private static async ValueTask<TextDocumentStates<SourceGeneratedDocumentState>> GetSourceGeneratedDocumentStatesAsync(Project project, ArrayBuilder<ProjectDiagnostics>? diagnostics, CancellationToken cancellationToken)
{
var generatorDiagnostics = await project.Solution.CompilationState.GetSourceGeneratorDiagnosticsAsync(project.State, cancellationToken).ConfigureAwait(false);

var runResult = await project.Solution.CompilationState.GetSourceGeneratorRunResultAsync(project.State, cancellationToken).ConfigureAwait(false);
var generatorDiagnostics = runResult?.Diagnostics ?? [];
if (generatorDiagnostics is not [])
{
diagnostics?.Add(new ProjectDiagnostics(project.Id, generatorDiagnostics));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1808,7 +1808,8 @@ public async Task HasChanges_SourceGeneratorFailure()
AssertEx.AreEqual("generated: text", generatedText);
Assert.Equal(1, generatorExecutionCount);

var generatorDiagnostics = await solution.CompilationState.GetSourceGeneratorDiagnosticsAsync(project.State, CancellationToken.None);
var runResult = await solution.CompilationState.GetSourceGeneratorRunResultAsync(project.State, CancellationToken.None);
var generatorDiagnostics = runResult.Diagnostics;
Assert.Empty(generatorDiagnostics);

var debuggingSession = await StartDebuggingSessionAsync(service, solution);
Expand Down Expand Up @@ -1841,7 +1842,8 @@ public async Task HasChanges_SourceGeneratorFailure()
generatedDocuments = await solution.CompilationState.GetSourceGeneratedDocumentStatesAsync(project.State, CancellationToken.None);
Assert.Empty(generatedDocuments.States);

generatorDiagnostics = await solution.CompilationState.GetSourceGeneratorDiagnosticsAsync(project.State, CancellationToken.None);
runResult = await solution.CompilationState.GetSourceGeneratorRunResultAsync(project.State, CancellationToken.None);
generatorDiagnostics = runResult.Diagnostics;
Assert.Contains("System.InvalidOperationException: Source generator failed", generatorDiagnostics.Single().GetMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ public async Task SourceGeneratorFailure()
AssertEx.AreEqual("generated: text", generatedText);
Assert.Equal(1, generatorExecutionCount);

var generatorDiagnostics = await solution.CompilationState.GetSourceGeneratorDiagnosticsAsync(project.State, CancellationToken.None);
var runResult = await solution.CompilationState.GetSourceGeneratorRunResultAsync(project.State, CancellationToken.None);
var generatorDiagnostics = runResult.Diagnostics;
Assert.Empty(generatorDiagnostics);

var hotReload = new WatchHotReloadService(workspace.Services, ["Baseline", "AddDefinitionToExistingType", "NewTypeDefinition"]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,6 @@ public async Task<IEnumerable<DiagnosticData>> ComputeDiagnosticsAsync(Diagnosti
if (loadDiagnostic != null)
return [];

if (analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance)
{
// We will count generator diagnostics as semantic diagnostics; some filtering to either syntax/semantic is necessary or else we'll report diagnostics twice.
if (kind == AnalysisKind.Semantic)
{
var generatorDiagnostics = await GetSourceGeneratorDiagnosticsAsync(textDocument.Project, cancellationToken).ConfigureAwait(false);
return ConvertToLocalDiagnostics(generatorDiagnostics, textDocument, span);
}
else
{
return [];
}
}

if (analyzer is DocumentDiagnosticAnalyzer documentAnalyzer)
{
if (document == null)
Expand Down Expand Up @@ -196,19 +182,6 @@ private async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisRes
}
}

private async Task<ImmutableArray<Diagnostic>> GetSourceGeneratorDiagnosticsAsync(Project project, CancellationToken cancellationToken)
{
try
{
return await _diagnosticAnalyzerRunner.GetSourceGeneratorDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
}
catch when (_onAnalysisException != null)
{
_onAnalysisException.Invoke();
throw;
}
}

private async Task<ImmutableArray<DiagnosticData>> GetCompilerAnalyzerDiagnosticsAsync(DiagnosticAnalyzer analyzer, TextSpan? span, CancellationToken cancellationToken)
{
RoslynDebug.Assert(analyzer.IsCompilerAnalyzer());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private static void AssertCompilation(Project project, Compilation compilation1)
public static bool IsAnalyzerEnabledForProject(DiagnosticAnalyzer analyzer, Project project, IGlobalOptionService globalOptions)
{
var options = project.CompilationOptions;
if (options == null || analyzer == FileContentLoadAnalyzer.Instance || analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance)
if (options == null || analyzer == FileContentLoadAnalyzer.Instance)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,19 +325,6 @@ private async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisRes
others: [],
documentIds: null));

var generatorDiagnostics = await _diagnosticAnalyzerRunner.GetSourceGeneratorDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
var diagnosticResultBuilder = new DiagnosticAnalysisResultBuilder(project, version);
foreach (var generatorDiagnostic in generatorDiagnostics)
{
// We'll always treat generator diagnostics that are associated with a tree as a local diagnostic, because
// we want that to be refreshed and deduplicated with regular document analysis.
diagnosticResultBuilder.AddDiagnosticTreatedAsLocalSemantic(generatorDiagnostic);
}

results = results.SetItem(
GeneratorDiagnosticsPlaceholderAnalyzer.Instance,
DiagnosticAnalysisResult.CreateFromBuilder(diagnosticResultBuilder));

return (results, failedDocuments?.ToImmutable());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,29 +88,6 @@ async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAnalysisRes
}
}

public async Task<ImmutableArray<Diagnostic>> GetSourceGeneratorDiagnosticsAsync(Project project, CancellationToken cancellationToken)
{
if (!_enabled)
return [];

var options = project.Solution.Services.GetRequiredService<IWorkspaceConfigurationService>().Options;
var remoteHostClient = await RemoteHostClient.TryGetClientAsync(project, cancellationToken).ConfigureAwait(false);
if (remoteHostClient != null)
{
var result = await remoteHostClient.TryInvokeAsync<IRemoteDiagnosticAnalyzerService, ImmutableArray<DiagnosticData>>(
project.Solution,
invocation: (service, solutionInfo, cancellationToken) => service.GetSourceGeneratorDiagnosticsAsync(solutionInfo, project.Id, cancellationToken),
cancellationToken).ConfigureAwait(false);

if (!result.HasValue)
return [];

return await result.Value.ToDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false);
}

return await project.GetSourceGeneratorDiagnosticsAsync(cancellationToken).ConfigureAwait(false);
}

private async Task<DiagnosticAnalysisResultMap<DiagnosticAnalyzer, DiagnosticAnalysisResult>> AnalyzeInProcAsync(
DocumentAnalysisScope? documentAnalysisScope,
Project project,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ static ImmutableHashSet<object> GetFeaturesAnalyzerReferenceIds(HostDiagnosticAn

private sealed class HostAnalyzerStateSets
{
private const int FileContentLoadAnalyzerPriority = -4;
private const int GeneratorDiagnosticsPlaceholderAnalyzerPriority = -3;
private const int FileContentLoadAnalyzerPriority = -3;
private const int BuiltInCompilerPriority = -2;
private const int RegularDiagnosticAnalyzerPriority = -1;

Expand Down Expand Up @@ -158,7 +157,6 @@ private static int GetPriority(StateSet state)
return state.Analyzer switch
{
FileContentLoadAnalyzer _ => FileContentLoadAnalyzerPriority,
GeneratorDiagnosticsPlaceholderAnalyzer _ => GeneratorDiagnosticsPlaceholderAnalyzerPriority,
DocumentDiagnosticAnalyzer analyzer => Math.Max(0, analyzer.Priority),
ProjectDiagnosticAnalyzer analyzer => Math.Max(0, analyzer.Priority),
_ => RegularDiagnosticAnalyzerPriority,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,13 @@ private static ImmutableDictionary<DiagnosticAnalyzer, StateSet> CreateStateSetM
if (includeWorkspacePlaceholderAnalyzers)
{
builder.Add(FileContentLoadAnalyzer.Instance, new StateSet(language, FileContentLoadAnalyzer.Instance, isHostAnalyzer: true));
builder.Add(GeneratorDiagnosticsPlaceholderAnalyzer.Instance, new StateSet(language, GeneratorDiagnosticsPlaceholderAnalyzer.Instance, isHostAnalyzer: true));
}

foreach (var analyzers in projectAnalyzerCollection)
{
foreach (var analyzer in analyzers)
{
Debug.Assert(analyzer != FileContentLoadAnalyzer.Instance && analyzer != GeneratorDiagnosticsPlaceholderAnalyzer.Instance);
Debug.Assert(analyzer != FileContentLoadAnalyzer.Instance);

// TODO:
// #1, all de-duplication should move to DiagnosticAnalyzerInfoCache
Expand All @@ -167,7 +166,7 @@ private static ImmutableDictionary<DiagnosticAnalyzer, StateSet> CreateStateSetM
{
foreach (var analyzer in analyzers)
{
Debug.Assert(analyzer != FileContentLoadAnalyzer.Instance && analyzer != GeneratorDiagnosticsPlaceholderAnalyzer.Instance);
Debug.Assert(analyzer != FileContentLoadAnalyzer.Instance);

// TODO:
// #1, all de-duplication should move to DiagnosticAnalyzerInfoCache
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,6 @@ static bool ShouldIncludeAnalyzer(
if (analyzer is DocumentDiagnosticAnalyzer)
return true;

// Special case GeneratorDiagnosticsPlaceholderAnalyzer to never skip it based on
// 'shouldIncludeDiagnostic' predicate. More specifically, this is a placeholder analyzer
// for threading through all source generator reported diagnostics, but this special analyzer
// reports 0 supported diagnostics, and we always want to execute it.
if (analyzer is GeneratorDiagnosticsPlaceholderAnalyzer)
return true;

// Skip analyzer if none of its reported diagnostics should be included.
if (shouldIncludeDiagnostic != null &&
!owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).Any(static (a, shouldIncludeDiagnostic) => shouldIncludeDiagnostic(a.Id), shouldIncludeDiagnostic))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, boo
{
// PERF: Don't query descriptors for compiler analyzer or workspace load analyzer, always execute them.
if (analyzer == FileContentLoadAnalyzer.Instance ||
analyzer == GeneratorDiagnosticsPlaceholderAnalyzer.Instance ||
analyzer.IsCompilerAnalyzer())
{
return true;
Expand Down
Loading
Loading