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

Implement MEF based pull diagnostics #73073

Merged
merged 27 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageServer;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSources;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript;
Expand All @@ -17,8 +18,9 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript;
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal class VSTypeScriptDocumentPullDiagnosticHandlerFactory(
IDiagnosticAnalyzerService analyzerService,
IDiagnosticSourceManager diagnosticSourceManager,
IDiagnosticsRefresher diagnosticsRefresher,
IGlobalOptionService globalOptions) : DocumentPullDiagnosticHandlerFactory(analyzerService, diagnosticsRefresher, globalOptions)
IGlobalOptionService globalOptions) : DocumentPullDiagnosticHandlerFactory(analyzerService, diagnosticSourceManager, diagnosticsRefresher, globalOptions)
{
}

Expand All @@ -28,7 +30,8 @@ internal class VSTypeScriptDocumentPullDiagnosticHandlerFactory(
internal class VSTypeScriptWorkspacePullDiagnosticHandler(
LspWorkspaceRegistrationService registrationService,
IDiagnosticAnalyzerService analyzerService,
IDiagnosticSourceManager diagnosticSourceManager,
IDiagnosticsRefresher diagnosticsRefresher,
IGlobalOptionService globalOptions) : WorkspacePullDiagnosticHandlerFactory(registrationService, analyzerService, diagnosticsRefresher, globalOptions)
IGlobalOptionService globalOptions) : WorkspacePullDiagnosticHandlerFactory(registrationService, analyzerService, diagnosticSourceManager, diagnosticsRefresher, globalOptions)
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageServer;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSources;
using Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens;
using Microsoft.CodeAnalysis.Options;
using Microsoft.VisualStudio.Composition;
Expand Down Expand Up @@ -40,9 +41,11 @@ internal class AlwaysActivateInProcLanguageClient(
ILspServiceLoggerFactory lspLoggerFactory,
IThreadingContext threadingContext,
ExportProvider exportProvider,
IDiagnosticSourceManager diagnosticSourceManager,
[ImportMany] IEnumerable<Lazy<ILspBuildOnlyDiagnostics, ILspBuildOnlyDiagnosticsMetadata>> buildOnlyDiagnostics) : AbstractInProcLanguageClient(lspServiceProvider, globalOptions, lspLoggerFactory, threadingContext, exportProvider)
{
private readonly ExperimentalCapabilitiesProvider _experimentalCapabilitiesProvider = defaultCapabilitiesProvider;
private readonly IDiagnosticSourceManager _diagnosticSourceManager = diagnosticSourceManager;
private readonly IEnumerable<Lazy<ILspBuildOnlyDiagnostics, ILspBuildOnlyDiagnosticsMetadata>> _buildOnlyDiagnostics = buildOnlyDiagnostics;

protected override ImmutableArray<string> SupportedLanguages => ProtocolConstants.RoslynLspLanguages;
Expand All @@ -69,28 +72,15 @@ public override ServerCapabilities GetCapabilities(ClientCapabilities clientCapa

serverCapabilities.SupportsDiagnosticRequests = true;
serverCapabilities.DiagnosticProvider ??= new();

// VS does not distinguish between document and workspace diagnostics, so we need to merge them.
var diagnosticSourceNames = _diagnosticSourceManager.GetSourceNames(true)
etvorun marked this conversation as resolved.
Show resolved Hide resolved
.Concat(_diagnosticSourceManager.GetSourceNames(false))
.Distinct();
serverCapabilities.DiagnosticProvider = serverCapabilities.DiagnosticProvider with
{
SupportsMultipleContextsDiagnostics = true,
DiagnosticKinds =
[
// Support a specialized requests dedicated to task-list items. This way the client can ask just
// for these, independently of other diagnostics. They can also throttle themselves to not ask if
// the task list would not be visible.
new(PullDiagnosticCategories.Task),
new(PullDiagnosticCategories.EditAndContinue),
// Dedicated request for workspace-diagnostics only. We will only respond to these if FSA is on.
new(PullDiagnosticCategories.WorkspaceDocumentsAndProject),
// Fine-grained diagnostics requests. Importantly, this separates out syntactic vs semantic
// requests, allowing the former to quickly reach the user without blocking on the latter. In a
// similar vein, compiler diagnostics are explicitly distinct from analyzer-diagnostics, allowing
// the former to appear as soon as possible as they are much more critical for the user and should
// not be delayed by a slow analyzer.
new(PullDiagnosticCategories.DocumentCompilerSyntax),
new(PullDiagnosticCategories.DocumentCompilerSemantic),
new(PullDiagnosticCategories.DocumentAnalyzerSyntax),
new(PullDiagnosticCategories.DocumentAnalyzerSemantic),
],
DiagnosticKinds = diagnosticSourceNames.Select(n => new VSInternalDiagnosticKind(n)).ToArray(),
BuildOnlyDiagnosticIds = _buildOnlyDiagnostics
.SelectMany(lazy => lazy.Metadata.BuildOnlyDiagnostics)
.Distinct()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Razor.UnitTests" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.UnitTesting" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.VisualDiagnostics" />
etvorun marked this conversation as resolved.
Show resolved Hide resolved
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.ExternalAccess.Xaml" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.Features.DiagnosticsTests.Utilities" />
<InternalsVisibleTo Include="Microsoft.CodeAnalysis.Features.Test.Utilities" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,6 @@ public ServerCapabilities GetCapabilities(ClientCapabilities clientCapabilities)
// Using VS server capabilities because we have our own custom client.
capabilities.OnAutoInsertProvider = new VSInternalDocumentOnAutoInsertOptions { TriggerCharacters = ["'", "/", "\n"] };

if (!supportsVsExtensions)
{
capabilities.DiagnosticOptions = new DiagnosticOptions
{
InterFileDependencies = true,
WorkDoneProgress = true,
WorkspaceDiagnostics = true,
};
}
etvorun marked this conversation as resolved.
Show resolved Hide resolved

return capabilities;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.FindUsages;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private sealed class OpenDocumentSource(Document document) : AbstractDocumentDia
public override bool IsLiveSource()
=> true;

public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken)
public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
{
var designTimeDocument = Document;
var designTimeSolution = designTimeDocument.Project.Solution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private sealed class ProjectSource(Project project, ImmutableArray<DiagnosticDat
public override bool IsLiveSource()
=> true;

public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken)
public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(diagnostics);
}

Expand All @@ -31,7 +31,7 @@ private sealed class ClosedDocumentSource(TextDocument document, ImmutableArray<
public override bool IsLiveSource()
=> true;

public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken)
public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
=> Task.FromResult(diagnostics);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +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.

using System;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Options;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;

internal abstract class AbstractDocumentDiagnosticSourceProvider(string name) : IDiagnosticSourceProvider
{
public bool IsDocument => true;
public string Name => name;

public abstract ValueTask<ImmutableArray<IDiagnosticSource>> CreateDiagnosticSourcesAsync(RequestContext context, CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,61 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSources;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Roslyn.LanguageServer.Protocol;
using LSP = Roslyn.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;

internal abstract class AbstractDocumentPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>(
IDiagnosticAnalyzerService diagnosticAnalyzerService,
IDiagnosticsRefresher diagnosticRefresher,
IDiagnosticSourceManager diagnosticSourceManager,
IGlobalOptionService globalOptions)
: AbstractPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>(
diagnosticAnalyzerService,
diagnosticRefresher,
globalOptions), ITextDocumentIdentifierHandler<TDiagnosticsParams, TextDocumentIdentifier?>
where TDiagnosticsParams : IPartialResultParams<TReport>
{
public abstract LSP.TextDocumentIdentifier? GetTextDocumentIdentifier(TDiagnosticsParams diagnosticsParams);
protected readonly IDiagnosticSourceManager DiagnosticSourceManager = diagnosticSourceManager;

public abstract TextDocumentIdentifier? GetTextDocumentIdentifier(TDiagnosticsParams diagnosticsParams);

protected override ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagnosticSourcesAsync(TDiagnosticsParams diagnosticsParams, string? requestDiagnosticCategory, RequestContext context, CancellationToken cancellationToken)
{
if (GetOpenDocument(context) is null)
return new([]);

return DiagnosticSourceManager.CreateDiagnosticSourcesAsync(context, requestDiagnosticCategory, isDocument: true, cancellationToken);
}

protected static TextDocument? GetOpenDocument(RequestContext context)
etvorun marked this conversation as resolved.
Show resolved Hide resolved
{
// Note: context.Document may be null in the case where the client is asking about a document that we have
// since removed from the workspace. In this case, we don't really have anything to process.
// GetPreviousResults will be used to properly realize this and notify the client that the doc is gone.
//
// Only consider open documents here (and only closed ones in the WorkspacePullDiagnosticHandler). Each
// handler treats those as separate worlds that they are responsible for.
var textDocument = context.TextDocument;
if (textDocument is null)
{
context.TraceInformation("Ignoring diagnostics request because no text document was provided");
return null;
}

if (!context.IsTracking(textDocument.GetURI()))
{
context.TraceWarning($"Ignoring diagnostics request for untracked document: {textDocument.GetURI()}");
return null;
}
etvorun marked this conversation as resolved.
Show resolved Hide resolved

return textDocument;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +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.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down Expand Up @@ -38,8 +37,6 @@ internal abstract partial class AbstractPullDiagnosticHandler<TDiagnosticsParams
/// </summary>
protected const int WorkspaceDiagnosticIdentifier = 1;
protected const int DocumentDiagnosticIdentifier = 2;
// internal for testing purposes
internal const int DocumentNonLocalDiagnosticIdentifier = 3;

private readonly IDiagnosticsRefresher _diagnosticRefresher;
protected readonly IGlobalOptionService GlobalOptions;
Expand Down Expand Up @@ -68,8 +65,6 @@ protected AbstractPullDiagnosticHandler(
GlobalOptions = globalOptions;
}

protected virtual string? GetDiagnosticSourceIdentifier(TDiagnosticsParams diagnosticsParams) => null;

/// <summary>
/// Retrieve the previous results we reported. Used so we can avoid resending data for unchanged files. Also
/// used so we can report which documents were removed and can have all their diagnostics cleared.
Expand All @@ -80,7 +75,7 @@ protected AbstractPullDiagnosticHandler(
/// Returns all the documents that should be processed in the desired order to process them in.
/// </summary>
protected abstract ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagnosticSourcesAsync(
TDiagnosticsParams diagnosticsParams, RequestContext context, CancellationToken cancellationToken);
TDiagnosticsParams diagnosticsParams, string? requestDiagnosticCategory, RequestContext context, CancellationToken cancellationToken);

/// <summary>
/// Creates the appropriate LSP type to report a new set of diagnostics and resultId.
Expand All @@ -106,7 +101,7 @@ protected abstract ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagno
/// </summary>
protected abstract DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource);

protected abstract string? GetDiagnosticCategory(TDiagnosticsParams diagnosticsParams);
protected abstract string? GetRequestDiagnosticCategory(TDiagnosticsParams diagnosticsParams);

/// <summary>
/// Used by public workspace pull diagnostics to allow it to keep the connection open until
Expand Down Expand Up @@ -134,9 +129,8 @@ protected virtual Task WaitForChangesAsync(RequestContext context, CancellationT
Contract.ThrowIfNull(context.Solution);

var clientCapabilities = context.GetRequiredClientCapabilities();
var category = GetDiagnosticCategory(diagnosticsParams) ?? "";
var sourceIdentifier = GetDiagnosticSourceIdentifier(diagnosticsParams) ?? "";
etvorun marked this conversation as resolved.
Show resolved Hide resolved
var handlerName = $"{this.GetType().Name}(category: {category}, source: {sourceIdentifier})";
var category = GetRequestDiagnosticCategory(diagnosticsParams);
var handlerName = $"{this.GetType().Name}(category: {category})";
context.TraceInformation($"{handlerName} started getting diagnostics");

var versionedCache = _categoryToVersionedCache.GetOrAdd(handlerName, static handlerName => new(handlerName));
Expand All @@ -160,7 +154,7 @@ protected virtual Task WaitForChangesAsync(RequestContext context, CancellationT
// Next process each file in priority order. Determine if diagnostics are changed or unchanged since the
// last time we notified the client. Report back either to the client so they can update accordingly.
var orderedSources = await GetOrderedDiagnosticSourcesAsync(
diagnosticsParams, context, cancellationToken).ConfigureAwait(false);
diagnosticsParams, category, context, cancellationToken).ConfigureAwait(false);

context.TraceInformation($"Processing {orderedSources.Length} documents");

Expand Down Expand Up @@ -295,7 +289,7 @@ private async Task ComputeAndReportCurrentDiagnosticsAsync(
CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<LSP.Diagnostic>.GetInstance(out var result);
var diagnostics = await diagnosticSource.GetDiagnosticsAsync(DiagnosticAnalyzerService, context, cancellationToken).ConfigureAwait(false);
var diagnostics = await diagnosticSource.GetDiagnosticsAsync(context, cancellationToken).ConfigureAwait(false);

// If we can't get a text document identifier we can't report diagnostics for this source.
// This can happen for 'fake' projects (e.g. used for TS script blocks).
Expand Down
Loading
Loading