Skip to content

Fix code action diagnostics #352

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
kaisalmen merged 6 commits into from
May 20, 2022
Merged

Fix code action diagnostics #352

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2ac0d1
Remove ProtocolIMarkerData as it doesn't work anymore
CGNonofr May 19, 2022
c3e49c6
Provide the exact same diagnostic objects in code action request
CGNonofr May 19, 2022
42dc6ec
Add a FIXME comment for next version of monaco
CGNonofr May 19, 2022
3a99258
Fix wrong type
CGNonofr May 19, 2022
164f420
Extract constant
CGNonofr May 20, 2022
00cfc7f
Add some explanatory comments
CGNonofr May 20, 2022
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
39 changes: 12 additions & 27 deletions packages/client/src/monaco-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
Command, CodeLens, FormattingOptions, TextEdit, WorkspaceEdit, DocumentLinkParams, DocumentLink,
MarkedString, MarkupContent, ColorInformation, ColorPresentation, FoldingRange, FoldingRangeKind,
DiagnosticRelatedInformation, MarkupKind, SymbolKind, DocumentSymbol, CodeAction, SignatureHelpContext, SignatureHelpTriggerKind,
SemanticTokens, InsertTextMode, AnnotatedTextEdit, ChangeAnnotation, CodeDescription, InlayHint, InlayHintLabelPart
SemanticTokens, InsertTextMode, AnnotatedTextEdit, ChangeAnnotation, InlayHint, InlayHintLabelPart
} from './services';

export type RecursivePartial<T> = {
Expand Down Expand Up @@ -57,15 +57,6 @@ export namespace ProtocolCompletionItem {
return !!item && 'data' in item;
}
}
export interface ProtocolIMarkerData extends monaco.editor.IMarkerData {
data?: unknown;
codeDescription?: CodeDescription;
}
export namespace ProtocolIMarkerData {
export function is(item: any): item is ProtocolIMarkerData {
return !!item && 'data' in item;
}
}

