Skip to content

fix: Populate diagnostic fields on send codeAction #5341

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

Open
apatenotre wants to merge 1 commit into
base: main
Choose a base branch
from
+201 −84
Open

fix: Populate diagnostic fields on send codeAction #5341

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
32 changes: 32 additions & 0 deletions monaco-lsp-client/src/adapters/DiagnosticsCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Diagnostic } from '../../src/types';

/**
* Per-document cache of the last set of LSP `Diagnostic` objects received
* from the server (via `publishDiagnostics` or `textDocument/diagnostic`).
*
* Monaco's `IMarker` model can't carry the full Diagnostic shape: `data` and
* `codeDescription` have no corresponding IMarker field, so once a Diagnostic
* is translated to a marker those values are lost. Features that need to
* round-trip the original Diagnostic to the server (notably
* `textDocument/codeAction`, which the spec requires to include `data`) read
* from this cache to recover the missing fields.
*
* Keyed by Monaco URI string (`model.uri.toString()`) so cleanup can be done
* directly from `monaco.editor.onWillDisposeModel` without needing the
* text-model bridge.
*/
export class DiagnosticsCache {
private readonly _byUri = new Map<string, readonly Diagnostic[]>();

public set(uri: string, diagnostics: readonly Diagnostic[]): void {
this._byUri.set(uri, diagnostics);
}

public get(uri: string): readonly Diagnostic[] | undefined {
return this._byUri.get(uri);
}

public delete(uri: string): void {
this._byUri.delete(uri);
}
}
159 changes: 83 additions & 76 deletions monaco-lsp-client/src/adapters/LspClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,90 +1,97 @@
import { IMessageTransport, TypedChannel } from "@hediet/json-rpc";
import { LspCompletionFeature } from "./languageFeatures/LspCompletionFeature";
import { LspHoverFeature } from "./languageFeatures/LspHoverFeature";
import { LspSignatureHelpFeature } from "./languageFeatures/LspSignatureHelpFeature";
import { LspDefinitionFeature } from "./languageFeatures/LspDefinitionFeature";
import { LspDeclarationFeature } from "./languageFeatures/LspDeclarationFeature";
import { LspTypeDefinitionFeature } from "./languageFeatures/LspTypeDefinitionFeature";
import { LspImplementationFeature } from "./languageFeatures/LspImplementationFeature";
import { LspReferencesFeature } from "./languageFeatures/LspReferencesFeature";
import { LspDocumentHighlightFeature } from "./languageFeatures/LspDocumentHighlightFeature";
import { LspDocumentSymbolFeature } from "./languageFeatures/LspDocumentSymbolFeature";
import { LspRenameFeature } from "./languageFeatures/LspRenameFeature";
import { LspCodeActionFeature } from "./languageFeatures/LspCodeActionFeature";
import { LspCodeLensFeature } from "./languageFeatures/LspCodeLensFeature";
import { LspDocumentLinkFeature } from "./languageFeatures/LspDocumentLinkFeature";
import { LspFormattingFeature } from "./languageFeatures/LspFormattingFeature";
import { LspRangeFormattingFeature } from "./languageFeatures/LspRangeFormattingFeature";
import { LspOnTypeFormattingFeature } from "./languageFeatures/LspOnTypeFormattingFeature";
import { LspFoldingRangeFeature } from "./languageFeatures/LspFoldingRangeFeature";
import { LspSelectionRangeFeature } from "./languageFeatures/LspSelectionRangeFeature";
import { LspInlayHintsFeature } from "./languageFeatures/LspInlayHintsFeature";
import { LspSemanticTokensFeature } from "./languageFeatures/LspSemanticTokensFeature";
import { LspDiagnosticsFeature } from "./languageFeatures/LspDiagnosticsFeature";
import { api } from "../../src/types";
import { LspConnection } from "./LspConnection";
import { IMessageTransport, TypedChannel } from '@hediet/json-rpc';
import { LspCompletionFeature } from './languageFeatures/LspCompletionFeature';
import { LspHoverFeature } from './languageFeatures/LspHoverFeature';
import { LspSignatureHelpFeature } from './languageFeatures/LspSignatureHelpFeature';
import { LspDefinitionFeature } from './languageFeatures/LspDefinitionFeature';
import { LspDeclarationFeature } from './languageFeatures/LspDeclarationFeature';
import { LspTypeDefinitionFeature } from './languageFeatures/LspTypeDefinitionFeature';
import { LspImplementationFeature } from './languageFeatures/LspImplementationFeature';
import { LspReferencesFeature } from './languageFeatures/LspReferencesFeature';
import { LspDocumentHighlightFeature } from './languageFeatures/LspDocumentHighlightFeature';
import { LspDocumentSymbolFeature } from './languageFeatures/LspDocumentSymbolFeature';
import { LspRenameFeature } from './languageFeatures/LspRenameFeature';
import { LspCodeActionFeature } from './languageFeatures/LspCodeActionFeature';
import { LspCodeLensFeature } from './languageFeatures/LspCodeLensFeature';
import { LspDocumentLinkFeature } from './languageFeatures/LspDocumentLinkFeature';
import { LspFormattingFeature } from './languageFeatures/LspFormattingFeature';
import { LspRangeFormattingFeature } from './languageFeatures/LspRangeFormattingFeature';
import { LspOnTypeFormattingFeature } from './languageFeatures/LspOnTypeFormattingFeature';
import { LspFoldingRangeFeature } from './languageFeatures/LspFoldingRangeFeature';
import { LspSelectionRangeFeature } from './languageFeatures/LspSelectionRangeFeature';
import { LspInlayHintsFeature } from './languageFeatures/LspInlayHintsFeature';
import { LspSemanticTokensFeature } from './languageFeatures/LspSemanticTokensFeature';
import { LspDiagnosticsFeature } from './languageFeatures/LspDiagnosticsFeature';
import { api } from '../../src/types';
import { DiagnosticsCache } from './DiagnosticsCache';
import { LspConnection } from './LspConnection';
import { LspCapabilitiesRegistry } from './LspCapabilitiesRegistry';
import { TextDocumentSynchronizer } from "./TextDocumentSynchronizer";
import { DisposableStore, IDisposable } from "../utils";
import { TextDocumentSynchronizer } from './TextDocumentSynchronizer';
import { DisposableStore, IDisposable, Disposable } from '../utils';

