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.

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

Remove nested types in diagnostic analyzer service #77124

Merged
merged 9 commits into from
Feb 11, 2025
Merged

Remove nested types in diagnostic analyzer service #77124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
809eddd
Remove inner class
CyrusNajmabadi Feb 9, 2025
126b5a9
Name
CyrusNajmabadi Feb 9, 2025
55c02e9
Remove inner class
CyrusNajmabadi Feb 9, 2025
d7c3556
Cleanup
CyrusNajmabadi Feb 9, 2025
fa379b6
xml docs
CyrusNajmabadi Feb 9, 2025
f09861a
Make local function
CyrusNajmabadi Feb 9, 2025
98b5528
Merge branch 'projectDiagBased' into localFunctionDiags
CyrusNajmabadi Feb 10, 2025
b3b79e3
Merge branch 'projectDiagBased' into localFunctionDiags
CyrusNajmabadi Feb 10, 2025
dba01c3
Merge branch 'main' into localFunctionDiags
CyrusNajmabadi Feb 10, 2025
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
2 changes: 0 additions & 2 deletions src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Runtime.CompilerServices;
Expand All @@ -13,7 +12,6 @@
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.SolutionCrawler;
using Microsoft.CodeAnalysis.Text;
Expand Down
197 changes: 86 additions & 111 deletions ...er/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,57 +18,82 @@ internal partial class DiagnosticAnalyzerService
{
private partial class DiagnosticIncrementalAnalyzer
{
public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Project project, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new DiagnosticGetter(this, project, documentId, diagnosticIds, shouldIncludeAnalyzer, includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics).GetDiagnosticsAsync(cancellationToken);
public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForIdsAsync(Project project, DocumentId? documentId, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeLocalDocumentDiagnostics, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
{
return await ProduceProjectDiagnosticsAsync(
project, diagnosticIds, shouldIncludeAnalyzer,
// Ensure we compute and return diagnostics for both the normal docs and the additional docs in this
// project if no specific document id was requested.
documentId != null ? [documentId] : [.. project.DocumentIds, .. project.AdditionalDocumentIds],
includeLocalDocumentDiagnostics,
includeNonLocalDocumentDiagnostics,
// return diagnostics specific to one project or document
includeProjectNonLocalResult: documentId == null,
cancellationToken).ConfigureAwait(false);
}

public Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(Project project, ImmutableHashSet<string>? diagnosticIds, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer, bool includeNonLocalDocumentDiagnostics, CancellationToken cancellationToken)
=> new DiagnosticGetter(this, project, documentId: null, diagnosticIds, shouldIncludeAnalyzer, includeLocalDocumentDiagnostics: false, includeNonLocalDocumentDiagnostics).GetProjectDiagnosticsAsync(cancellationToken);
public async Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(
Project project,
ImmutableHashSet<string>? diagnosticIds,
Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer,
bool includeNonLocalDocumentDiagnostics,
CancellationToken cancellationToken)
{
return await ProduceProjectDiagnosticsAsync(
project, diagnosticIds, shouldIncludeAnalyzer,
documentIds: [],
includeLocalDocumentDiagnostics: false,
includeNonLocalDocumentDiagnostics: includeNonLocalDocumentDiagnostics,
includeProjectNonLocalResult: true,
cancellationToken).ConfigureAwait(false);
}

private sealed class DiagnosticGetter(
DiagnosticIncrementalAnalyzer owner,
private async Task<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync(
Project project,
DocumentId? documentId,
ImmutableHashSet<string>? diagnosticIds,
Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer,
IReadOnlyList<DocumentId> documentIds,
bool includeLocalDocumentDiagnostics,
bool includeNonLocalDocumentDiagnostics)
Copy link
Member Author

Choose a reason for hiding this comment

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

general idea is that instead of making tehse nested types, which have to grab all this data, just to store fields, just to use in methods within the type, we just use local functions.

bool includeNonLocalDocumentDiagnostics,
bool includeProjectNonLocalResult,
CancellationToken cancellationToken)
{
private readonly DiagnosticIncrementalAnalyzer Owner = owner;

private readonly Project Project = project;
private readonly DocumentId? DocumentId = documentId;
private readonly ImmutableHashSet<string>? _diagnosticIds = diagnosticIds;
private readonly Func<DiagnosticAnalyzer, bool>? _shouldIncludeAnalyzer = shouldIncludeAnalyzer;
private readonly bool IncludeLocalDocumentDiagnostics = includeLocalDocumentDiagnostics;
private readonly bool IncludeNonLocalDocumentDiagnostics = includeNonLocalDocumentDiagnostics;
using var _ = ArrayBuilder<DiagnosticData>.GetInstance(out var builder);

private StateManager StateManager => Owner._stateManager;
var analyzersForProject = await _stateManager.GetOrCreateAnalyzersAsync(project, cancellationToken).ConfigureAwait(false);
var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false);
var analyzers = analyzersForProject.WhereAsArray(a => ShouldIncludeAnalyzer(project, a));

private bool ShouldIncludeDiagnostic(DiagnosticData diagnostic)
=> _diagnosticIds == null || _diagnosticIds.Contains(diagnostic.Id);
var result = await GetOrComputeDiagnosticAnalysisResultsAsync(analyzers).ConfigureAwait(false);

public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(CancellationToken cancellationToken)
foreach (var analyzer in analyzers)
{
// return diagnostics specific to one project or document
var includeProjectNonLocalResult = DocumentId == null;
return await ProduceProjectDiagnosticsAsync(
// Ensure we compute and return diagnostics for both the normal docs and the additional docs in this
// project if no specific document id was requested.
this.DocumentId != null ? [this.DocumentId] : [.. this.Project.DocumentIds, .. this.Project.AdditionalDocumentIds],
includeProjectNonLocalResult, cancellationToken).ConfigureAwait(false);
}
if (!result.TryGetValue(analyzer, out var analysisResult))
continue;

private async Task<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync(
IReadOnlyList<DocumentId> documentIds,
bool includeProjectNonLocalResult, CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<DiagnosticData>.GetInstance(out var builder);
await this.ProduceDiagnosticsAsync(
documentIds, includeProjectNonLocalResult, builder, cancellationToken).ConfigureAwait(false);
return builder.ToImmutableAndClear();
foreach (var documentId in documentIds)
{
if (includeLocalDocumentDiagnostics)
{
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax));
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic));
}

if (includeNonLocalDocumentDiagnostics)
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.NonLocal));
}

