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

Jump to bottom

WIP: code actions for diagnostics #1100

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

WIP: code actions for diagnostics #1100

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
17 changes: 9 additions & 8 deletions packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
MockNotebookAdapter
} from '../../testutils';
import { foreignCodeExtractors } from '../../transclusions/ipython/extractors';
import { VirtualDocument } from '../../virtual/document';

import { diagnosticsPanel } from './diagnostics';
import { DiagnosticsFeature } from './feature';
Expand Down Expand Up @@ -114,7 +115,7 @@ describe('Diagnostics', () => {
uri: env.documentOptions.path,
diagnostics: diagnostics
},
env.adapter.virtualDocument!,
env.adapter.virtualDocument as VirtualDocument,
env.adapter
);
await framePromise();
Expand All @@ -138,7 +139,7 @@ describe('Diagnostics', () => {
uri: env.documentOptions.path,
diagnostics: diagnostics
},
env.adapter.virtualDocument!,
env.adapter.virtualDocument as VirtualDocument,
env.adapter
);
await framePromise();
Expand All @@ -163,7 +164,7 @@ describe('Diagnostics', () => {
uri: env.documentOptions.path,
diagnostics: diagnostics
},
env.adapter.virtualDocument!,
env.adapter.virtualDocument as VirtualDocument,
env.adapter
);
await framePromise();
Expand All @@ -188,7 +189,7 @@ describe('Diagnostics', () => {
uri: env.documentOptions.path,
diagnostics: diagnostics
},
env.adapter.virtualDocument!,
env.adapter.virtualDocument as VirtualDocument,
env.adapter
);
await framePromise();
Expand Down Expand Up @@ -297,7 +298,7 @@ describe('Diagnostics', () => {
}
]
},
env.adapter.virtualDocument!,
env.adapter.virtualDocument as VirtualDocument,
env.adapter
);
await framePromise();
Expand Down Expand Up @@ -359,7 +360,7 @@ describe('Diagnostics', () => {
} as lsProtocol.PublishDiagnosticsParams;

// test guards against wrongly propagated responses:
await feature.handleDiagnostic(response, document, env.adapter);
await feature.handleDiagnostic(response, document as VirtualDocument, env.adapter);

await env.adapter.updateDocuments();
await (env.adapter as MockNotebookAdapter).foreingDocumentOpened.promise;
Expand All @@ -381,7 +382,7 @@ describe('Diagnostics', () => {
response.uri = foreignDocument.uri;

// correct propagation
await feature.handleDiagnostic(response, foreignDocument, env.adapter);
await feature.handleDiagnostic(response, foreignDocument as VirtualDocument, env.adapter);
await framePromise();
await framePromise();

Expand Down Expand Up @@ -423,7 +424,7 @@ describe('Diagnostics', () => {
}
]
},
document!,
document as VirtualDocument,
env.adapter
);

Expand Down
111 changes: 99 additions & 12 deletions packages/jupyterlab-lsp/src/features/diagnostics/feature.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { linter, Diagnostic, lintGutter } from '@codemirror/lint';
import { linter, Diagnostic, Action, lintGutter } from '@codemirror/lint';
import { StateField, StateEffect, StateEffectType } from '@codemirror/state';
import { EditorView } from '@codemirror/view';
import { INotebookShell } from '@jupyter-notebook/application';
Expand All @@ -13,9 +13,9 @@ import {
WidgetLSPAdapter,
IEditorPosition,
IVirtualPosition,
ILSPConnection,
VirtualDocument
ILSPConnection
} from '@jupyterlab/lsp';
import { LSPConnection } from '@jupyterlab/lsp/lib/connection';
import { TranslationBundle } from '@jupyterlab/translation';
import { PromiseDelegate } from '@lumino/coreutils';
import * as lsProtocol from 'vscode-languageserver-protocol';
Expand All @@ -27,6 +27,9 @@ import { DiagnosticSeverity, DiagnosticTag } from '../../lsp';
import { PLUGIN_ID } from '../../tokens';
import { urisEqual } from '../../utils';
import { BrowserConsole } from '../../virtual/console';
import { VirtualDocument } from '../../virtual/document';
import { EditApplicator } from '../../edits';


import { diagnosticsPanel } from './diagnostics';
import { DiagnosticsDatabase } from './listing';
Expand All @@ -43,6 +46,49 @@ const SeverityMap: Record<
4: 'hint'
};