export class MonacoLspClient {
private _connection: LspConnection;
private readonly _capabilitiesRegistry: LspCapabilitiesRegistry;
private readonly _bridge: TextDocumentSynchronizer;
private _connection: LspConnection;
private readonly _capabilitiesRegistry: LspCapabilitiesRegistry;
private readonly _bridge: TextDocumentSynchronizer;

private _initPromise: Promise<void>;
private _initPromise: Promise<void>;

constructor(transport: IMessageTransport) {
const c = TypedChannel.fromTransport(transport);
const s = api.getServer(c, {});
c.startListen();
constructor(transport: IMessageTransport) {
const c = TypedChannel.fromTransport(transport);
const s = api.getServer(c, {});
c.startListen();

this._capabilitiesRegistry = new LspCapabilitiesRegistry(c);
this._bridge = new TextDocumentSynchronizer(s.server, this._capabilitiesRegistry);
this._capabilitiesRegistry = new LspCapabilitiesRegistry(c);
this._bridge = new TextDocumentSynchronizer(s.server, this._capabilitiesRegistry);

this._connection = new LspConnection(s.server, this._bridge, this._capabilitiesRegistry, c);
this.createFeatures();
this._connection = new LspConnection(
s.server,
this._bridge,
this._capabilitiesRegistry,
c,
new DiagnosticsCache()
);
this.createFeatures();

this._initPromise = this._init();
}
this._initPromise = this._init();
}

private async _init() {
const result = await this._connection.server.initialize({
processId: null,
capabilities: this._capabilitiesRegistry.getClientCapabilities(),
rootUri: null,
});
private async _init() {
const result = await this._connection.server.initialize({
processId: null,
capabilities: this._capabilitiesRegistry.getClientCapabilities(),
rootUri: null
});

this._connection.server.initialized({});
this._capabilitiesRegistry.setServerCapabilities(result.capabilities);
}
this._connection.server.initialized({});
this._capabilitiesRegistry.setServerCapabilities(result.capabilities);
}

protected createFeatures(): IDisposable {
const store = new DisposableStore();
protected createFeatures(): IDisposable {
const store = new DisposableStore();

store.add(new LspCompletionFeature(this._connection));
store.add(new LspHoverFeature(this._connection));
store.add(new LspSignatureHelpFeature(this._connection));
store.add(new LspDefinitionFeature(this._connection));
store.add(new LspDeclarationFeature(this._connection));
store.add(new LspTypeDefinitionFeature(this._connection));
store.add(new LspImplementationFeature(this._connection));
store.add(new LspReferencesFeature(this._connection));
store.add(new LspDocumentHighlightFeature(this._connection));
store.add(new LspDocumentSymbolFeature(this._connection));
store.add(new LspRenameFeature(this._connection));
store.add(new LspCodeActionFeature(this._connection));
store.add(new LspCodeLensFeature(this._connection));
store.add(new LspDocumentLinkFeature(this._connection));
store.add(new LspFormattingFeature(this._connection));
store.add(new LspRangeFormattingFeature(this._connection));
store.add(new LspOnTypeFormattingFeature(this._connection));
store.add(new LspFoldingRangeFeature(this._connection));
store.add(new LspSelectionRangeFeature(this._connection));
store.add(new LspInlayHintsFeature(this._connection));
store.add(new LspSemanticTokensFeature(this._connection));
store.add(new LspDiagnosticsFeature(this._connection));
store.add(new LspCompletionFeature(this._connection));
store.add(new LspHoverFeature(this._connection));
store.add(new LspSignatureHelpFeature(this._connection));
store.add(new LspDefinitionFeature(this._connection));
store.add(new LspDeclarationFeature(this._connection));
store.add(new LspTypeDefinitionFeature(this._connection));
store.add(new LspImplementationFeature(this._connection));
store.add(new LspReferencesFeature(this._connection));
store.add(new LspDocumentHighlightFeature(this._connection));
store.add(new LspDocumentSymbolFeature(this._connection));
store.add(new LspRenameFeature(this._connection));
store.add(new LspCodeActionFeature(this._connection));
store.add(new LspCodeLensFeature(this._connection));
store.add(new LspDocumentLinkFeature(this._connection));
store.add(new LspFormattingFeature(this._connection));
store.add(new LspRangeFormattingFeature(this._connection));
store.add(new LspOnTypeFormattingFeature(this._connection));
store.add(new LspFoldingRangeFeature(this._connection));
store.add(new LspSelectionRangeFeature(this._connection));
store.add(new LspInlayHintsFeature(this._connection));
store.add(new LspSemanticTokensFeature(this._connection));
store.add(new LspDiagnosticsFeature(this._connection));

return store;
}
return store;
}
}
2 changes: 2 additions & 0 deletions monaco-lsp-client/src/adapters/LspConnection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TypedChannel } from '@hediet/json-rpc';
import { api } from '../../src/types';
import { DiagnosticsCache } from './DiagnosticsCache';
import { ITextModelBridge } from './ITextModelBridge';
import { LspCapabilitiesRegistry } from './LspCapabilitiesRegistry';