// include project diagnostics if there is no target document
if (includeProjectNonLocalResult)
AddIncludedDiagnostics(builder, analysisResult.GetOtherDiagnostics());
}

private void AddIncludedDiagnostics(ArrayBuilder<DiagnosticData> builder, ImmutableArray<DiagnosticData> diagnostics)
return builder.ToImmutableAndClear();

bool ShouldIncludeDiagnostic(DiagnosticData diagnostic)
=> diagnosticIds == null || diagnosticIds.Contains(diagnostic.Id);

void AddIncludedDiagnostics(ArrayBuilder<DiagnosticData> builder, ImmutableArray<DiagnosticData> diagnostics)
{
foreach (var diagnostic in diagnostics)
{
Expand All @@ -77,93 +102,43 @@ private void AddIncludedDiagnostics(ArrayBuilder<DiagnosticData> builder, Immuta
}
}

public async Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsAsync(CancellationToken cancellationToken)
{
return await ProduceProjectDiagnosticsAsync(
documentIds: [], includeProjectNonLocalResult: true, cancellationToken).ConfigureAwait(false);
}

private async Task ProduceDiagnosticsAsync(
IReadOnlyList<DocumentId> documentIds,
bool includeProjectNonLocalResult,
ArrayBuilder<DiagnosticData> builder,
CancellationToken cancellationToken)
async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> GetOrComputeDiagnosticAnalysisResultsAsync(
ImmutableArray<DiagnosticAnalyzer> analyzers)
{
var project = this.Project;
var analyzersForProject = await StateManager.GetOrCreateAnalyzersAsync(project, cancellationToken).ConfigureAwait(false);
var hostAnalyzerInfo = await StateManager.GetOrCreateHostAnalyzerInfoAsync(project, cancellationToken).ConfigureAwait(false);
var analyzers = analyzersForProject.WhereAsArray(a => ShouldIncludeAnalyzer(project, a));

var result = await GetOrComputeDiagnosticAnalysisResultsAsync(analyzers).ConfigureAwait(false);

foreach (var analyzer in analyzers)
// If there was a 'ForceAnalyzeProjectAsync' run for this project, we can piggy back off of the
// prior computed/cached results as they will be a superset of the results we want.
//
// Note: the caller will loop over *its* analzyers, grabbing from the full set of data we've cached
// for this project, and filtering down further. So it's ok to return this potentially larger set.
//
// Note: While ForceAnalyzeProjectAsync should always run with a larger set of analyzers than us
// (since it runs all analyzers), we still run a paranoia check that the analyzers we care about are
// a subset of that call so that we don't accidentally reuse results that would not correspond to
// what we are computing ourselves.
if (_projectToForceAnalysisData.TryGetValue(project, out var box) &&
analyzers.IsSubsetOf(box.Value.analyzers))
{
if (!result.TryGetValue(analyzer, out var analysisResult))
continue;

foreach (var documentId in documentIds)
{
if (IncludeLocalDocumentDiagnostics)
{
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Syntax));
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.Semantic));
}