export interface ProtocolCodeAction extends monaco.languages.CodeAction {
data?: unknown;
Expand Down Expand Up @@ -409,10 +400,6 @@ export class MonacoToProtocolConverter {
const range = this.asRange(new this._monaco.Range(marker.startLineNumber, marker.startColumn, marker.endLineNumber, marker.endColumn))
const severity = this.asDiagnosticSeverity(marker.severity);
const diag = Diagnostic.create(range, marker.message, severity, marker.code as string, marker.source);
if (ProtocolIMarkerData.is(marker)) {
diag.data = marker.data
diag.codeDescription = marker.codeDescription;
}
return diag
}

Expand All @@ -423,19 +410,19 @@ export class MonacoToProtocolConverter {
return markers.map(marker => this.asDiagnostic(marker));
}

asCodeActionContext(context: monaco.languages.CodeActionContext): CodeActionContext {
asCodeActionContext(context: monaco.languages.CodeActionContext, diagnostics: Diagnostic[]): CodeActionContext {
if (context === void 0 || context === null) {
return context;
}
const diagnostics = this.asDiagnostics(context.markers);
return CodeActionContext.create(diagnostics, Is.string(context.only) ? [context.only] : undefined);
// FIXME: CodeActionTriggerKind is missing
return CodeActionContext.create(diagnostics, Is.string(context.only) ? [context.only] : undefined, undefined);
Comment thread
kaisalmen marked this conversation as resolved.
}

asCodeActionParams(model: monaco.editor.IReadOnlyModel, range: monaco.Range, context: monaco.languages.CodeActionContext): CodeActionParams {
asCodeActionParams(model: monaco.editor.IReadOnlyModel, range: monaco.Range, context: monaco.languages.CodeActionContext, diagnostics: Diagnostic[]): CodeActionParams {
return {
textDocument: this.asTextDocumentIdentifier(model),
range: this.asRange(range),
context: this.asCodeActionContext(context)
context: this.asCodeActionContext(context, diagnostics)
}
}

Expand Down Expand Up @@ -989,7 +976,7 @@ export class ProtocolToMonacoConverter {
};
}

asSeverity(severity?: number): monaco.MarkerSeverity {
asSeverity(severity?: ls.DiagnosticSeverity): monaco.MarkerSeverity {
if (severity === 1) {
return this._monaco.MarkerSeverity.Error;
}
Expand All @@ -1003,16 +990,16 @@ export class ProtocolToMonacoConverter {
}

asDiagnostics(diagnostics: undefined): undefined;
asDiagnostics(diagnostics: Diagnostic[]): ProtocolIMarkerData[];
asDiagnostics(diagnostics: Diagnostic[] | undefined): ProtocolIMarkerData[] | undefined;
asDiagnostics(diagnostics: Diagnostic[] | undefined): ProtocolIMarkerData[] | undefined {
asDiagnostics(diagnostics: Diagnostic[]): monaco.editor.IMarkerData[];
asDiagnostics(diagnostics: Diagnostic[] | undefined): monaco.editor.IMarkerData[] | undefined;
asDiagnostics(diagnostics: Diagnostic[] | undefined): monaco.editor.IMarkerData[] | undefined {
if (!diagnostics) {
return undefined;
}
return diagnostics.map(diagnostic => this.asDiagnostic(diagnostic));
}

asDiagnostic(diagnostic: Diagnostic): ProtocolIMarkerData {
asDiagnostic(diagnostic: Diagnostic): monaco.editor.IMarkerData {
return {
code: typeof diagnostic.code === "number" ? diagnostic.code.toString() : diagnostic.code,
severity: this.asSeverity(diagnostic.severity),
Expand All @@ -1023,9 +1010,7 @@ export class ProtocolToMonacoConverter {
endLineNumber: diagnostic.range.end.line + 1,
endColumn: diagnostic.range.end.character + 1,
relatedInformation: this.asRelatedInformations(diagnostic.relatedInformation),
codeDescription: diagnostic.codeDescription,
tags: diagnostic.tags,
data: diagnostic.data
tags: diagnostic.tags
}
}

Expand Down
70 changes: 65 additions & 5 deletions packages/client/src/monaco-languages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
OnTypeFormattingEditProvider, RenameProvider,
DocumentFilter, DocumentSelector, DocumentLinkProvider, ImplementationProvider, TypeDefinitionProvider, DocumentColorProvider,
FoldingRangeProvider, SemanticTokensLegend,
DocumentSemanticTokensProvider, DocumentRangeSemanticTokensProvider, TextDocumentFilter, InlayHintsProvider
DocumentSemanticTokensProvider, DocumentRangeSemanticTokensProvider, TextDocumentFilter, InlayHintsProvider, Diagnostic
} from "./services";

import { MonacoDiagnosticCollection } from './monaco-diagnostic-collection';
Expand Down Expand Up @@ -46,20 +46,70 @@ export function testGlob(pattern: string, value: string): boolean {
return regExp.test(value);
}

export class MonacoLanguages implements Languages {
class ExtHostDiagnostics {
private static _idPool: number = 0;
private readonly _collections = new Map<string, DiagnosticCollection>();

constructor(
protected readonly _monaco: typeof monaco,
protected readonly p2m: ProtocolToMonacoConverter
) {
}

createDiagnosticCollection(name?: string): DiagnosticCollection {
let owner: string;
if (!name) {
name = '_generated_diagnostic_collection_name_#' + ExtHostDiagnostics._idPool++;
owner = name;
} else if (!this._collections.has(name)) {
owner = name;
} else {
do {
owner = name + ExtHostDiagnostics._idPool++;
} while (this._collections.has(owner));
Comment thread
kaisalmen marked this conversation as resolved.
}

const _this = this
return new class extends MonacoDiagnosticCollection {
constructor() {
super(_this._monaco, name || 'default', _this.p2m);
_this._collections.set(owner, this);
}
override dispose() {
super.dispose();
_this._collections.delete(owner);
}
};
}

getDiagnostics(resourceUri: string): Diagnostic[] {
let res: Diagnostic[] = [];
for (let collection of this._collections.values()) {
const diagnostics = collection.get(resourceUri)
if (diagnostics) {
res = res.concat(diagnostics);
}
}
return res;
}
}

export class MonacoLanguages implements Languages {
private readonly extHostDiagnostics: ExtHostDiagnostics;
constructor(
protected readonly _monaco: typeof monaco,
protected readonly p2m: ProtocolToMonacoConverter,
protected readonly m2p: MonacoToProtocolConverter
) { }
) {
this.extHostDiagnostics = new ExtHostDiagnostics(_monaco, p2m)
}

match(selector: DocumentSelector, document: DocumentIdentifier): boolean {
return this.matchModel(selector, MonacoModelIdentifier.fromDocument(this._monaco, document));
}

createDiagnosticCollection(name?: string): DiagnosticCollection {
return new MonacoDiagnosticCollection(this._monaco, name || 'default', this.p2m);
return this.extHostDiagnostics.createDiagnosticCollection(name)
}

registerCompletionItemProvider(selector: DocumentSelector, provider: CompletionItemProvider, ...triggerCharacters: string[]): Disposable {
Expand Down Expand Up @@ -190,7 +240,17 @@ export class MonacoLanguages implements Languages {
protected createCodeActionProvider(provider: CodeActionProvider): monaco.languages.CodeActionProvider {
return {
provideCodeActions: async (model, range, context, token) => {
const params = this.m2p.asCodeActionParams(model, range, context);
const allDiagnostics: Diagnostic[] = [];
for (const diagnostic of this.extHostDiagnostics.getDiagnostics(model.uri.toString())) {
if (range.intersectRanges(this.p2m.asRange(diagnostic.range))) {
const newLen = allDiagnostics.push(diagnostic);
if (newLen > 1000) {
Comment thread
kaisalmen marked this conversation as resolved.
Outdated
break;
}
}
}

const params = this.m2p.asCodeActionParams(model, range, context, allDiagnostics);
let result = await provider.provideCodeActions(params, token);
return result && this.p2m.asCodeActionList(result);
},
Expand Down
1 change: 1 addition & 0 deletions packages/client/src/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export namespace Services {

export interface DiagnosticCollection extends Disposable {
set(uri: string, diagnostics: Diagnostic[]): void;
get(uri: string): Diagnostic[];
}

export type ProviderResult<T> = T | undefined | null | PromiseLike<T | undefined | null>;
Expand Down
48 changes: 2 additions & 46 deletions packages/client/src/vscode-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
MessageActionItem
} from "./services";
import * as ServicesModule from "./services";
import { CancellationTokenSource, DiagnosticSeverity, DocumentSelector } from "vscode-languageserver-protocol";
import { CancellationTokenSource, DiagnosticSeverity, DocumentSelector, Diagnostic as ProtocolDiagnostics } from "vscode-languageserver-protocol";

export function createVSCodeApi(servicesProvider: Services.Provider): typeof vscode {
const unsupported = () => { throw new Error('unsupported') };
Expand Down Expand Up @@ -361,53 +361,9 @@ export function createVSCodeApi(servicesProvider: Services.Provider): typeof vsc
set(arg0: vscode.Uri | ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic> | undefined]>,
arg1?: ReadonlyArray<vscode.Diagnostic>): void {

function toInternalSeverity(severity: vscode.DiagnosticSeverity): DiagnosticSeverity {
// there is a typing mismatch, trying to use the proper switch
// mixes error with warnings etc...
// just cast for now, this as the correct behaviour
return severity as DiagnosticSeverity;
// we don't want to rely on the runtime vscode module here, so we use our version
// of the enum
/*
switch ((severity as unknown) as VsCodeDiagnosticSeverity)
{
case VsCodeDiagnosticSeverity.Warning:
return DiagnosticSeverity.Warning;
case VsCodeDiagnosticSeverity.Information:
return DiagnosticSeverity.Information;
case VsCodeDiagnosticSeverity.Hint:
return DiagnosticSeverity.Hint;
case VsCodeDiagnosticSeverity.Error:
return DiagnosticSeverity.Error;
}
return DiagnosticSeverity.Error;
// */
}

function toInternalCode(code: vscode.Diagnostic['code']): Diagnostic['code'] {
if (code != null && typeof code === 'object') {
return code.value as Diagnostic['code']
}
return code as Diagnostic['code'];
}

if (isVsCodeUri(arg0)) {
if (this.collection) {
if (arg1) {
this.collection.set(arg0.toString(), arg1.map(diag => {
return {
range: diag.range,
code: toInternalCode(diag.code),
source: diag.source,
message: diag.message,
tags: diag.tags,
relatedInformation: undefined,
severity: toInternalSeverity(diag.severity)
};
}));
} else {
this.collection.set(arg0.toString(), []);
}
this.collection.set(arg0.toString(), arg1 as unknown as ProtocolDiagnostics[] ?? []);
}
} else {
arg0.forEach(element => {
Expand Down