Expand All @@ -9,5 +10,6 @@ export class LspConnection {
public readonly bridge: ITextModelBridge,
public readonly capabilities: LspCapabilitiesRegistry,
public readonly connection: TypedChannel,
public readonly diagnosticsCache: DiagnosticsCache,
) { }
}
76 changes: 68 additions & 8 deletions monaco-lsp-client/src/adapters/languageFeatures/LspCodeActionFeature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
import * as monaco from 'monaco-editor-core';
import { capabilities, CodeActionRegistrationOptions, Command, WorkspaceEdit, CodeAction } from '../../../src/types';
import {
capabilities,
CodeAction,
CodeActionRegistrationOptions,
Command,
Diagnostic,
DiagnosticTag,
WorkspaceEdit,
} from '../../../src/types';
import { Disposable } from '../../utils';
import { LspConnection } from '../LspConnection';
import { toMonacoLanguageSelector } from './common';
import { lspCodeActionKindToMonacoCodeActionKind, toMonacoCodeActionKind, toLspDiagnosticSeverity, toLspCodeActionTriggerKind, toMonacoCommand } from './common';
import {
lspCodeActionKindToMonacoCodeActionKind,
toLspCodeActionTriggerKind,
toLspDiagnosticSeverity,
toLspDiagnosticTag,
toMonacoCodeActionKind,
toMonacoCommand,
toMonacoLanguageSelector,
} from './common';