class DiagnosticView implements Diagnostic {
constructor(
protected options: Diagnostic,
protected actionsPromise: Promise<(lsProtocol.Command | lsProtocol.CodeAction)[] | null>,
protected applicator: EditApplicator
) {
// no-op
this.from = options.from;
this.to = options.to;
this.severity = options.severity;
this.markClass = options.markClass;
this.source = options.source;
this.message = options.message;
this._actions = [];
actionsPromise.then((response) => {
if (!response) {
return;
}
this._actions = response.filter(actionOrCommand => (actionOrCommand as lsProtocol.CodeAction).edit).map((action: lsProtocol.CodeAction) => {
return {
name: action.title,
apply: () => {
applicator.applyEdit(action.edit!);
console.log('applying');
}
}
});
});
}
private _actions: Action[]
from: number;
to: number;
severity: 'error' | 'warning' | 'info' | 'hint';
markClass?: string;
source?: string;
message: string;

get actions(): Action[] {
return this._actions;
}
}

export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
readonly id = DiagnosticsFeature.id;
readonly capabilities: lsProtocol.ClientCapabilities = {
Expand All @@ -51,6 +97,16 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
tagSupport: {
valueSet: [DiagnosticTag.Deprecated, DiagnosticTag.Unnecessary]
}
},
codeAction: {
dynamicRegistration: true,
/*
codeActionLiteralSupport: {
codeActionKind: {
valueSet: ['quickfix']
}
}
*/
}
}
};
Expand All @@ -73,12 +129,12 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
// TODO: unregister
connection.serverNotifications['textDocument/publishDiagnostics'].connect(
async (connection: ILSPConnection, diagnostics) => {
await this.handleDiagnostic(diagnostics, virtualDocument, adapter);
await this.handleDiagnostic(diagnostics, virtualDocument as VirtualDocument, adapter);
}
);
virtualDocument.foreignDocumentClosed.connect((document, context) => {
// TODO: check if we need to cast
this.clearDocumentDiagnostics(adapter, context.foreignDocument);
this.clearDocumentDiagnostics(adapter, context.foreignDocument as VirtualDocument);
});
});

Expand Down Expand Up @@ -206,15 +262,42 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
classNames.push('cm-lsp-diagnostic-tag-' + DiagnosticTag[tag]);
}

diagnostics.push({
const virtualDocument = adapter.virtualDocument!;
const connection = this.connectionManager.connections.get(
virtualDocument.uri
)!;
let actionsPromise: Promise<(lsProtocol.Command | lsProtocol.CodeAction)[] | null>;
const applicator = new EditApplicator(editorDiagnostic.document, adapter);

if (connection.serverCapabilities.codeActionProvider) {
const params: lsProtocol.CodeActionParams = {
textDocument: {
uri: editorDiagnostic.document.documentInfo.uri
},
context: {
diagnostics: [editorDiagnostic.diagnostic],
//only: ['quickfix']
},
range: editorDiagnostic.diagnostic.range
};
// @ts-ignore
const messageConnection = (connection as LSPConnection).connection;
actionsPromise = messageConnection.sendRequest(
'textDocument/codeAction',
params
);
} else {
actionsPromise = Promise.resolve(null);
}

diagnostics.push(new DiagnosticView({
from: Math.min(from, view.state.doc.length),
to: Math.min(to, view.state.doc.length),
severity: severity,
message: diagnostic.message,
source: diagnostic.source,
markClass: classNames.join(' ')
// TODO: actions
});
}, actionsPromise, applicator));
}
}
return diagnostics;
Expand Down Expand Up @@ -419,7 +502,8 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
setDiagnostics(
response: lsProtocol.PublishDiagnosticsParams,
document: VirtualDocument,
adapter: WidgetLSPAdapter<any>
adapter: WidgetLSPAdapter<any>,
force = false
) {
let diagnosticsList: IEditorDiagnostic[] = [];
// TODO: test case for severity class always being set, even if diagnostic has no severity
Expand Down Expand Up @@ -504,7 +588,8 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
range: {
start: startInEditor,
end: endInEditor
}
},
document
});
}
}
Expand Down Expand Up @@ -535,7 +620,8 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
!(
editorsWithDiagnostics.has(editor) ||
editorsWhichHadDiagnostics.has(editor)
)
) &&
!force
) {
continue;
}
Expand Down Expand Up @@ -585,7 +671,8 @@ export class DiagnosticsFeature extends Feature implements IDiagnosticsFeature {
this.setDiagnostics(
this._lastResponse,
this._lastDocument,
this._lastAdapter
this._lastAdapter,
true
);
}
diagnosticsPanel.update();
Expand Down
2 changes: 2 additions & 0 deletions packages/jupyterlab-lsp/src/features/diagnostics/tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Token } from '@lumino/coreutils';
import * as lsProtocol from 'vscode-languageserver-protocol';

import { PLUGIN_ID } from '../../tokens';
import { VirtualDocument } from '../../virtual/document';

/**
* Diagnostic which is localized at a specific editor (cell) within a notebook
Expand All @@ -15,6 +16,7 @@ export interface IEditorDiagnostic {
start: IEditorPosition;
end: IEditorPosition;
};
document: VirtualDocument
}

export interface IReadonlyDiagnosticsDatabase {
Expand Down
Loading