if (IncludeNonLocalDocumentDiagnostics)
AddIncludedDiagnostics(builder, analysisResult.GetDocumentDiagnostics(documentId, AnalysisKind.NonLocal));
}

if (includeProjectNonLocalResult)
{
// include project diagnostics if there is no target document
AddIncludedDiagnostics(builder, analysisResult.GetOtherDiagnostics());
}
return box.Value.diagnosticAnalysisResults;
}

async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> GetOrComputeDiagnosticAnalysisResultsAsync(
ImmutableArray<DiagnosticAnalyzer> analyzers)
{
// If there was a 'ForceAnalyzeProjectAsync' run for this project, we can piggy back off of the
// prior computed/cached results as they will be a superset of the results we want.
//
// Note: the caller will loop over *its* analzyers, grabbing from the full set of data we've cached
// for this project, and filtering down further. So it's ok to return this potentially larger set.
//
// Note: While ForceAnalyzeProjectAsync should always run with a larger set of analyzers than us
// (since it runs all analyzers), we still run a paranoia check that the analyzers we care about are
// a subset of that call so that we don't accidentally reuse results that would not correspond to
// what we are computing ourselves.
if (this.Owner._projectToForceAnalysisData.TryGetValue(project, out var box) &&
analyzers.IsSubsetOf(box.Value.analyzers))
{
return box.Value.diagnosticAnalysisResults;
}

// Otherwise, just compute for the analyzers we care about.
var compilation = await GetOrCreateCompilationWithAnalyzersAsync(
project, analyzers, hostAnalyzerInfo, Owner.AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
// Otherwise, just compute for the analyzers we care about.
var compilation = await GetOrCreateCompilationWithAnalyzersAsync(
project, analyzers, hostAnalyzerInfo, AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);

var result = await Owner.ComputeDiagnosticAnalysisResultsAsync(compilation, project, analyzers, cancellationToken).ConfigureAwait(false);
return result;
}
var result = await ComputeDiagnosticAnalysisResultsAsync(compilation, project, analyzers, cancellationToken).ConfigureAwait(false);
return result;
}

private bool ShouldIncludeAnalyzer(Project project, DiagnosticAnalyzer analyzer)
bool ShouldIncludeAnalyzer(Project project, DiagnosticAnalyzer analyzer)
{
if (!DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(analyzer, project, Owner.GlobalOptions))
{
if (!DocumentAnalysisExecutor.IsAnalyzerEnabledForProject(analyzer, project, this.GlobalOptions))
return false;
}

if (_shouldIncludeAnalyzer != null && !_shouldIncludeAnalyzer(analyzer))
{
if (shouldIncludeAnalyzer != null && !shouldIncludeAnalyzer(analyzer))
return false;
}

if (_diagnosticIds != null && Owner.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).All(d => !_diagnosticIds.Contains(d.Id)))
{
if (diagnosticIds != null && this.DiagnosticAnalyzerInfoCache.GetDiagnosticDescriptors(analyzer).All(d => !diagnosticIds.Contains(d.Id)))
return false;
}

return true;
}
Expand Down
Loading
Loading