export class LspCodeActionFeature extends Disposable {
constructor(
Expand Down Expand Up @@ -73,16 +88,15 @@ class LspCodeActionProvider implements monaco.languages.CodeActionProvider {
token: monaco.CancellationToken
): Promise<monaco.languages.CodeActionList | null> {
const translated = this._client.bridge.translate(model, range.getStartPosition());
const cachedDiagnostics = this._client.diagnosticsCache.get(model.uri.toString());

const result = await this._client.server.textDocumentCodeAction({
textDocument: translated.textDocument,
range: this._client.bridge.translateRange(model, range),
context: {
diagnostics: context.markers.map(marker => ({
range: this._client.bridge.translateRange(model, monaco.Range.lift(marker)),
message: marker.message,
severity: toLspDiagnosticSeverity(marker.severity),
})),
diagnostics: context.markers.map(marker =>
toLspDiagnosticForCodeAction(marker, model, this._client, cachedDiagnostics),
),
triggerKind: toLspCodeActionTriggerKind(context.trigger),
},
});
Expand Down Expand Up @@ -123,6 +137,52 @@ class LspCodeActionProvider implements monaco.languages.CodeActionProvider {
}
}

/**
* Build the `Diagnostic` we send to the server as part of
* `textDocument/codeAction#context.diagnostics`.
*
* Servers (notably `ruff server`) require fields that don't survive the
* round-trip through Monaco's `IMarker` — specifically `data`, which the LSP
* spec mandates be round-tripped. We look up the original `Diagnostic` in
* `DiagnosticsCache` by URI + range + message and forward it verbatim. For
* markers that didn't come from this LSP session (e.g. set under a different
* owner by another extension), we synthesize a Diagnostic that still passes
* through `code` / `source` / `tags` / `relatedInformation` from `IMarker`.
*/
function toLspDiagnosticForCodeAction(
marker: monaco.editor.IMarkerData,
model: monaco.editor.ITextModel,
client: LspConnection,
cached: readonly Diagnostic[] | undefined,
): Diagnostic {
const lspRange = client.bridge.translateRange(model, monaco.Range.lift(marker));
const original = cached?.find(d =>
d.range.start.line === lspRange.start.line &&
d.range.start.character === lspRange.start.character &&
d.range.end.line === lspRange.end.line &&
d.range.end.character === lspRange.end.character &&
d.message === marker.message
);
if (original) {
return original;
}
const markerCode = marker.code;
const code =
markerCode === undefined
? undefined
: typeof markerCode === 'string'
? markerCode
: markerCode.value;
return {
range: lspRange,
message: marker.message,
severity: toLspDiagnosticSeverity(marker.severity),
code,
source: marker.source,
tags: marker.tags?.map(tag => toLspDiagnosticTag(tag)).filter((tag): tag is DiagnosticTag => tag !== undefined),
};
}

function toMonacoWorkspaceEdit(
edit: WorkspaceEdit,
client: LspConnection
Expand Down
7 changes: 7 additions & 0 deletions monaco-lsp-client/src/adapters/languageFeatures/LspDiagnosticsFeature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export class LspDiagnosticsFeature extends Disposable {
(params) => this._handlePublishDiagnostics(params)
));

this._register(monaco.editor.onWillDisposeModel(model => {
this._connection.diagnosticsCache.delete(model.uri.toString());
}));

this._register(this._connection.capabilities.registerCapabilityHandler(
capabilities.textDocumentDiagnostic,
true,
Expand Down Expand Up @@ -91,6 +95,7 @@ export class LspDiagnosticsFeature extends Disposable {
return;
}

this._connection.diagnosticsCache.set(model.uri.toString(), params.diagnostics);
const markers = params.diagnostics.map(diagnostic =>
toDiagnosticMarker(diagnostic)
);
Expand Down Expand Up @@ -163,6 +168,7 @@ class ModelDiagnosticProvider extends Disposable {
// Full diagnostic report
this._previousResultId = report.resultId;

this._connection.diagnosticsCache.set(this._model.uri.toString(), report.items);
const markers = report.items.map(diagnostic => toDiagnosticMarker(diagnostic));
monaco.editor.setModelMarkers(this._model, this._markerOwner, markers);

Expand All @@ -188,6 +194,7 @@ class ModelDiagnosticProvider extends Disposable {
}

if (report.kind === 'full') {
this._connection.diagnosticsCache.set(model.uri.toString(), report.items);
const markers = report.items.map((diagnostic: Diagnostic) => toDiagnosticMarker(diagnostic));
monaco.editor.setModelMarkers(model, this._markerOwner, markers);
}
Expand Down
9 changes: 9 additions & 0 deletions monaco-lsp-client/src/adapters/languageFeatures/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,15 @@ export function toMonacoDiagnosticTag(tag: DiagnosticTag): monaco.MarkerTag | un
return lspDiagnosticTagToMonacoMarkerTag.get(tag);
}

export const monacoMarkerTagToLspDiagnosticTag = new Map<monaco.MarkerTag, DiagnosticTag>([
[monaco.MarkerTag.Unnecessary, DiagnosticTag.Unnecessary],
[monaco.MarkerTag.Deprecated, DiagnosticTag.Deprecated],
]);

export function toLspDiagnosticTag(tag: monaco.MarkerTag): DiagnosticTag | undefined {
return monacoMarkerTagToLspDiagnosticTag.get(tag);
}

// ============================================================================
// Signature Help Trigger Kind
// ============================================================================
Expand Down