From 16e26f1ec4657f8eb7d56d29d588122231c28c1a Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 26 Dec 2021 21:05:14 +0000 Subject: [PATCH 1/8] Enable strict null checks --- packages/_example-extractor/src/api.spec.ts | 2 +- packages/code-jumpers/src/history.ts | 2 +- .../code-jumpers/src/jumpers/fileeditor.ts | 4 +- packages/code-jumpers/src/jumpers/jumper.ts | 8 +- packages/code-jumpers/src/jumpers/notebook.ts | 8 +- packages/code-jumpers/src/positions.ts | 7 +- packages/completion-theme/src/about.tsx | 2 +- packages/completion-theme/src/index.ts | 20 +-- .../jupyterlab-lsp/src/adapter_manager.ts | 4 +- .../jupyterlab-lsp/src/adapters/adapter.ts | 105 +++++++++----- .../src/adapters/file_editor/file_editor.ts | 13 +- .../src/adapters/notebook/notebook.ts | 94 ++++++++++--- .../src/command_manager.spec.ts | 22 +-- .../jupyterlab-lsp/src/command_manager.ts | 9 +- .../src/components/free_tooltip.ts | 16 ++- .../src/components/statusbar.tsx | 21 +-- .../jupyterlab-lsp/src/components/utils.tsx | 15 +- packages/jupyterlab-lsp/src/connection.ts | 41 ++++-- .../jupyterlab-lsp/src/connection_manager.ts | 12 +- .../src/editor_integration/cm_adapter.spec.ts | 2 +- .../src/editor_integration/codemirror.ts | 24 ++-- .../src/editor_integration/editor_adapter.ts | 10 +- .../src/editor_integration/testutils.ts | 18 +-- .../src/extractors/regexp.spec.ts | 24 ++-- .../jupyterlab-lsp/src/extractors/regexp.ts | 23 +++- .../src/extractors/testutils.ts | 4 +- .../jupyterlab-lsp/src/extractors/types.ts | 3 +- packages/jupyterlab-lsp/src/feature.ts | 6 +- .../src/features/completion/completion.ts | 30 ++-- .../features/completion/completion_handler.ts | 128 ++++++++++-------- .../src/features/completion/index.ts | 3 +- .../src/features/completion/item.ts | 29 ++-- .../src/features/completion/model.spec.ts | 8 +- .../src/features/completion/model.ts | 10 +- .../src/features/completion/renderer.ts | 26 ++-- .../features/diagnostics/diagnostics.spec.ts | 22 +-- .../src/features/diagnostics/diagnostics.ts | 60 ++++++-- .../src/features/diagnostics/listing.tsx | 56 ++++++-- .../jupyterlab-lsp/src/features/highlights.ts | 24 ++-- packages/jupyterlab-lsp/src/features/hover.ts | 54 ++++---- .../jupyterlab-lsp/src/features/jump_to.ts | 22 ++- .../jupyterlab-lsp/src/features/rename.ts | 29 ++-- .../src/features/signature.spec.ts | 10 +- .../jupyterlab-lsp/src/features/signature.ts | 17 ++- packages/jupyterlab-lsp/src/index.ts | 12 +- packages/jupyterlab-lsp/src/manager.ts | 8 +- packages/jupyterlab-lsp/src/overrides/maps.ts | 6 +- packages/jupyterlab-lsp/src/tokens.ts | 2 +- .../ipython-rpy2/overrides.spec.ts | 22 +-- .../transclusions/ipython/overrides.spec.ts | 28 ++-- packages/jupyterlab-lsp/src/utils.ts | 2 +- .../src/virtual/codemirror_editor.ts | 72 ++++++---- .../jupyterlab-lsp/src/virtual/console.ts | 4 +- .../src/virtual/document.spec.ts | 16 +-- .../jupyterlab-lsp/src/virtual/document.ts | 87 +++++++----- packages/jupyterlab-lsp/src/virtual/editor.ts | 4 +- .../src/test/connection.test.ts | 3 +- .../src/test/mock-connection.ts | 3 +- packages/lsp-ws-connection/src/types.ts | 1 + .../lsp-ws-connection/src/ws-connection.ts | 2 +- packages/tsconfigbase.json | 1 + 61 files changed, 809 insertions(+), 511 deletions(-) diff --git a/packages/_example-extractor/src/api.spec.ts b/packages/_example-extractor/src/api.spec.ts index 265af4e3b..388cc1eec 100644 --- a/packages/_example-extractor/src/api.spec.ts +++ b/packages/_example-extractor/src/api.spec.ts @@ -37,7 +37,7 @@ describe('The foo extractor', () => { test.each(Object.entries(FIXTURES))( '%s', (_: string, expected: IExtractedCode) => { - const extracted = extractor.extract_foreign_code(expected.host_code); + const extracted = extractor.extract_foreign_code(expected.host_code!); expect(extracted).to.have.length(1); expect(extracted[0]).to.deep.equal(expected); } diff --git a/packages/code-jumpers/src/history.ts b/packages/code-jumpers/src/history.ts index 25be9308d..058abaf1d 100644 --- a/packages/code-jumpers/src/history.ts +++ b/packages/code-jumpers/src/history.ts @@ -30,7 +30,7 @@ export class JumpHistory { this.jump_history.push(JSON.stringify(position)); } - recollect(): IGlobalPosition { + recollect(): IGlobalPosition | undefined { this.ensure_history_is_ready(); if (this.jump_history.length === 0) { return; diff --git a/packages/code-jumpers/src/jumpers/fileeditor.ts b/packages/code-jumpers/src/jumpers/fileeditor.ts index d6fc5e2b3..cf252b762 100644 --- a/packages/code-jumpers/src/jumpers/fileeditor.ts +++ b/packages/code-jumpers/src/jumpers/fileeditor.ts @@ -36,7 +36,7 @@ export class FileEditorJumper extends CodeJumper { // TODO: this is common // place cursor in the line with the definition - let position = this.editor.editor.getPositionAt(token.offset); + let position = this.editor.editor.getPositionAt(token.offset)!; this.editor.editor.setSelection({ start: position, end: position }); this.editor.editor.focus(); } @@ -58,7 +58,7 @@ export class FileEditorJumper extends CodeJumper { getCurrentPosition(): IGlobalPosition { let position = this.editor.editor.getCursorPosition(); return { - editor_index: null, + editor_index: 0, line: position.line, column: position.column, contents_path: this.editor.context.path, diff --git a/packages/code-jumpers/src/jumpers/jumper.ts b/packages/code-jumpers/src/jumpers/jumper.ts index 6cfe1f479..815decb38 100644 --- a/packages/code-jumpers/src/jumpers/jumper.ts +++ b/packages/code-jumpers/src/jumpers/jumper.ts @@ -69,12 +69,16 @@ export abstract class CodeJumper { let document_widget = this.document_manager.openOrReveal( position.contents_path ); + if (!document_widget) { + console.log('Widget failed to open for jump'); + return; + } let is_symlink = position.is_symlink; document_widget.revealed .then(() => { this.go_to_position( - document_widget, + document_widget!, position.contents_path.endsWith('.ipynb') ? 'notebook' : 'fileeditor', position.column, position.line, @@ -83,7 +87,7 @@ export abstract class CodeJumper { // protect external files from accidental edition if (is_symlink) { - this.protectFromAccidentalEditing(document_widget); + this.protectFromAccidentalEditing(document_widget!); } }) .catch(console.warn); diff --git a/packages/code-jumpers/src/jumpers/notebook.ts b/packages/code-jumpers/src/jumpers/notebook.ts index bdf80e1b0..ada4c488b 100644 --- a/packages/code-jumpers/src/jumpers/notebook.ts +++ b/packages/code-jumpers/src/jumpers/notebook.ts @@ -19,7 +19,7 @@ export class NotebookJumper extends CodeJumper { super(); this.widget = notebook_widget; this.notebook = notebook_widget.content; - this.history = new JumpHistory(this.notebook.model.modelDB); + this.history = new JumpHistory(this.notebook.model!.modelDB); this.document_manager = document_manager; } @@ -33,15 +33,15 @@ export class NotebookJumper extends CodeJumper { // Prevents event propagation issues setTimeout(() => { this.notebook.deselectAll(); - this.notebook.activeCellIndex = index; + this.notebook.activeCellIndex = index!; _ensureFocus(this.notebook); this.notebook.mode = 'edit'; // find out offset for the element - let activeEditor = this.notebook.activeCell.editor; + let activeEditor = this.notebook.activeCell!.editor; // place cursor in the line with the definition - let position = activeEditor.getPositionAt(token.offset); + let position = activeEditor.getPositionAt(token.offset)!; activeEditor.setSelection({ start: position, end: position }); }, 0); } diff --git a/packages/code-jumpers/src/positions.ts b/packages/code-jumpers/src/positions.ts index 502b2567e..cb52d2b09 100644 --- a/packages/code-jumpers/src/positions.ts +++ b/packages/code-jumpers/src/positions.ts @@ -6,14 +6,15 @@ export interface ILocalPosition { */ token: CodeEditor.IToken; /** - * Optional number identifying the cell in a notebook + * Optional number identifying the cell in a notebook. + * 0 in widgets with single editor */ - index?: number; + index: number; } export interface IGlobalPosition { /** - * In notebooks, the index of the target editor + * In notebooks, the index of the target editor; 0 in widgets with single editor. */ editor_index: number; diff --git a/packages/completion-theme/src/about.tsx b/packages/completion-theme/src/about.tsx index 4d68cae2a..d289f3048 100644 --- a/packages/completion-theme/src/about.tsx +++ b/packages/completion-theme/src/about.tsx @@ -69,7 +69,7 @@ export function render_themes_list( trans: TranslationBundle, props: { themes: ICompletionTheme[]; - current: ICompletionTheme; + current: ICompletionTheme | null; get_set: IconSetGetter; } ): React.ReactElement { diff --git a/packages/completion-theme/src/index.ts b/packages/completion-theme/src/index.ts index 2505525d8..b671209cc 100644 --- a/packages/completion-theme/src/index.ts +++ b/packages/completion-theme/src/index.ts @@ -23,7 +23,7 @@ import { export class CompletionThemeManager implements ILSPCompletionThemeManager { protected current_icons: Map; protected themes: Map; - private current_theme_id: string; + private current_theme_id: string | null = null; private icons_cache: Map; private icon_overrides: Map; private trans: TranslationBundle; @@ -50,17 +50,17 @@ export class CompletionThemeManager implements ILSPCompletionThemeManager { const dark_mode_and_dark_supported = !this.is_theme_light() && typeof icons_sets.dark !== 'undefined'; const set: ICompletionIconSet = dark_mode_and_dark_supported - ? icons_sets.dark + ? icons_sets.dark! : icons_sets.light; const icons: Map = new Map(); - let options = this.current_theme.icons.options || {}; + let options = this.current_theme?.icons?.options || {}; const mode = this.is_theme_light() ? 'light' : 'dark'; for (let [completion_kind, svg] of Object.entries(set)) { let name = 'lsp:' + theme.id + '-' + completion_kind.toLowerCase() + '-' + mode; let icon: LabIcon; if (this.icons_cache.has(name)) { - icon = this.icons_cache.get(name); + icon = this.icons_cache.get(name)!; } else { icon = new LabIcon({ name: name, @@ -83,19 +83,19 @@ export class CompletionThemeManager implements ILSPCompletionThemeManager { this.current_icons = this.get_iconset(this.current_theme); } - get_icon(type: string): LabIcon { + get_icon(type: string): LabIcon | null { if (this.current_theme === null) { return null; } if (type) { if (this.icon_overrides.has(type.toLowerCase())) { - type = this.icon_overrides.get(type.toLowerCase()); + type = this.icon_overrides.get(type.toLowerCase())!; } type = type.substring(0, 1).toUpperCase() + type.substring(1).toLowerCase(); } if (this.current_icons.has(type)) { - return this.current_icons.get(type); + return this.current_icons.get(type)!; } if (type === KernelKind) { @@ -112,7 +112,7 @@ export class CompletionThemeManager implements ILSPCompletionThemeManager { if (this.current_theme_id) { document.body.classList.remove(this.current_theme_class); } - if (!this.themes.has(id)) { + if (id && !this.themes.has(id)) { console.warn( `[LSP][Completer] Icons theme ${id} cannot be set yet (it may be loaded later).` ); @@ -123,8 +123,8 @@ export class CompletionThemeManager implements ILSPCompletionThemeManager { } protected get current_theme(): ICompletionTheme | null { - if (this.themes.has(this.current_theme_id)) { - return this.themes.get(this.current_theme_id); + if (this.current_theme_id && this.themes.has(this.current_theme_id)) { + return this.themes.get(this.current_theme_id)!; } return null; } diff --git a/packages/jupyterlab-lsp/src/adapter_manager.ts b/packages/jupyterlab-lsp/src/adapter_manager.ts index a2c7fdd03..33ea6467d 100644 --- a/packages/jupyterlab-lsp/src/adapter_manager.ts +++ b/packages/jupyterlab-lsp/src/adapter_manager.ts @@ -132,9 +132,9 @@ export class WidgetAdapterManager implements ILSPAdapterManager { this.refreshAdapterFromCurrentWidget(); } - isAnyActive() { + isAnyActive(): boolean { return ( - this.shell.currentWidget && + this.shell.currentWidget !== null && this.adapterTypes.some(type => type.tracker.currentWidget) && this.adapterTypes.some( type => type.tracker.currentWidget == this.shell.currentWidget diff --git a/packages/jupyterlab-lsp/src/adapters/adapter.ts b/packages/jupyterlab-lsp/src/adapters/adapter.ts index e02b655fe..ecd5558d5 100644 --- a/packages/jupyterlab-lsp/src/adapters/adapter.ts +++ b/packages/jupyterlab-lsp/src/adapters/adapter.ts @@ -32,7 +32,7 @@ export class StatusMessage { */ message: string; changed: Signal; - private timer: number; + private timer: number | null; constructor() { this.message = ''; @@ -183,11 +183,11 @@ export abstract class WidgetAdapter { this.disconnect(); // just to be sure - this.virtual_editor = null; - this.app = null; - this.widget = null; - this.connection_manager = null; - this.widget = null; + this.virtual_editor = null as any; + this.app = null as any; + this.widget = null as any; + this.connection_manager = null as any; + this.widget = null as any; this.isDisposed = true; } @@ -222,7 +222,7 @@ export abstract class WidgetAdapter { } } - abstract get language_file_extension(): string; + abstract get language_file_extension(): string | undefined; disconnect() { this.connection_manager.unregister_document( @@ -297,7 +297,7 @@ export abstract class WidgetAdapter { for (let virtual_document of documents_to_save) { let connection = this.connection_manager.connections.get( virtual_document.uri - ); + )!; this.console.log( 'Sending save notification for', virtual_document.uri, @@ -312,7 +312,7 @@ export abstract class WidgetAdapter { } } - abstract activeEditor: CodeEditor.IEditor; + abstract activeEditor: CodeEditor.IEditor | undefined; abstract get editors(): CodeEditor.IEditor[]; @@ -345,7 +345,13 @@ export abstract class WidgetAdapter { this.adapterConnected.emit(data); this.isConnected = true; - await this.update_documents().then(() => { + const promise = this.update_documents(); + + if (!promise) { + this.console.warn('Could not update documents'); + return; + } + await promise.then(() => { // refresh the document on the LSP server this.document_changed(virtual_document, virtual_document, true); @@ -359,7 +365,18 @@ export abstract class WidgetAdapter { // Note: the logger extension behaves badly with non-default names // as it changes the source to the active file afterwards anyways const loggerSourceName = virtual_document.uri; - const logger = this.extension.user_console.getLogger(loggerSourceName); + let log: (text: string) => void; + if (this.extension.user_console) { + const logger = this.extension.user_console.getLogger(loggerSourceName); + log = (text: string) => { + logger.log({ + type: 'text', + data: text + } as ILogPayload); + }; + } else { + log = (text: string) => console.log(text); + } data.connection.serverNotifications['$/logTrace'].connect( (connection, message) => { @@ -379,10 +396,7 @@ export abstract class WidgetAdapter { virtual_document.uri, message ); - logger.log({ - type: 'text', - data: connection.serverIdentifier + ': ' + message.message - } as ILogPayload); + log(connection.serverIdentifier + ': ' + message.message); } ); @@ -408,11 +422,13 @@ export abstract class WidgetAdapter { params ); const actionItems = params.actions; - const buttons = actionItems.map(action => { - return createButton({ - label: action.title - }); - }); + const buttons = actionItems + ? actionItems.map(action => { + return createButton({ + label: action.title + }); + }) + : [createButton({ label: this.trans.__('Dismiss') })]; const result = await showDialog({ title: this.trans.__('Message from ') + data.connection.serverIdentifier, @@ -421,9 +437,12 @@ export abstract class WidgetAdapter { }); const choice = buttons.indexOf(result.button); if (choice === -1) { - return; + return null; } - return actionItems[choice]; + if (actionItems) { + return actionItems[choice]; + } + return null; } ); } @@ -468,7 +487,7 @@ export abstract class WidgetAdapter { private create_virtual_editor( options: IVirtualEditor.IOptions - ): IVirtualEditor { + ): IVirtualEditor | null { let editorType = this.extension.editor_type_manager.findBestImplementation( this.editors ); @@ -547,7 +566,7 @@ export abstract class WidgetAdapter { let connection = this.connection_manager.connections.get( virtual_document.uri ); - let adapter = this.adapters.get(virtual_document.id_path); + let adapter = this.adapters.get(virtual_document.id_path)!; if (!connection?.isReady) { this.console.log('Skipping document update signal: connection not ready'); @@ -588,7 +607,7 @@ export abstract class WidgetAdapter { connect_adapter( virtual_document: VirtualDocument, connection: LSPConnection, - features: IFeature[] = null + features: IFeature[] | null = null ): EditorAdapter { let adapter = this.create_adapter(virtual_document, connection, features); this.adapters.set(virtual_document.id_path, adapter); @@ -621,12 +640,14 @@ export abstract class WidgetAdapter { let connection = await this.connection_manager.connect(options); - await this.on_connected({ virtual_document, connection }); + if (connection) { + await this.on_connected({ virtual_document, connection }); - return { - connection, - virtual_document - }; + return { + connection, + virtual_document + }; + } } /** @@ -649,7 +670,7 @@ export abstract class WidgetAdapter { private create_adapter( virtual_document: VirtualDocument, connection: LSPConnection, - features: IFeature[] = null + features: IFeature[] | null = null ): EditorAdapter> { let adapter_features = new Array< IFeatureEditorIntegration> @@ -661,14 +682,14 @@ export abstract class WidgetAdapter { for (let feature of features) { let featureEditorIntegrationConstructor = - feature.editorIntegrationFactory.get(this.virtual_editor.editor_name); + feature.editorIntegrationFactory.get(this.virtual_editor.editor_name)!; let integration = new featureEditorIntegrationConstructor({ feature: feature, virtual_editor: this.virtual_editor, virtual_document: virtual_document, connection: connection, status_message: this.status_message, - settings: feature.settings, + settings: feature.settings!, adapter: this, trans: this.trans }); @@ -689,17 +710,26 @@ export abstract class WidgetAdapter { private async onContentChanged(_slot: any) { // update the virtual documents (sending the updates to LSP is out of scope here) - this.update_finished = this.update_documents().catch(this.console.warn); + const promise = this.update_documents(); + if (!promise) { + this.console.warn('Could not update documents'); + return; + } + this.update_finished = promise.catch(this.console.warn); await this.update_finished; } - get_position_from_context_menu(): IRootPosition { + get_position_from_context_menu(): IRootPosition | null { // Note: could also try using this.app.contextMenu.menu.contentNode position. // Note: could add a guard on this.app.contextMenu.menu.isAttached // get the first node as it gives the most accurate approximation let leaf_node = this.app.contextMenuHitTest(() => true); + if (!leaf_node) { + return null; + } + let { left, top } = leaf_node.getBoundingClientRect(); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -735,7 +765,7 @@ export abstract class WidgetAdapter { connection: this.connection_manager.connections.get(document.uri), virtual_position, root_position, - features: this.get_features(document), + features: this.get_features(document)!, editor: this.virtual_editor, app: this.app, adapter: this @@ -744,6 +774,9 @@ export abstract class WidgetAdapter { get_context_from_context_menu(): ICommandContext | null { let root_position = this.get_position_from_context_menu(); + if (!root_position) { + return null; + } return this.get_context(root_position); } diff --git a/packages/jupyterlab-lsp/src/adapters/file_editor/file_editor.ts b/packages/jupyterlab-lsp/src/adapters/file_editor/file_editor.ts index a30c9f076..b90396ead 100644 --- a/packages/jupyterlab-lsp/src/adapters/file_editor/file_editor.ts +++ b/packages/jupyterlab-lsp/src/adapters/file_editor/file_editor.ts @@ -37,9 +37,8 @@ export class FileEditorAdapter extends WidgetAdapter< // registry (and this is arguably easier to extend), so let's check it // just in case; this is also how the "Klingon" language for testing // gets registered, so we need it for tests too. - let fileType = this.extension.app.docRegistry.getFileTypeForModel( - this.editor.context.contentsModel - ); + let fileType = + this.extension.app.docRegistry.getFileTypeForModel(contentsModel); return fileType.mimeTypes[0]; } else { // "text/plain" this is @@ -105,11 +104,15 @@ export class FileEditorAdapter extends WidgetAdapter< return this.widget.context.path; } - context_from_active_document(): ICommandContext { + context_from_active_document(): ICommandContext | null { + // short circuit if disposed + if (!this.virtual_editor || !this.get_context) { + return null; + } let editor = this.widget.content.editor; let ce_cursor = editor.getCursorPosition(); let root_position = PositionConverter.ce_to_cm(ce_cursor) as IRootPosition; - return this?.get_context(root_position); + return this.get_context(root_position); } get_editor_index_at(position: IVirtualPosition): number { diff --git a/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts b/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts index cc8c07ef9..c91655070 100644 --- a/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts +++ b/packages/jupyterlab-lsp/src/adapters/notebook/notebook.ts @@ -39,9 +39,16 @@ export class NotebookAdapter extends WidgetAdapter { } private async update_language_info() { - this._language_info = ( - await this.widget.context.sessionContext.session.kernel.info - ).language_info; + const language_info = ( + await this.widget.context.sessionContext?.session?.kernel?.info + )?.language_info; + if (language_info) { + this._language_info = language_info; + } else { + throw new Error( + 'Language info update failed (no session, kernel, or info available)' + ); + } } async on_kernel_changed( @@ -49,16 +56,30 @@ export class NotebookAdapter extends WidgetAdapter { change: Session.ISessionConnection.IKernelChangedArgs ) { if (!change.newValue) { - this.console.log('kernel was shut down'); + this.console.log('Kernel was shut down'); return; } try { - await this.update_language_info(); - this.console.log( - `Changed to ${this._language_info.name} kernel, reconnecting` - ); + // note: we need to wait until ready before updating language info + this.console.log('Changed kernel, will try to reconnect'); + const old_language_info = this._language_info; await until_ready(this.is_ready, -1); - this.reload_connection(); + await this.update_language_info(); + const new_language_info = this._language_info; + if ( + old_language_info?.name != new_language_info.name || + old_language_info?.mimetype != new_language_info?.mimetype || + old_language_info?.file_extension != new_language_info?.file_extension + ) { + this.console.log( + `Changed to ${this._language_info.name} kernel, reconnecting` + ); + this.reload_connection(); + } else { + this.console.log( + 'Keeping old LSP connection as the new kernel uses the same langauge' + ); + } } catch (err) { this.console.warn(err); // try to reconnect anyway @@ -106,17 +127,17 @@ export class NotebookAdapter extends WidgetAdapter { get mime_type(): string { let language_metadata = this.language_info(); - if (!language_metadata) { + if (!language_metadata || !language_metadata.mimetype) { // fallback to the code cell mime type if no kernel in use return this.widget.content.codeMimetype; } return language_metadata.mimetype; } - get language_file_extension(): string { + get language_file_extension(): string | undefined { let language_metadata = this.language_info(); - if (!language_metadata) { - return null; + if (!language_metadata || !language_metadata.file_extension) { + return; } return language_metadata.file_extension.replace('.', ''); } @@ -146,7 +167,7 @@ export class NotebookAdapter extends WidgetAdapter { ); this.widget.content.activeCellChanged.connect(this.activeCellChanged, this); - this.widget.model.cells.changed.connect(this.handle_cell_change, this); + this._connectModelSignals(this.widget); this.editor.modelChanged.connect(notebook => { // note: this should not usually happen; // there is no default action that would trigger this, @@ -155,10 +176,20 @@ export class NotebookAdapter extends WidgetAdapter { this.console.warn( 'Model changed, connecting cell change handler; this is not something we were expecting' ); - notebook.model.cells.changed.connect(this.handle_cell_change, this); + this._connectModelSignals(notebook); }); } + private _connectModelSignals(notebook: NotebookPanel | Notebook) { + if (notebook.model === null) { + this.console.warn( + `Model is missing for notebook ${notebook}, cannot connet cell changed signal!` + ); + } else { + notebook.model.cells.changed.connect(this.handle_cell_change, this); + } + } + async handle_cell_change( cells: IObservableUndoableList, change: IObservableList.IChangedArgs @@ -217,6 +248,12 @@ export class NotebookAdapter extends WidgetAdapter { let cellWidget = this.widget.content.widgets.find( cell => cell.model.id === cellModel.id ); + if (!cellWidget) { + this.console.warn( + `Widget for removed cell with ID: ${cellModel.id} not found!` + ); + continue; + } this.known_editors_ids.delete(cellWidget.editor.uuid); // for practical purposes this editor got removed from our consideration; @@ -232,6 +269,12 @@ export class NotebookAdapter extends WidgetAdapter { let cellWidget = this.widget.content.widgets.find( cell => cell.model.id === cellModel.id ); + if (!cellWidget) { + this.console.warn( + `Widget for added cell with ID: ${cellModel.id} not found!` + ); + continue; + } this.known_editors_ids.add(cellWidget.editor.uuid); this.editorAdded.emit({ @@ -242,7 +285,7 @@ export class NotebookAdapter extends WidgetAdapter { get editors(): CodeEditor.IEditor[] { if (this.isDisposed) { - return; + return []; } let notebook = this.widget.content; @@ -277,7 +320,7 @@ export class NotebookAdapter extends WidgetAdapter { } get activeEditor() { - return this.widget.content.activeCell.editor; + return this.widget.content.activeCell?.editor; } private activeCellChanged(notebook: Notebook, cell: Cell) { @@ -297,6 +340,13 @@ export class NotebookAdapter extends WidgetAdapter { context_from_active_document(): ICommandContext | null { let cell = this.widget.content.activeCell; + if (cell === null) { + return null; + } + // short circuit if disposed + if (!this.virtual_editor || !this.get_context) { + return null; + } if (cell.model.type !== this.type) { // context will be sought on all cells to verify if the context menu should be visible, // thus it is ok to just return null; it seems to stem from the implementation detail @@ -308,7 +358,7 @@ export class NotebookAdapter extends WidgetAdapter { let ce_cursor = editor.getCursorPosition(); let cm_cursor = PositionConverter.ce_to_cm(ce_cursor) as IEditorPosition; - let virtual_editor = this?.virtual_editor; + let virtual_editor = this.virtual_editor; if (virtual_editor == null) { return null; @@ -324,7 +374,7 @@ export class NotebookAdapter extends WidgetAdapter { return null; } - return this?.get_context(root_position); + return this.get_context(root_position); } get_editor_index_at(position: IVirtualPosition): number { @@ -336,7 +386,7 @@ export class NotebookAdapter extends WidgetAdapter { } get_editor_index(ce_editor: CodeEditor.IEditor): number { - let cell = this.ce_editor_to_cell.get(ce_editor); + let cell = this.ce_editor_to_cell.get(ce_editor)!; let notebook = this.widget.content; return notebook.widgets.findIndex(other_cell => { return cell === other_cell; @@ -344,13 +394,13 @@ export class NotebookAdapter extends WidgetAdapter { } get_editor_wrapper(ce_editor: CodeEditor.IEditor): HTMLElement { - let cell = this.ce_editor_to_cell.get(ce_editor); + let cell = this.ce_editor_to_cell.get(ce_editor)!; return cell.node; } private get_cell_at(pos: IVirtualPosition): Cell { let ce_editor = this.virtual_editor.virtual_document.get_editor_at_virtual_line(pos); - return this.ce_editor_to_cell.get(ce_editor); + return this.ce_editor_to_cell.get(ce_editor)!; } } diff --git a/packages/jupyterlab-lsp/src/command_manager.spec.ts b/packages/jupyterlab-lsp/src/command_manager.spec.ts index 77dd3890d..1cb46edd1 100644 --- a/packages/jupyterlab-lsp/src/command_manager.spec.ts +++ b/packages/jupyterlab-lsp/src/command_manager.spec.ts @@ -14,12 +14,12 @@ describe('ContextMenuCommandManager', () => { class ManagerImplementation extends ContextCommandManager { constructor(options: IContextMenuOptions) { super({ - app: null, - adapter_manager: null, - palette: null, - tracker: null, - suffix: null, - entry_point: null, + app: null as any, + adapter_manager: null as any, + palette: null as any, + tracker: null as any, + suffix: null as any, + entry_point: null as any, console: new BrowserConsole(), ...options }); @@ -33,7 +33,7 @@ describe('ContextMenuCommandManager', () => { selector: string; get current_adapter(): WidgetAdapter { - return undefined; + return undefined as any; } } let manager: ManagerImplementation; @@ -52,7 +52,7 @@ describe('ContextMenuCommandManager', () => { describe('#get_rank()', () => { it('uses in-group (relative) positioning by default', () => { manager = new ManagerImplementation({ - selector: null, + selector: 'body', rank_group: 0, rank_group_size: 5 }); @@ -63,7 +63,7 @@ describe('ContextMenuCommandManager', () => { expect(rank).to.equal(1 / 5); manager = new ManagerImplementation({ - selector: null, + selector: 'body', rank_group: 1, rank_group_size: 5 }); @@ -72,7 +72,7 @@ describe('ContextMenuCommandManager', () => { expect(rank).to.equal(1 + 1 / 5); manager = new ManagerImplementation({ - selector: null + selector: 'body' }); rank = manager.get_rank(base_command); expect(rank).to.equal(Infinity); @@ -81,7 +81,7 @@ describe('ContextMenuCommandManager', () => { it('respects is_rank_relative value', () => { manager = new ManagerImplementation({ - selector: null, + selector: 'body', rank_group: 0, rank_group_size: 5 }); diff --git a/packages/jupyterlab-lsp/src/command_manager.ts b/packages/jupyterlab-lsp/src/command_manager.ts index 3efc0f438..b758a7e96 100644 --- a/packages/jupyterlab-lsp/src/command_manager.ts +++ b/packages/jupyterlab-lsp/src/command_manager.ts @@ -138,7 +138,7 @@ export class ContextCommandManager extends LSPCommandManager { this.app.contextMenu.addItem({ type: 'separator', selector: this.selector, - rank: this.rank_group + position_in_group + rank: (this.rank_group ?? 0) + position_in_group }); } @@ -171,7 +171,7 @@ export class ContextCommandManager extends LSPCommandManager { } get_context(): ICommandContext | null { - let context: ICommandContext = null; + let context: ICommandContext | null = null; if (this.is_context_menu_open) { try { context = this.current_adapter.get_context_from_context_menu(); @@ -195,7 +195,8 @@ export class ContextCommandManager extends LSPCommandManager { return ( context != null && this.current_adapter && - context.connection?.isReady && + context.connection != null && + context.connection.isReady && command.is_enabled(context) ); } catch (e) { @@ -225,7 +226,7 @@ export class ContextCommandManager extends LSPCommandManager { export interface ICommandContext { app: JupyterFrontEnd; document: VirtualDocument; - connection: LSPConnection; + connection?: LSPConnection; virtual_position: IVirtualPosition; root_position: IRootPosition; features: Map>; diff --git a/packages/jupyterlab-lsp/src/components/free_tooltip.ts b/packages/jupyterlab-lsp/src/components/free_tooltip.ts index 555d375b9..2d466e1e8 100644 --- a/packages/jupyterlab-lsp/src/components/free_tooltip.ts +++ b/packages/jupyterlab-lsp/src/components/free_tooltip.ts @@ -107,7 +107,7 @@ export class FreeTooltip extends Tooltip { return; } - let position: CodeEditor.IPosition; + let position: CodeEditor.IPosition | undefined; switch (this.options.alignment) { case 'start': { @@ -165,7 +165,7 @@ export namespace EditorTooltip { } export class EditorTooltipManager { - private currentTooltip: FreeTooltip = null; + private currentTooltip: FreeTooltip | null = null; private currentOptions: EditorTooltip.IOptions | null; constructor(private rendermime_registry: IRenderMimeRegistry) {} @@ -175,7 +175,7 @@ export class EditorTooltipManager { this.currentOptions = options; let { markup, position, adapter } = options; let widget = adapter.widget; - const bundle = + const bundle: { 'text/plain': string } | { 'text/markdown': string } = markup.kind === 'plaintext' ? { 'text/plain': markup.value } : { 'text/markdown': markup.value }; @@ -188,14 +188,16 @@ export class EditorTooltipManager { position: PositionConverter.cm_to_ce(position) }); tooltip.addClass(CLASS_NAME); - tooltip.addClass(options.className); + if (options.className) { + tooltip.addClass(options.className); + } Widget.attach(tooltip, document.body); this.currentTooltip = tooltip; return tooltip; } get position(): IEditorPosition { - return this.currentOptions.position; + return this.currentOptions!.position; } isShown(id?: string): boolean { @@ -203,7 +205,7 @@ export class EditorTooltipManager { return false; } return ( - this.currentTooltip && + this.currentTooltip !== null && !this.currentTooltip.isDisposed && this.currentTooltip.isVisible ); @@ -212,7 +214,7 @@ export class EditorTooltipManager { remove() { if (this.currentTooltip !== null) { this.currentTooltip.dispose(); - this.currentTooltip = null; + this.currentTooltip = null as any; } } } diff --git a/packages/jupyterlab-lsp/src/components/statusbar.tsx b/packages/jupyterlab-lsp/src/components/statusbar.tsx index b25f8befa..2e29064ad 100644 --- a/packages/jupyterlab-lsp/src/components/statusbar.tsx +++ b/packages/jupyterlab-lsp/src/components/statusbar.tsx @@ -54,7 +54,7 @@ interface IServerStatusProps { } function ServerStatus(props: IServerStatusProps) { - let list = props.server.spec.languages.map((language, i) => ( + let list = props.server.spec.languages!.map((language, i) => (
  • {language}
    • )); return ( @@ -290,7 +290,7 @@ class LSPPopup extends VDomRenderer {
  • {this.model.trans.__(status)} @@ -399,7 +399,7 @@ const SELECTED_CLASS = 'jp-mod-selected'; * StatusBar item. */ export class LSPStatus extends VDomRenderer { - protected _popup: Popup = null; + protected _popup: Popup | null = null; private interactiveStateObserver: MutationObserver; private trans: TranslationBundle; /** @@ -471,7 +471,9 @@ export class LSPStatus extends VDomRenderer { className={'lsp-status-message'} source={model.short_message} /> - ) : null} + ) : ( + <> + )} ); @@ -597,7 +599,7 @@ export namespace LSPStatus { * A VDomModel for the LSP of current file editor/notebook. */ export class Model extends VDomModel { - server_extension_status: SCHEMA.ServersResponse = null; + server_extension_status: SCHEMA.ServersResponse | null = null; language_server_manager: ILanguageServerManager; trans: TranslationBundle; private _connection_manager: DocumentConnectionManager; @@ -625,7 +627,7 @@ export namespace LSPStatus { get supported_languages(): Set { const languages = new Set(); for (let server of this.available_servers.values()) { - for (let language of server.spec.languages) { + for (let language of server.spec.languages!) { languages.add(language.toLocaleLowerCase()); } } @@ -646,6 +648,7 @@ export namespace LSPStatus { return true; } } + return false; } get documents_by_server(): Map< @@ -673,19 +676,19 @@ export namespace LSPStatus { continue; } // For now only use the server with the highest priority - let server = this.language_server_manager.sessions.get(server_ids[0]); + let server = this.language_server_manager.sessions.get(server_ids[0])!; if (!data.has(server)) { data.set(server, new Map()); } - let documents_map = data.get(server); + let documents_map = data.get(server)!; if (!documents_map.has(language)) { documents_map.set(language, new Array()); } - let documents = documents_map.get(language); + let documents = documents_map.get(language)!; documents.push(document); } return data; diff --git a/packages/jupyterlab-lsp/src/components/utils.tsx b/packages/jupyterlab-lsp/src/components/utils.tsx index 7d960b0c9..f729cf7b1 100644 --- a/packages/jupyterlab-lsp/src/components/utils.tsx +++ b/packages/jupyterlab-lsp/src/components/utils.tsx @@ -19,6 +19,7 @@ export function getBreadcrumbs( let path = document.path; if ( !document.has_lsp_supported_file && + document.file_extension && path.endsWith(document.file_extension) ) { path = path.slice(0, -document.file_extension.length - 1); @@ -41,18 +42,18 @@ export function getBreadcrumbs( } try { if (adapter.has_multiple_editors) { - let first_line = document.virtual_lines.get(0); + let first_line = document.virtual_lines.get(0)!; let last_line = document.virtual_lines.get( document.last_virtual_line - 1 - ); + )!; let first_cell = adapter.get_editor_index(first_line.editor); let last_cell = adapter.get_editor_index(last_line.editor); let cell_locator = first_cell === last_cell - ? trans.__('cell %1', first_cell + 1) - : trans.__('cells: %1-%2', first_cell + 1, last_cell + 1); + ? trans!.__('cell %1', first_cell + 1) + : trans!.__('cells: %1-%2', first_cell + 1, last_cell + 1); return ( @@ -75,7 +76,7 @@ export function get_breadcrumbs( adapter: WidgetAdapter, collapse = true ) { - return getBreadcrumbs(document, adapter, null, collapse); + return getBreadcrumbs(document, adapter, undefined, collapse); } export function focus_on(node: HTMLElement) { @@ -92,7 +93,7 @@ export function DocumentLocator(props: { trans?: TranslationBundle; }) { let { document, adapter } = props; - let target: HTMLElement = null; + let target: HTMLElement | null = null; if (adapter.has_multiple_editors) { let first_line = document.virtual_lines.get(0); if (first_line) { @@ -105,7 +106,7 @@ export function DocumentLocator(props: { return (
    focus_on(target ? target : null)} + onClick={() => (target ? focus_on(target) : null)} > {breadcrumbs}
    diff --git a/packages/jupyterlab-lsp/src/connection.ts b/packages/jupyterlab-lsp/src/connection.ts index 9f55af3a6..0e56d72eb 100644 --- a/packages/jupyterlab-lsp/src/connection.ts +++ b/packages/jupyterlab-lsp/src/connection.ts @@ -213,10 +213,12 @@ class ServerRequestHandler< T extends keyof IServerRequestParams = keyof IServerRequestParams > implements IServerRequestHandler { - private _handler: ( - params: IServerRequestParams[T], - connection?: LSPConnection - ) => Promise; + private _handler: + | (( + params: IServerRequestParams[T], + connection?: LSPConnection + ) => Promise) + | null; constructor( protected connection: MessageConnection, @@ -228,13 +230,15 @@ class ServerRequestHandler< this._handler = null; } - private handle(request: IServerRequestParams[T]): Promise { + private handle( + request: IServerRequestParams[T] + ): Promise { this.emitter.log(MessageKind.server_requested, { method: this.method, message: request }); if (!this._handler) { - return; + return new Promise(() => undefined); } return this._handler(request, this.emitter).then(result => { this.emitter.log(MessageKind.response_for_server, { @@ -324,7 +328,7 @@ interface IMessageLog { export class LSPConnection extends LspWsConnection { protected documentsToOpen: IDocumentInfo[]; - public serverIdentifier: string; + public serverIdentifier?: string; public clientNotifications: ClientNotifications; public serverNotifications: ServerNotifications; @@ -375,7 +379,7 @@ export class LSPConnection extends LspWsConnection { constructor(options: ILSPOptions) { super(options); this.logAllCommunication = false; - this.serverIdentifier = options?.serverIdentifier; + this.serverIdentifier = options.serverIdentifier; this.console = options.console.scope(this.serverIdentifier + ' connection'); this.documentsToOpen = []; this.clientNotifications = @@ -400,7 +404,7 @@ export class LSPConnection extends LspWsConnection { this.afterInitialized(); super.onServerInitialized(params); while (this.documentsToOpen.length) { - this.sendOpen(this.documentsToOpen.pop()); + this.sendOpen(this.documentsToOpen.pop()!); } } @@ -443,12 +447,19 @@ export class LSPConnection extends LspWsConnection { params.registrations.forEach( (capabilityRegistration: lsp.Registration) => { try { - this.serverCapabilities = registerServerCapability( + const updatedCapabilities = registerServerCapability( this.serverCapabilities, capabilityRegistration ); + if (updatedCapabilities === null) { + this.console.error( + `Failed to register server capability: ${capabilityRegistration}` + ); + return; + } + this.serverCapabilities = updatedCapabilities; } catch (err) { - console.error(err); + this.console.error(err); } } ); @@ -512,9 +523,9 @@ export class LSPConnection extends LspWsConnection { documentInfo: IDocumentInfo, newName: string, emit = true - ): Promise { + ): Promise { if (!this.isReady || !this.isRenameSupported()) { - return; + return null; } const params: lsp.RenameParams = { @@ -608,6 +619,8 @@ export class LSPConnection extends LspWsConnection { * @deprecated The method should not be used in new code */ public isCompletionResolveProvider(): boolean { - return this.serverCapabilities?.completionProvider?.resolveProvider; + return ( + this.serverCapabilities?.completionProvider?.resolveProvider || false + ); } } diff --git a/packages/jupyterlab-lsp/src/connection_manager.ts b/packages/jupyterlab-lsp/src/connection_manager.ts index 237014bdb..1511dfe33 100644 --- a/packages/jupyterlab-lsp/src/connection_manager.ts +++ b/packages/jupyterlab-lsp/src/connection_manager.ts @@ -150,7 +150,7 @@ export class DocumentConnectionManager { // of connecting. const connection = await Private.connection( language, - language_server_id, + language_server_id!, uris, this.on_new_connection, this.console @@ -191,9 +191,11 @@ export class DocumentConnectionManager { if (!allServerSettings.hasOwnProperty(language_server_id)) { continue; } - const rawSettings = allServerSettings[language_server_id]; + const rawSettings = allServerSettings[language_server_id]!; - const parsedSettings = expandDottedPaths(rawSettings.serverSettings); + const parsedSettings = expandDottedPaths( + rawSettings.serverSettings || {} + ); const serverSettings: protocol.DidChangeConfigurationParams = { settings: parsedSettings @@ -267,7 +269,7 @@ export class DocumentConnectionManager { if (connection !== a_connection) { continue; } - callback(this.documents.get(virtual_document_uri)); + callback(this.documents.get(virtual_document_uri)!); } } @@ -496,7 +498,7 @@ namespace Private { onCreate(connection); } - connection = _connections.get(language_server_id); + connection = _connections.get(language_server_id)!; return connection; } diff --git a/packages/jupyterlab-lsp/src/editor_integration/cm_adapter.spec.ts b/packages/jupyterlab-lsp/src/editor_integration/cm_adapter.spec.ts index aac72db38..ab18a0ec0 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/cm_adapter.spec.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/cm_adapter.spec.ts @@ -17,7 +17,7 @@ describe('CodeMirrorAdapter', () => { it('updates on change', async () => { class UpdateReceivingFeature extends CodeMirrorIntegration { public received_update = false; - public last_change: CodeMirror.EditorChange = null; + public last_change: CodeMirror.EditorChange = null as any; public last_change_position: IRootPosition; afterChange(change: IEditorChange, root_position: IRootPosition): void { diff --git a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts index 931be167c..7743c8386 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts @@ -104,7 +104,7 @@ export abstract class CodeMirrorIntegration protected trans: TranslationBundle; - get settings(): IFeatureSettings { + get settings(): IFeatureSettings | undefined { return this.feature.settings; } @@ -176,12 +176,12 @@ export abstract class CodeMirrorIntegration this.transform_virtual_position_to_root_position(start); let ce_editor = this.virtual_editor.get_editor_at_root_position(start_in_root); - cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); + cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor)!; } return { - start: this.virtual_document.transform_virtual_to_editor(start), - end: this.virtual_document.transform_virtual_to_editor(end), + start: this.virtual_document.transform_virtual_to_editor(start)!, + end: this.virtual_document.transform_virtual_to_editor(end)!, editor: cm_editor }; } @@ -199,13 +199,13 @@ export abstract class CodeMirrorIntegration public transform_virtual_position_to_root_position( start: IVirtualPosition ): IRootPosition { - let ce_editor = this.virtual_document.virtual_lines.get(start.line).editor; + let ce_editor = this.virtual_document.virtual_lines.get(start.line)!.editor; let editor_position = this.virtual_document.transform_virtual_to_editor(start); return this.virtual_editor.transform_from_editor_to_root( ce_editor, - editor_position - ); + editor_position! + )!; } protected get_cm_editor(position: IRootPosition) { @@ -261,10 +261,10 @@ export abstract class CodeMirrorIntegration ? workspaceEdit.documentChanges.map( change => change as lsProtocol.TextDocumentEdit ) - : toDocumentChanges(workspaceEdit.changes); + : toDocumentChanges(workspaceEdit.changes!); let applied_changes = 0; - let edited_cells: number; - let is_whole_document_edit: boolean; + let edited_cells: number = 0; + let is_whole_document_edit: boolean = false; let errors: string[] = []; for (let change of changes) { @@ -310,7 +310,7 @@ export abstract class CodeMirrorIntegration // going over the edits in descending order of start points: let start_offsets = [...edits_by_offset.keys()].sort((a, b) => a - b); for (let start of start_offsets) { - let edit = edits_by_offset.get(start); + let edit = edits_by_offset.get(start)!; let prefix = value.slice(last_end, start); for (let i = 0; i < prefix.split('\n').length; i++) { let new_lines = old_to_new_line.get_or_create(current_old_line); @@ -379,7 +379,7 @@ export abstract class CodeMirrorIntegration newFragmentText = newFragmentText.slice(0, -1); } - let doc = this.virtual_editor.ce_editor_to_cm_editor.get(editor).getDoc(); + let doc = this.virtual_editor.ce_editor_to_cm_editor.get(editor)!.getDoc(); let raw_value = doc.getValue('\n'); // extract foreign documents and substitute magics, diff --git a/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts b/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts index ae6152ea0..213130292 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts @@ -13,7 +13,7 @@ export class EditorAdapter> { features: Map>; isDisposed = false; - private last_change: IEditorChange; + private last_change: IEditorChange | null; private console: ILSPLogConsole; constructor( @@ -50,7 +50,7 @@ export class EditorAdapter> { return; } - let change: IEditorChange = this.last_change; + let change: IEditorChange | null = this.last_change; let root_position: IRootPosition; @@ -75,7 +75,9 @@ export class EditorAdapter> { for (let feature of this.features.values()) { try { - feature.afterChange(change, root_position); + if (feature.afterChange) { + feature.afterChange(change, root_position); + } } catch (err) { console.warn('afterChange of feature', feature, 'failed with', err); } @@ -107,7 +109,7 @@ export class EditorAdapter> { this.editor.change.disconnect(this.saveChange); // just to be sure - this.editor = null; + this.editor = null as any; // actually disposed this.isDisposed = true; diff --git a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts index c9f4a7d7c..c22a54ace 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/testutils.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/testutils.ts @@ -90,8 +90,8 @@ export class MockSettings implements IFeatureSettings { this.changed = new Signal(this); } - get composite(): T { - return this.settings; + get composite(): Required { + return this.settings as Required; } set(setting: keyof T, value: any): void { @@ -112,7 +112,7 @@ export class MockExtension implements ILSPExtension { translator: ITranslator; constructor() { - this.app = null; + this.app = null as any; this.feature_manager = new FeatureManager(); this.editor_type_manager = new VirtualEditorManager(); this.language_server_manager = new MockLanguageServerManager({ @@ -130,7 +130,7 @@ export class MockExtension implements ILSPExtension { this.foreign_code_extractors = {}; this.code_overrides = {}; this.console = new BrowserConsole(); - this.user_console = null; + this.user_console = null as any; } } @@ -247,7 +247,7 @@ function FeatureSupport(Base: TBase) { } public dispose_feature(feature: CodeMirrorIntegration) { - let connection = this._connections.get(feature); + let connection = this._connections.get(feature)!; connection.close(); feature.is_registered = false; } @@ -410,16 +410,16 @@ export const python_notebook_metadata = { export function showAllCells(notebook: Notebook) { notebook.show(); // iterate over every cell to activate the editors - for (let i = 0; i < notebook.model.cells.length; i++) { + for (let i = 0; i < notebook.model!.cells.length; i++) { notebook.activeCellIndex = i; - notebook.activeCell.show(); + notebook.activeCell!.show(); } } export function getCellsJSON(notebook: Notebook): Array { let cells: Array = []; - for (let i = 0; i < notebook.model.cells.length; i++) { - cells.push(notebook.model.cells.get(i)); + for (let i = 0; i < notebook.model!.cells.length; i++) { + cells.push(notebook.model!.cells.get(i)); } return cells.map(cell => cell.toJSON()); } diff --git a/packages/jupyterlab-lsp/src/extractors/regexp.spec.ts b/packages/jupyterlab-lsp/src/extractors/regexp.spec.ts index ce6b6c064..1789bd094 100644 --- a/packages/jupyterlab-lsp/src/extractors/regexp.spec.ts +++ b/packages/jupyterlab-lsp/src/extractors/regexp.spec.ts @@ -112,10 +112,10 @@ describe('RegExpForeignCodeExtractor', () => { expect(result.host_code).to.equal(PYTHON_CELL_MAGIC_WITH_H); expect(result.foreign_code).to.equal('h'); - expect(result.range.start.line).to.equal(1); - expect(result.range.start.column).to.equal(0); - expect(result.range.end.line).to.equal(1); - expect(result.range.end.column).to.equal(1); + expect(result.range!.start.line).to.equal(1); + expect(result.range!.start.column).to.equal(0); + expect(result.range!.end.line).to.equal(1); + expect(result.range!.end.column).to.equal(1); }); it('should work with non-line magic and non-cell magic code snippets as well', () => { @@ -138,10 +138,10 @@ describe('RegExpForeignCodeExtractor', () => { expect(result.foreign_code).to.equal( '\nimportant link\n' ); - expect(result.range.start.line).to.equal(1); - expect(result.range.start.column).to.equal(7); - expect(result.range.end.line).to.equal(3); - expect(result.range.end.column).to.equal(4); + expect(result.range!.start.line).to.equal(1); + expect(result.range!.start.column).to.equal(7); + expect(result.range!.end.line).to.equal(3); + expect(result.range!.end.column).to.equal(4); let last_bit = results[1]; expect(last_bit.host_code).to.equal('""";\nprint(x)'); }); @@ -153,8 +153,8 @@ describe('RegExpForeignCodeExtractor', () => { expect(result.host_code).to.equal(R_CELL_MAGIC_EXISTS); expect(result.foreign_code).to.equal('some text\n'); - expect(result.range.start.line).to.equal(1); - expect(result.range.start.column).to.equal(0); + expect(result.range!.start.line).to.equal(1); + expect(result.range!.start.column).to.equal(0); }); it('should extract and remove from host', () => { @@ -215,8 +215,8 @@ describe('RegExpForeignCodeExtractor', () => { expect(first_magic.foreign_code).to.equal('df = data.frame()'); expect(first_magic.host_code).to.equal('%R df = data.frame()\n'); - expect(first_magic.range.end.line).to.equal(0); - expect(first_magic.range.end.column).to.equal(20); + expect(first_magic.range!.end.line).to.equal(0); + expect(first_magic.range!.end.column).to.equal(20); let second_magic = results[1]; diff --git a/packages/jupyterlab-lsp/src/extractors/regexp.ts b/packages/jupyterlab-lsp/src/extractors/regexp.ts index 52c2b6435..f98cb6100 100644 --- a/packages/jupyterlab-lsp/src/extractors/regexp.ts +++ b/packages/jupyterlab-lsp/src/extractors/regexp.ts @@ -15,9 +15,17 @@ export function getIndexOfCaptureGroup( // get index of the part that is being extracted to foreign document let captured_groups = expression.exec(matched_string); + + if (captured_groups == null) { + console.warn( + `No capture group found for ${expression} in ${matched_string}` + ); + return -1; + } + let offset_in_match = 0; - // first element is full match + // first element is a full match let full_matched = captured_groups[0]; for (let group of captured_groups.slice(1)) { @@ -70,7 +78,7 @@ export class RegExpForeignCodeExtractor implements IForeignCodeExtractor { let extracts = new Array(); let started_from = this.global_expression.lastIndex; - let match: RegExpExecArray = this.global_expression.exec(code); + let match: RegExpExecArray | null = this.global_expression.exec(code); let host_code_fragment: string; let chosen_replacer: string | replacer; @@ -82,13 +90,18 @@ export class RegExpForeignCodeExtractor implements IForeignCodeExtractor { } else if (typeof this.options.foreign_capture_groups !== 'undefined') { chosen_replacer = '$' + this.options.foreign_capture_groups.join('$'); is_new_api_replacer = true; - } else { + } else if (this.options.extract_to_foreign) { chosen_replacer = this.options.extract_to_foreign; + } else { + console.warn( + `Foreign replacer not defined for extractor: {this.expression} - this is deprecated; use 'foreign_replacer' to define it` + ); + return []; } while (match != null) { let matched_string = match[0]; - let position_shift: CodeEditor.IPosition = null; + let position_shift: CodeEditor.IPosition | null = null; let foreign_code_fragment = matched_string.replace( this.expression, @@ -127,7 +140,7 @@ export class RegExpForeignCodeExtractor implements IForeignCodeExtractor { if (is_new_api_replacer) { foreign_code_group_value = matched_string.replace( this.expression, - '$' + Math.min(...this.options.foreign_capture_groups) + '$' + Math.min(...this.options.foreign_capture_groups!) ); } diff --git a/packages/jupyterlab-lsp/src/extractors/testutils.ts b/packages/jupyterlab-lsp/src/extractors/testutils.ts index 7fd38b8bb..4c6c63141 100644 --- a/packages/jupyterlab-lsp/src/extractors/testutils.ts +++ b/packages/jupyterlab-lsp/src/extractors/testutils.ts @@ -5,7 +5,7 @@ import { IVirtualDocumentBlock, VirtualDocument } from '../virtual/document'; export function extract_code(document: VirtualDocument, code: string) { return document.extract_foreign_code( - { value: code, ce_editor: null }, + { value: code, ce_editor: null as any }, { line: 0, column: 0 @@ -24,7 +24,7 @@ export function get_the_only_pair( expect(foreign_document_map.size).to.equal(1); let range = foreign_document_map.keys().next().value; - let { virtual_document } = foreign_document_map.get(range); + let { virtual_document } = foreign_document_map.get(range)!; return { range, virtual_document }; } diff --git a/packages/jupyterlab-lsp/src/extractors/types.ts b/packages/jupyterlab-lsp/src/extractors/types.ts index c9e3085b0..710f65f75 100644 --- a/packages/jupyterlab-lsp/src/extractors/types.ts +++ b/packages/jupyterlab-lsp/src/extractors/types.ts @@ -9,8 +9,9 @@ export interface IExtractedCode { foreign_code: string | null; /** * Range of the foreign code relative to the original source. + * `null` is used internally to represent a leftover host code after extraction. */ - range: CodeEditor.IRange; + range: CodeEditor.IRange | null; /** * Shift due to any additional code inserted at the beginning of the virtual document * (usually in order to mock the arguments passed to a magic, or to provide other context clues for the linters) diff --git a/packages/jupyterlab-lsp/src/feature.ts b/packages/jupyterlab-lsp/src/feature.ts index b98e6150f..c5540ee52 100644 --- a/packages/jupyterlab-lsp/src/feature.ts +++ b/packages/jupyterlab-lsp/src/feature.ts @@ -54,7 +54,7 @@ export interface IFeatureCommand { } export interface IFeatureSettings { - readonly composite: T; + readonly composite: Required; readonly changed: Signal, void>; readonly ready?: Promise; @@ -90,8 +90,8 @@ export class FeatureSettings implements IFeatureSettings { } } - get composite(): T { - return this.settings.composite as unknown as T; + get composite(): Required { + return this.settings.composite as unknown as Required; } set(setting: keyof T, value: any) { diff --git a/packages/jupyterlab-lsp/src/features/completion/completion.ts b/packages/jupyterlab-lsp/src/features/completion/completion.ts index 9fb36bb3c..fd1e82f2b 100644 --- a/packages/jupyterlab-lsp/src/features/completion/completion.ts +++ b/packages/jupyterlab-lsp/src/features/completion/completion.ts @@ -85,9 +85,9 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { // TODO: maybe instead of creating it each time, keep a hash map instead? protected current_completion_connector: LSPConnector; protected current_completion_handler: CompletionHandler; - protected current_adapter: WidgetAdapter = null; + protected current_adapter: WidgetAdapter | null = null; protected renderer: LSPCompletionRenderer; - private _latestActiveItem: LazyCompletionItem = null; + private _latestActiveItem: LazyCompletionItem | null = null; constructor( private app: JupyterFrontEnd, @@ -141,7 +141,7 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { item .resolve() .then(resolvedCompletionItem => { - if (item.self !== this._latestActiveItem.self) { + if (item.self !== this._latestActiveItem!.self) { return; } this.set_doc_panel_placeholder(false); @@ -153,7 +153,7 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { .catch(e => { // disabling placeholder can remove currently displayed documentation, // so only do that if this is really the active item! - if (item.self === this._latestActiveItem.self) { + if (item.self === this._latestActiveItem!.self) { this.set_doc_panel_placeholder(false); } this.console.warn(e); @@ -253,7 +253,8 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { // connect the new adapter if (this.current_adapter.isConnected) { this.connect_completion(this.current_adapter); - this.set_connector(adapter, { editor: adapter.activeEditor }); + // TODO: what to do if adapter.activeEditor was just deleted/there is none because focus shifted? + this.set_connector(adapter, { editor: adapter.activeEditor! }); } // connect signals to the new adapter this.current_adapter.activeEditorChanged.connect(this.set_connector, this); @@ -355,6 +356,10 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { } const docPanel = completer.node.querySelector(DOC_PANEL_SELECTOR); + if (!docPanel) { + this.console.warn('Could not find completer panel to refresh'); + return; + } docPanel.classList.remove(DOC_PANEL_PLACEHOLDER_CLASS); if (item.documentation) { @@ -372,9 +377,13 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { } } - set_doc_panel_placeholder(enable: boolean) { + set_doc_panel_placeholder(enable: boolean): void { let completer = this.current_completion_handler.completer; const docPanel = completer.node.querySelector(DOC_PANEL_SELECTOR); + if (!docPanel) { + this.console.warn('Could not find completer panel for placeholder'); + return; + } if (enable) { docPanel.setAttribute('style', ''); docPanel.classList.add(DOC_PANEL_PLACEHOLDER_CLASS); @@ -388,18 +397,15 @@ export class CompletionLabIntegration implements IFeatureLabIntegration { adapter: WidgetAdapter, editor: CodeEditor.IEditor ) { - if (this.current_completion_connector) { - delete this.current_completion_connector; - } this.current_completion_connector = new LSPConnector({ editor: editor, themeManager: this.completionThemeManager, - connections: this.current_adapter.connection_manager.connections, - virtual_editor: this.current_adapter.virtual_editor, + connections: this.current_adapter!.connection_manager.connections, + virtual_editor: this.current_adapter!.virtual_editor, settings: this.settings, labIntegration: this, // it might or might not be a notebook panel (if it is not, the sessionContext and session will just be undefined) - session: (this.current_adapter.widget as NotebookPanel)?.sessionContext + session: (this.current_adapter!.widget as NotebookPanel)?.sessionContext ?.session, console: this.console }); diff --git a/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts b/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts index 5b0dfbea3..e13c2f19a 100644 --- a/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts +++ b/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts @@ -49,7 +49,7 @@ export interface ICompletionsReply extends CompletionHandler.ICompletionItemsReply { // TODO: it is not clear when the source is set here and when on IExtendedCompletionItem. // it might be good to separate the two stages for both interfaces - source: ICompletionsSource; + source: ICompletionsSource | null; items: IExtendedCompletionItem[]; } @@ -131,13 +131,13 @@ export class LSPConnector if (this.isDisposed) { return; } - this._connections = null; - this.virtual_editor = null; - this._context_connector = null; - this._kernel_connector = null; - this._kernel_and_context_connector = null; - this.options = null; - this._editor = null; + this._connections = null as any; + this.virtual_editor = null as any; + this._context_connector = null as any; + this._kernel_connector = null as any; + this._kernel_and_context_connector = null as any; + this.options = null as any; + this._editor = null as any; this.isDisposed = true; } @@ -154,7 +154,7 @@ export class LSPConnector } protected async _kernel_language(): Promise { - return (await this.options.session.kernel.info).language_info.name; + return (await this.options.session!.kernel!.info).language_info.name; } protected get _kernel_timeout(): number { @@ -176,7 +176,7 @@ export class LSPConnector return this.virtual_editor.transform_from_editor_to_root( this._editor, editor_position - ); + )!; } /** @@ -186,28 +186,34 @@ export class LSPConnector */ async fetch( request: CompletionHandler.IRequest - ): Promise { + ): Promise { let editor = this._editor; const cursor = editor.getCursorPosition(); const token = editor.getTokenForPosition(cursor); if (this.trigger_kind == AdditionalCompletionTriggerKinds.AutoInvoked) { - if (this.suppress_continuous_hinting_in.indexOf(token.type) !== -1) { + if ( + token.type && + this.suppress_continuous_hinting_in.indexOf(token.type) !== -1 + ) { this.console.debug('Suppressing completer auto-invoke in', token.type); this.trigger_kind = CompletionTriggerKind.Invoked; return; } } else if (this.trigger_kind == CompletionTriggerKind.TriggerCharacter) { - if (this.suppress_trigger_character_in.indexOf(token.type) !== -1) { + if ( + token.type && + this.suppress_trigger_character_in.indexOf(token.type) !== -1 + ) { this.console.debug('Suppressing completer auto-invoke in', token.type); this.trigger_kind = CompletionTriggerKind.Invoked; return; } } - const start = editor.getPositionAt(token.offset); - const end = editor.getPositionAt(token.offset + token.value.length); + const start = editor.getPositionAt(token.offset)!; + const end = editor.getPositionAt(token.offset + token.value.length)!; let position_in_token = cursor.column - start.column - 1; const typed_character = token.value[cursor.column - start.column - 1]; @@ -227,8 +233,9 @@ export class LSPConnector virtual_editor.root_position_to_virtual_position(end_in_root); let virtual_cursor = virtual_editor.root_position_to_virtual_position(cursor_in_root); - const lsp_promise: Promise = this - .use_lsp_completions + const lsp_promise: Promise< + CompletionHandler.ICompletionItemsReply | undefined + > = this.use_lsp_completions ? this.fetch_lsp( token, typed_character, @@ -238,9 +245,11 @@ export class LSPConnector document, position_in_token ) - : Promise.resolve(null); + : Promise.resolve(undefined); - let promise: Promise = null; + let promise: Promise< + CompletionHandler.ICompletionItemsReply | undefined + > | null = null; try { const kernelTimeout = this._kernel_timeout; @@ -278,10 +287,10 @@ export class LSPConnector return setTimeout( () => resolve({ - start: null, - end: null, + start: 0, + end: 0, matches: [], - metadata: null + metadata: {} }), kernelTimeout ); @@ -322,7 +331,9 @@ export class LSPConnector this.console.debug('All promises set up and ready.'); return promise.then(reply => { reply = this.suppress_if_needed(reply, token, cursor); - this.items = reply.items; + if (reply) { + this.items = reply.items; + } this.trigger_kind = CompletionTriggerKind.Invoked; return reply; }); @@ -341,7 +352,7 @@ export class LSPConnector document: VirtualDocument, position_in_token: number ): Promise { - let connection = this.get_connection(document.uri); + let connection = this.get_connection(document.uri)!; this.console.debug('Fetching'); this.console.debug('Token:', token, start, end); @@ -459,9 +470,6 @@ export class LSPConnector } protected icon_for(type: string): LabIcon { - if (!this.options.settings.composite.theme) { - return undefined; - } if (typeof type === 'undefined') { type = KernelKind; } @@ -514,7 +522,7 @@ export class LSPConnector replies = replies.filter(reply => { if (reply instanceof Error) { this.console.warn( - `Caught ${reply.source.name} completions error`, + `Caught ${reply.source!.name} completions error`, reply ); return false; @@ -527,7 +535,7 @@ export class LSPConnector return true; }); - replies.sort((a, b) => b.source.priority - a.source.priority); + replies.sort((a, b) => b.source!.priority - a.source!.priority); this.console.debug('Sorted replies:', replies); @@ -541,24 +549,30 @@ export class LSPConnector if (minStart != maxStart) { const cursor = editor.getCursorPosition(); const line = editor.getLine(cursor.line); - - replies = replies.map(reply => { - // no prefix to strip, return as-is - if (reply.start == maxStart) { - return reply; - } - let prefix = line.substring(reply.start, maxStart); - this.console.debug(`Removing ${reply.source.name} prefix: `, prefix); - return { - ...reply, - items: reply.items.map(item => { - item.insertText = item.insertText.startsWith(prefix) - ? item.insertText.substr(prefix.length) - : item.insertText; - return item; - }) - }; - }); + if (line == null) { + this.console.warn( + `Could not remove prefixes: line is undefined`, + cursor.line + ); + } else { + replies = replies.map(reply => { + // no prefix to strip, return as-is + if (reply.start == maxStart) { + return reply; + } + let prefix = line.substring(reply.start, maxStart); + this.console.debug(`Removing ${reply.source!.name} prefix: `, prefix); + return { + ...reply, + items: reply.items.map(item => { + item.insertText = item.insertText.startsWith(prefix) + ? item.insertText.substr(prefix.length) + : item.insertText; + return item; + }) + }; + }); + } } const insertTextSet = new Set(); @@ -580,9 +594,11 @@ export class LSPConnector // lead to processing hundreds of suggestions - e.g. from numpy // multiple times if multiple sources provide them). let processedItem = item as IExtendedCompletionItem; - processedItem.source = reply.source; - if (!processedItem.icon) { - processedItem.icon = reply.source.fallbackIcon; + if (reply.source) { + processedItem.source = reply.source; + if (!processedItem.icon) { + processedItem.icon = reply.source.fallbackIcon; + } } processedItems.push(processedItem); }); @@ -598,10 +614,7 @@ export class LSPConnector }; } - list(query: string | undefined): Promise<{ - ids: CompletionHandler.IRequest[]; - values: CompletionHandler.ICompletionItemsReply[]; - }> { + list(query: string | undefined): Promise { return Promise.resolve(undefined); } @@ -614,10 +627,13 @@ export class LSPConnector } private suppress_if_needed( - reply: CompletionHandler.ICompletionItemsReply, + reply: CompletionHandler.ICompletionItemsReply | undefined, token: CodeEditor.IToken, cursor_at_request: CodeEditor.IPosition ) { + if (reply == null) { + return reply; + } if (!this._editor.hasFocus()) { this.console.debug( 'Ignoring completion response: the corresponding editor lost focus' @@ -689,7 +705,7 @@ export namespace LSPConnector { themeManager: ILSPCompletionThemeManager; - session?: Session.ISessionConnection; + session?: Session.ISessionConnection | null; console: ILSPLogConsole; } diff --git a/packages/jupyterlab-lsp/src/features/completion/index.ts b/packages/jupyterlab-lsp/src/features/completion/index.ts index 47b414ffb..8d9941033 100644 --- a/packages/jupyterlab-lsp/src/features/completion/index.ts +++ b/packages/jupyterlab-lsp/src/features/completion/index.ts @@ -9,6 +9,7 @@ import { LabIcon } from '@jupyterlab/ui-components'; import { ILSPCompletionThemeManager } from '@krassowski/completion-theme/lib/types'; import completionSvg from '../../../style/icons/completion.svg'; +import { CodeCompletion as LSPCompletionSettings } from '../../_completion'; import { FeatureSettings } from '../../feature'; import { ILSPAdapterManager, @@ -52,7 +53,7 @@ export const COMPLETION_PLUGIN: JupyterFrontEndPlugin = { const labIntegration = new CompletionLabIntegration( app, completionManager, - settings, + settings as FeatureSettings, adapterManager, iconsThemeManager, logConsole.scope('CompletionLab'), diff --git a/packages/jupyterlab-lsp/src/features/completion/item.ts b/packages/jupyterlab-lsp/src/features/completion/item.ts index b69699870..c3a3c1bcc 100644 --- a/packages/jupyterlab-lsp/src/features/completion/item.ts +++ b/packages/jupyterlab-lsp/src/features/completion/item.ts @@ -35,8 +35,8 @@ export interface IExtendedCompletionItem } export class LazyCompletionItem implements IExtendedCompletionItem { - private _detail: string; - private _documentation: string; + private _detail: string | undefined; + private _documentation: string | undefined; private _is_documentation_markdown: boolean; private _requested_resolution: boolean; private _resolved: boolean; @@ -82,7 +82,9 @@ export class LazyCompletionItem implements IExtendedCompletionItem { this.self = this; } - private _setDocumentation(documentation: string | lsProtocol.MarkupContent) { + private _setDocumentation( + documentation: string | lsProtocol.MarkupContent | undefined + ) { if (lsProtocol.MarkupContent.is(documentation)) { this._documentation = documentation.value; this._is_documentation_markdown = documentation.kind === 'markdown'; @@ -107,17 +109,17 @@ export class LazyCompletionItem implements IExtendedCompletionItem { return this.match.sortText || this.match.label; } - get filterText(): string { + get filterText(): string | undefined { return this.match.filterText; } public supportsResolution() { const connection = this.connector.get_connection(this.uri); - return connection.isCompletionResolveProvider(); + return connection != null && connection.isCompletionResolveProvider(); } - get detail(): string { + get detail(): string | undefined { return this._detail; } @@ -155,7 +157,7 @@ export class LazyCompletionItem implements IExtendedCompletionItem { return until_ready(() => this._resolved, 100, 50).then(() => this); } - const connection = this.connector.get_connection(this.uri); + const connection = this.connector.get_connection(this.uri)!; this._requested_resolution = true; @@ -165,11 +167,12 @@ export class LazyCompletionItem implements IExtendedCompletionItem { if (resolvedCompletionItem === null) { return resolvedCompletionItem; } - this._setDocumentation(resolvedCompletionItem.documentation); - this._detail = resolvedCompletionItem.detail; + this._setDocumentation(resolvedCompletionItem?.documentation); + this._detail = resolvedCompletionItem?.detail; // TODO: implement in pyls and enable with proper LSP communication // this.label = resolvedCompletionItem.label; this._resolved = true; + return this; }); } @@ -177,14 +180,14 @@ export class LazyCompletionItem implements IExtendedCompletionItem { * A human-readable string with additional information * about this item, like type or symbol information. */ - get documentation(): string { + get documentation(): string | undefined { if (!this.connector.should_show_documentation) { - return null; + return undefined; } if (this._documentation) { return this._documentation; } - return null; + return undefined; } /** @@ -195,7 +198,7 @@ export class LazyCompletionItem implements IExtendedCompletionItem { return this.match.deprecated; } return ( - this.match.tags && + this.match.tags != null && this.match.tags.some( tag => tag == lsProtocol.CompletionItemTag.Deprecated ) diff --git a/packages/jupyterlab-lsp/src/features/completion/model.spec.ts b/packages/jupyterlab-lsp/src/features/completion/model.spec.ts index d423a91d0..066b5a92d 100644 --- a/packages/jupyterlab-lsp/src/features/completion/model.spec.ts +++ b/packages/jupyterlab-lsp/src/features/completion/model.spec.ts @@ -11,7 +11,13 @@ describe('LSPCompleterModel', () => { match: lsProtocol.CompletionItem, type: string = 'dummy' ) { - return new LazyCompletionItem(type, null, match, null, null); + return new LazyCompletionItem( + type, + null as any, + match, + null as any, + 'file://test.ipynb' + ); } const jupyter_icon_completion = create_dummy_item({ diff --git a/packages/jupyterlab-lsp/src/features/completion/model.ts b/packages/jupyterlab-lsp/src/features/completion/model.ts index 98d0a3970..b94736b30 100644 --- a/packages/jupyterlab-lsp/src/features/completion/model.ts +++ b/packages/jupyterlab-lsp/src/features/completion/model.ts @@ -39,7 +39,7 @@ export class GenericCompleterModel< completionItems(): T[] { let query = this.query; this.query = ''; - let unfilteredItems = super.completionItems() as T[]; + let unfilteredItems = super.completionItems!() as T[]; this.query = query; // always want to sort @@ -48,7 +48,7 @@ export class GenericCompleterModel< } setCompletionItems(newValue: T[]) { - super.setCompletionItems(newValue); + super.setCompletionItems!(newValue); } private _markFragment(value: string): string { @@ -80,8 +80,8 @@ export class GenericCompleterModel< let matched: boolean; - let filterText: string = null; - let filterMatch: StringExt.IMatchResult; + let filterText: string | null = null; + let filterMatch: StringExt.IMatchResult | null = null; let lowerCaseQuery = query.toLowerCase(); @@ -108,7 +108,7 @@ export class GenericCompleterModel< // If the matches are substrings of label, highlight them // in this part of the label that can be highlighted (must be a prefix), // which is intended to avoid highlighting matches in function arguments etc. - let labelMatch: StringExt.IMatchResult; + let labelMatch: StringExt.IMatchResult | null = null; if (query) { let labelPrefix = escapeHTML(this.getHighlightableLabelRegion(item)); if (labelPrefix == filterText) { diff --git a/packages/jupyterlab-lsp/src/features/completion/renderer.ts b/packages/jupyterlab-lsp/src/features/completion/renderer.ts index 34b7fcd64..6c0d3df35 100644 --- a/packages/jupyterlab-lsp/src/features/completion/renderer.ts +++ b/packages/jupyterlab-lsp/src/features/completion/renderer.ts @@ -46,7 +46,7 @@ export class LSPCompletionRenderer return; } let li = entry.target as HTMLLIElement; - let item = this.elementToItem.get(li); + let item = this.elementToItem.get(li)!; this.itemShown.emit({ item: item, element: li @@ -71,7 +71,7 @@ export class LSPCompletionRenderer if (li.classList.contains('jp-mod-active')) { if (inactive) { this.wasActivated.set(li, true); - let item = this.elementToItem.get(li); + let item = this.elementToItem.get(li)!; this.activeChanged.emit({ item: item, element: li @@ -88,14 +88,14 @@ export class LSPCompletionRenderer const labelExtra = this.options.integrator.settings.composite.labelExtra; switch (labelExtra) { case 'detail': - return item?.detail; + return item?.detail || ''; case 'type': return item?.type?.toLowerCase?.(); case 'source': return item?.source?.name; case 'auto': return [ - item?.detail, + item?.detail || '', item?.type?.toLowerCase?.(), item?.source?.name ].filter(x => !!x)[0]; @@ -104,6 +104,7 @@ export class LSPCompletionRenderer 'labelExtra does not match any of the expected values', labelExtra ); + return ''; } } @@ -145,7 +146,7 @@ export class LSPCompletionRenderer createDocumentationNode(item: LazyCompletionItem): HTMLElement { // note: not worth trying to `fetchDocumentation()` as this is not // invoked if documentation is empty (as of jlab 3.2) - if (item.isDocumentationMarkdown) { + if (item.isDocumentationMarkdown && this.options.markdownRenderer) { let documentation = item.documentation; this.options.markdownRenderer .renderModel({ @@ -159,7 +160,12 @@ export class LSPCompletionRenderer } }) .then(() => { - if (this.options.latexTypesetter && documentation.includes('$')) { + if ( + this.options.markdownRenderer && + this.options.latexTypesetter && + documentation && + documentation.includes('$') + ) { this.options.latexTypesetter.typeset( this.options.markdownRenderer.node ); @@ -169,7 +175,9 @@ export class LSPCompletionRenderer return this.options.markdownRenderer.node; } else { let node = document.createElement('pre'); - node.textContent = item.documentation; + if (item.documentation) { + node.textContent = item.documentation; + } return node; } } @@ -178,8 +186,8 @@ export class LSPCompletionRenderer export namespace LSPCompletionRenderer { export interface IOptions { integrator: CompletionLabIntegration; - markdownRenderer: IRenderMime.IRenderer; - latexTypesetter?: IRenderMime.ILatexTypesetter; + markdownRenderer: IRenderMime.IRenderer | null; + latexTypesetter?: IRenderMime.ILatexTypesetter | null; console: ILSPLogConsole; } } diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts index 805aee9ec..ff56427be 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.spec.ts @@ -77,7 +77,7 @@ describe('Diagnostics', () => { markers = env.ce_editor.editor.getDoc().getAllMarks(); expect(markers.length).to.equal(0); - feature.handleDiagnostic(null, { + feature.handleDiagnostic(null as any, { uri: env.document_options.path, diagnostics: diagnostics }); @@ -99,7 +99,7 @@ describe('Diagnostics', () => { env.ce_editor.model.value.text = text; await env.adapter.update_documents(); - feature.handleDiagnostic(null, { + feature.handleDiagnostic(null as any, { uri: env.document_options.path, diagnostics: diagnostics }); @@ -124,7 +124,7 @@ describe('Diagnostics', () => { env.ce_editor.model.value.text = text; await env.adapter.update_documents(); - feature.handleDiagnostic(null, { + feature.handleDiagnostic(null as any, { uri: env.document_options.path, diagnostics: diagnostics }); @@ -167,7 +167,7 @@ describe('Diagnostics', () => { let document = env.virtual_editor.virtual_document; let uri = env.virtual_editor.virtual_document.uri; - feature.handleDiagnostic(null, { + feature.handleDiagnostic(null as any, { uri: uri, diagnostics: [ { @@ -240,14 +240,14 @@ describe('Diagnostics', () => { expect(merged_mark_title).to.contain('\n'); expect(feature.diagnostics_db.size).to.equal(1); - expect(feature.diagnostics_db.get(document).length).to.equal(5); + expect(feature.diagnostics_db.get(document)!.length).to.equal(5); feature.switchDiagnosticsPanelSource(); diagnostics_panel.widget.content.update(); // the panel should contain all 5 diagnostics - let db = diagnostics_panel.content.model.diagnostics; + let db = diagnostics_panel.content.model.diagnostics!; expect(db.size).to.equal(1); - expect(db.get(document).length).to.equal(5); + expect(db.get(document)!.length).to.equal(5); }); it('Works in foreign documents', async () => { @@ -285,7 +285,7 @@ describe('Diagnostics', () => { } as lsProtocol.PublishDiagnosticsParams; // test guards against wrongly propagated responses: - feature.handleDiagnostic(null, response); + feature.handleDiagnostic(null as any, response); let cm_editors = env.adapter.editors.map( ce_editor => (ce_editor as CodeMirrorEditor).editor ); @@ -297,7 +297,7 @@ describe('Diagnostics', () => { expect(marks_cell_2.length).to.equal(0); // correct propagation - foreign_feature.handleDiagnostic(null, response); + foreign_feature.handleDiagnostic(null as any, response); marks_cell_1 = cm_editors[0].getDoc().getAllMarks(); marks_cell_2 = cm_editors[1].getDoc().getAllMarks(); @@ -307,14 +307,14 @@ describe('Diagnostics', () => { let mark = marks_cell_2[0] as TextMarker; - let mark_position = mark.find(); + let mark_position = mark.find()!; // second line (0th and 1st virtual lines) + 1 line for '%%python\n' => line: 2 expect(is_equal(mark_position.from, { line: 2, ch: 0 })).to.be.true; expect(is_equal(mark_position.to, { line: 2, ch: 1 })).to.be.true; // the silenced diagnostic for the %%python magic should be ignored - feature.handleDiagnostic(null, { + feature.handleDiagnostic(null as any, { uri: document.uri, diagnostics: [ { diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts index 7b3dcabfe..3f948c8de 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts @@ -50,8 +50,8 @@ function escapeRegExp(string: string) { } class DiagnosticsPanel { - private _content: DiagnosticsListing = null; - private _widget: MainAreaWidget = null; + private _content: DiagnosticsListing | null = null; + private _widget: MainAreaWidget | null = null; feature: DiagnosticsCM; is_registered = false; trans: TranslationBundle; @@ -114,14 +114,14 @@ class DiagnosticsPanel { app.commands.addCommand(CMD_COLUMN_VISIBILITY, { execute: args => { - let column = get_column(args['id'] as string); + let column = get_column(args['id'] as string)!; column.is_visible = !column.is_visible; widget.update(); }, label: args => this.trans.__(args['id'] as string), isToggled: args => { let column = get_column(args['id'] as string); - return column.is_visible; + return column ? column.is_visible : false; } }); @@ -143,14 +143,14 @@ class DiagnosticsPanel { 'Ignore diagnostics like this' ); - let get_row = (): IDiagnosticsRow => { + let get_row = (): IDiagnosticsRow | undefined => { let tr = app.contextMenuHitTest( node => node.tagName.toLowerCase() == 'tr' ); if (!tr) { return; } - return this.widget.content.get_diagnostic(tr.dataset.key); + return this.widget.content.get_diagnostic(tr.dataset.key!); }; ignore_diagnostics_menu.addItem({ @@ -161,7 +161,14 @@ class DiagnosticsPanel { }); app.commands.addCommand(CMD_IGNORE_DIAGNOSTIC_CODE, { execute: () => { - const diagnostic = get_row().data.diagnostic; + const row = get_row(); + if (!row) { + console.warn( + 'LPS: diagnostics row not found for ignore code execute()' + ); + return; + } + const diagnostic = row.data.diagnostic; let current = this.content.model.settings.composite.ignoreCodes; this.content.model.settings.set('ignoreCodes', [ ...current, @@ -192,6 +199,12 @@ class DiagnosticsPanel { app.commands.addCommand(CMD_IGNORE_DIAGNOSTIC_MSG, { execute: () => { const row = get_row(); + if (!row) { + console.warn( + 'LPS: diagnostics row not found for ignore message execute()' + ); + return; + } const diagnostic = row.data.diagnostic; let current = this.content.model.settings.composite.ignoreMessagesPatterns; @@ -225,6 +238,10 @@ class DiagnosticsPanel { app.commands.addCommand(CMD_JUMP_TO_DIAGNOSTIC, { execute: () => { const row = get_row(); + if (!row) { + console.warn('LPS: diagnostics row not found for jump execute()'); + return; + } this.widget.content.jump_to(row); }, label: this.trans.__('Jump to location'), @@ -235,6 +252,7 @@ class DiagnosticsPanel { execute: () => { const row = get_row(); if (!row) { + console.warn('LPS: diagnostics row not found for copy execute()'); return; } const message = row.data.diagnostic.message; @@ -335,7 +353,7 @@ export class DiagnosticsCM extends CodeMirrorIntegration { if (!diagnostics_databases.has(this.virtual_editor)) { diagnostics_databases.set(this.virtual_editor, new DiagnosticsDatabase()); } - return diagnostics_databases.get(this.virtual_editor); + return diagnostics_databases.get(this.virtual_editor)!; } switchDiagnosticsPanelSource = () => { @@ -385,7 +403,7 @@ export class DiagnosticsCM extends CodeMirrorIntegration { let range_id = get_range_id(range); range_id_to_range.set(range_id, range); if (range_id_to_diagnostics.has(range_id)) { - let ranges_list = range_id_to_diagnostics.get(range_id); + let ranges_list = range_id_to_diagnostics.get(range_id)!; ranges_list.push(diagnostic); } else { range_id_to_diagnostics.set(range_id, [diagnostic]); @@ -396,7 +414,7 @@ export class DiagnosticsCM extends CodeMirrorIntegration { range_id_to_diagnostics.forEach( (range_diagnostics: lsProtocol.Diagnostic[], range_id: RangeID) => { - let range = range_id_to_range.get(range_id); + let range = range_id_to_range.get(range_id)!; map.set(range, range_diagnostics); } ); @@ -517,7 +535,7 @@ export class DiagnosticsCM extends CodeMirrorIntegration { if ( document.virtual_lines - .get(start.line) + .get(start.line)! .skip_inspect.indexOf(document.id_path) !== -1 ) { this.console.log( @@ -535,10 +553,18 @@ export class DiagnosticsCM extends CodeMirrorIntegration { let ce_editor = document.get_editor_at_virtual_line(start); let cm_editor = - this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); + this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor)!; let start_in_editor = document.transform_virtual_to_editor(start); - let end_in_editor: IEditorPosition; + let end_in_editor: IEditorPosition | null; + + if (start_in_editor === null) { + this.console.warn( + 'Start in editor could not be be determined for', + diagnostics + ); + return; + } // some servers return strange positions for ends try { @@ -548,6 +574,14 @@ export class DiagnosticsCM extends CodeMirrorIntegration { end_in_editor = { ...start_in_editor, ch: start_in_editor.ch + 1 }; } + if (end_in_editor === null) { + this.console.warn( + 'End in editor could not be be determined for', + diagnostics + ); + return; + } + let range_in_editor = { start: start_in_editor, end: end_in_editor diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx b/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx index baf6363cc..963f47069 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx +++ b/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx @@ -33,6 +33,7 @@ export interface IEditorDiagnostic { } export const DIAGNOSTICS_LISTING_CLASS = 'lsp-diagnostics-listing'; +const DIAGNOSTICS_PLACEHOLDER_CLASS = 'lsp-diagnostics-placeholder'; export class DiagnosticsDatabase extends Map< VirtualDocument, @@ -119,7 +120,7 @@ function SortableTH(props: { props.listing.sort(props.id)} - className={is_sort_key ? 'lsp-sorted-header' : null} + className={is_sort_key ? 'lsp-sorted-header' : undefined} data-id={props.id} >
    @@ -212,22 +213,40 @@ export class DiagnosticsListing extends VDomRenderer { {this.severityTranslations[severity] || severity} ); }, - sort: (a, b) => - a.data.diagnostic.severity > b.data.diagnostic.severity ? 1 : -1 + sort: (a, b) => { + if (!a.data.diagnostic.severity) { + return +1; + } + if (!b.data.diagnostic.severity) { + return -1; + } + return a.data.diagnostic.severity > b.data.diagnostic.severity + ? 1 + : -1; + } }), new Column({ id: 'Source', label: this.trans.__('Source'), render_cell: row => {row.data.diagnostic.source}, - sort: (a, b) => - a.data.diagnostic.source.localeCompare(b.data.diagnostic.source) + sort: (a, b) => { + if (!a.data.diagnostic.source) { + return +1; + } + if (!b.data.diagnostic.source) { + return -1; + } + return a.data.diagnostic.source.localeCompare( + b.data.diagnostic.source + ); + } }), new Column({ id: 'Cell', label: this.trans.__('Cell'), render_cell: row => {row.cell_number}, sort: (a, b) => - a.cell_number - b.cell_number || + a.cell_number! - b.cell_number! || a.data.range.start.line - b.data.range.start.line || a.data.range.start.ch - b.data.range.start.ch, is_available: context => context.adapter.has_multiple_editors @@ -261,8 +280,19 @@ export class DiagnosticsListing extends VDomRenderer { let diagnostics_db = this.model.diagnostics; const editor = this.model.virtual_editor; const adapter = this.model.adapter; - if (!diagnostics_db || editor == null) { - return
    {this.trans.__('No issues detected, great job!')}
    ; + if (diagnostics_db == null || editor == null || !adapter) { + return ( +
    + {this.trans.__('Diagnostics are not available')} +
    + ); + } + if (diagnostics_db.size === 0) { + return ( +
    + {this.trans.__('No issues detected, great job!')} +
    + ); } let by_document = Array.from(diagnostics_db).map( @@ -271,7 +301,7 @@ export class DiagnosticsListing extends VDomRenderer { return []; } return diagnostics.map((diagnostic_data, i) => { - let cell_number: number = null; + let cell_number: number | null = null; if (adapter.has_multiple_editors) { let { index: cell_id } = editor.find_editor(diagnostic_data.editor); cell_number = cell_id + 1; @@ -337,7 +367,7 @@ export class DiagnosticsListing extends VDomRenderer { ); } - get_diagnostic(key: string): IDiagnosticsRow { + get_diagnostic(key: string): IDiagnosticsRow | undefined { if (!this._diagnostics.has(key)) { console.warn('Could not find the diagnostics row with key', key); return; @@ -358,9 +388,9 @@ export namespace DiagnosticsListing { * A VDomModel for the LSP of current file editor/notebook. */ export class Model extends VDomModel { - diagnostics: DiagnosticsDatabase; - virtual_editor: CodeMirrorVirtualEditor; - adapter: WidgetAdapter; + diagnostics: DiagnosticsDatabase | null; + virtual_editor: CodeMirrorVirtualEditor | null; + adapter: WidgetAdapter | null; settings: FeatureSettings; status_message: StatusMessage; trans: TranslationBundle; diff --git a/packages/jupyterlab-lsp/src/features/highlights.ts b/packages/jupyterlab-lsp/src/features/highlights.ts index 6e2099c0b..d9d553a1f 100644 --- a/packages/jupyterlab-lsp/src/features/highlights.ts +++ b/packages/jupyterlab-lsp/src/features/highlights.ts @@ -34,16 +34,18 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ { id: 'highlight-references', execute: ({ connection, virtual_position, document }) => - connection.getReferences(virtual_position, document.document_info), - is_enabled: ({ connection }) => connection.isReferencesSupported(), + connection?.getReferences(virtual_position, document.document_info), + is_enabled: ({ connection }) => + connection ? connection.isReferencesSupported() : false, label: trans.__('Highlight references'), icon: highlightIcon }, { id: 'highlight-type-definition', execute: ({ connection, virtual_position, document }) => - connection.getTypeDefinition(virtual_position, document.document_info), - is_enabled: ({ connection }) => connection.isTypeDefinitionSupported(), + connection?.getTypeDefinition(virtual_position, document.document_info), + is_enabled: ({ connection }) => + connection ? connection.isTypeDefinitionSupported() : false, label: trans.__('Highlight type definition'), icon: highlightTypeIcon } @@ -51,10 +53,12 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ export class HighlightsCM extends CodeMirrorIntegration { protected highlight_markers: CodeMirror.TextMarker[] = []; - private debounced_get_highlight: Debouncer; + private debounced_get_highlight: Debouncer< + lsProtocol.DocumentHighlight[] | undefined + >; private virtual_position: IVirtualPosition; private sent_version: number; - private last_token: CodeEditor.IToken; + private last_token: CodeEditor.IToken | null = null; get settings() { return super.settings as FeatureSettings; @@ -82,8 +86,6 @@ export class HighlightsCM extends CodeMirrorIntegration { }; remove(): void { - this.handleHighlight = null; - this.onCursorActivity = null; this.clear_markers(); super.remove(); } @@ -95,7 +97,9 @@ export class HighlightsCM extends CodeMirrorIntegration { this.highlight_markers = []; } - protected handleHighlight = (items: lsProtocol.DocumentHighlight[]) => { + protected handleHighlight = ( + items: lsProtocol.DocumentHighlight[] | undefined + ) => { this.clear_markers(); if (!items) { @@ -116,7 +120,7 @@ export class HighlightsCM extends CodeMirrorIntegration { }; protected create_debouncer() { - return new Debouncer( + return new Debouncer( this.on_cursor_activity, this.settings.composite.debouncerDelay ); diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 223085457..9d3827198 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -116,19 +116,20 @@ function to_markup( } export class HoverCM extends CodeMirrorIntegration { - protected last_hover_character: IRootPosition; - private last_hover_response: lsProtocol.Hover; - protected hover_marker: CodeMirror.TextMarker; + protected last_hover_character: IRootPosition | null = null; + private last_hover_response: lsProtocol.Hover | null; + protected hover_marker: CodeMirror.TextMarker | null = null; private virtual_position: IVirtualPosition; protected cache: ResponseCache; - private debounced_get_hover: Throttler>; + private debounced_get_hover: Throttler>; private tooltip: FreeTooltip; - private _previousHoverRequest: Promise> | null; + private _previousHoverRequest: Promise< + Promise + > | null = null; constructor(options: IEditorIntegrationOptions) { super(options); - this._previousHoverRequest = null; } protected get modifierKey(): ModifierKey { @@ -152,7 +153,7 @@ export class HoverCM extends CodeMirrorIntegration { if (cache_item.document !== document) { return false; } - let range = cache_item.response.range; + let range = cache_item.response.range!; return ( line >= range.start.line && line <= range.end.line && @@ -179,7 +180,7 @@ export class HoverCM extends CodeMirrorIntegration { // we simulate the mouseleave for the editor in wrapper's mousemove; // this is used to hide the tooltip on leaving cells in notebook this.updateUnderlineAndTooltip(event) - .then(keep_tooltip => { + ?.then(keep_tooltip => { if (!keep_tooltip) { this.maybeHideTooltip(event); } @@ -213,7 +214,7 @@ export class HoverCM extends CodeMirrorIntegration { protected onKeyDown = (event: KeyboardEvent) => { if ( getModifierState(event, this.modifierKey) && - typeof this.last_hover_character !== 'undefined' + this.last_hover_character !== null ) { // does not need to be shown if it is already visible (otherwise we would be creating an identical tooltip again!) if (this.tooltip && this.tooltip.isVisible && !this.tooltip.isDisposed) { @@ -249,7 +250,7 @@ export class HoverCM extends CodeMirrorIntegration { } protected create_throttler() { - return new Throttler>(this.on_hover, { + return new Throttler>(this.on_hover, { limit: this.settings.composite.throttlerDelay, edge: 'trailing' }); @@ -259,7 +260,7 @@ export class HoverCM extends CodeMirrorIntegration { super.afterChange(change, root_position); // reset cache on any change in the document this.cache.clean(); - this.last_hover_character = undefined; + this.last_hover_character = null; this.remove_range_highlight(); } @@ -383,7 +384,7 @@ export class HoverCM extends CodeMirrorIntegration { // if over an empty space in a line (and not over a token) then not worth checking if (target.classList.contains('CodeMirror-line')) { this.remove_range_highlight(); - return; + return false; } const show_tooltip = getModifierState(event, this.modifierKey); @@ -398,7 +399,7 @@ export class HoverCM extends CodeMirrorIntegration { // and because some regions of the editor (between lines) have no characters if (root_position == null) { this.remove_range_highlight(); - return; + return false; } let token = this.virtual_editor.getTokenAt(root_position); @@ -411,10 +412,13 @@ export class HoverCM extends CodeMirrorIntegration { !this.is_event_inside_visible(event) ) { this.remove_range_highlight(); - return; + return false; } - if (!is_equal(root_position, this.last_hover_character)) { + if ( + this.last_hover_character && + !is_equal(root_position, this.last_hover_character) + ) { let virtual_position = this.virtual_editor.root_position_to_virtual_position(root_position); this.virtual_position = virtual_position; @@ -441,11 +445,11 @@ export class HoverCM extends CodeMirrorIntegration { if (this._previousHoverRequest === promise) { this._previousHoverRequest = null; } - if (this.is_useful_response(response)) { + if (response && this.is_useful_response(response)) { let ce_editor = this.virtual_editor.get_editor_at_root_position(root_position); let cm_editor = - this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); + this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor)!; let editor_range = this.get_editor_range( response, @@ -468,7 +472,7 @@ export class HoverCM extends CodeMirrorIntegration { this.cache.store(response_data); } else { this.remove_range_highlight(); - return; + return false; } } @@ -497,8 +501,8 @@ export class HoverCM extends CodeMirrorIntegration { if (this.hover_marker) { this.hover_marker.clear(); this.hover_marker = null; - this.last_hover_response = undefined; - this.last_hover_character = undefined; + this.last_hover_response = null; + this.last_hover_character = null; } }; @@ -507,12 +511,6 @@ export class HoverCM extends CodeMirrorIntegration { this.remove_range_highlight(); this.debounced_get_hover.dispose(); super.remove(); - - // just to be sure - this.debounced_get_hover = null; - this.remove_range_highlight = null; - this.handleResponse = null; - this.on_hover = null; } private get_editor_range( @@ -558,7 +556,7 @@ export class HoverCM extends CodeMirrorIntegration { this.virtual_editor.transform_from_editor_to_root( ce_editor, editor_range.start - ) + )! ) ), end: PositionConverter.cm_to_lsp( @@ -566,7 +564,7 @@ export class HoverCM extends CodeMirrorIntegration { this.virtual_editor.transform_from_editor_to_root( ce_editor, editor_range.end - ) + )! ) ) } diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 0e2f13fb1..3c72b80aa 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -140,6 +140,13 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { // if in current file, transform from the position within virtual document to the editor position: let editor_position = this.virtual_editor.transform_virtual_to_editor(virtual_position); + if (editor_position === null) { + this.console.warn( + 'Could not jump: conversion from virtual position to editor position failed', + virtual_position + ); + return; + } let editor_position_ce = PositionConverter.cm_to_ce(editor_position); this.console.log(`Jumping to ${editor_index}th editor of ${uri}`); this.console.log('Jump target within editor:', editor_position_ce); @@ -176,6 +183,11 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { contents_path = decodeURI(uri.slice(7)); } + if (contents_path === null) { + this.console.warn('contents_path could not be resolved'); + return; + } + try { await this.jumper.document_manager.services.contents.get( contents_path, @@ -238,7 +250,7 @@ class JumperLabIntegration implements IFeatureLabIntegration { get jumper(): CodeJumper { let current = this.adapterManager.currentAdapter.widget.id; - return this.jumpers.get(current); + return this.jumpers.get(current)!; } } @@ -247,14 +259,15 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ id: 'jump-to-definition', execute: async ({ connection, virtual_position, document, features }) => { const jump_feature = features.get(FEATURE_ID) as CMJumpToDefinition; - const targets = await connection.getDefinition( + const targets = await connection?.getDefinition( virtual_position, document.document_info, false ); await jump_feature.handle_jump(targets, document.document_info.uri); }, - is_enabled: ({ connection }) => connection.isDefinitionSupported(), + is_enabled: ({ connection }) => + connection ? connection.isDefinitionSupported() : false, label: trans.__('Jump to definition'), icon: jumpToIcon }, @@ -264,7 +277,8 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ const jump_feature = features.get(FEATURE_ID) as CMJumpToDefinition; jump_feature.jumper.global_jump_back(); }, - is_enabled: ({ connection }) => connection.isDefinitionSupported(), + is_enabled: ({ connection }) => + connection ? connection.isDefinitionSupported() : false, label: trans.__('Jump back'), icon: jumpBackIcon, // do not attach to any of the context menus diff --git a/packages/jupyterlab-lsp/src/features/rename.ts b/packages/jupyterlab-lsp/src/features/rename.ts index 6a71bff99..c4c10f2a3 100644 --- a/packages/jupyterlab-lsp/src/features/rename.ts +++ b/packages/jupyterlab-lsp/src/features/rename.ts @@ -49,8 +49,8 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ virtual_position ); let old_value = editor.get_token_at(root_position).value; - let handle_failure = (error: any) => { - let status = ''; + let handle_failure = (error: Error) => { + let status: string | null = ''; if (features.has(DIAGNOSTICS_PLUGIN_ID)) { let diagnostics_feature = features.get( @@ -80,27 +80,32 @@ const COMMANDS = (trans: TranslationBundle): IFeatureCommand[] => [ }); try { - if (dialog_value.button.accept != true) { - // the user has cancelled the rename action + const new_value = dialog_value.value; + if (dialog_value.button.accept != true || new_value == null) { + // the user has cancelled the rename action or did not provide new value return; } - let new_value = dialog_value.value; rename_feature.setStatus( trans.__('Renaming %1 to %2...', old_value, new_value), 2 * 1000 ); - const edit = await connection.rename( + const edit = await connection?.rename( virtual_position, document.document_info, new_value, false ); - await rename_feature.handleRename(edit, old_value, new_value); + if (edit) { + await rename_feature.handleRename(edit, old_value, new_value); + } else { + handle_failure(new Error('no edit from server')); + } } catch (error) { handle_failure(error); } }, - is_enabled: ({ connection }) => connection.isRenameSupported(), + is_enabled: ({ connection }) => + connection ? connection.provides('renameProvider') : false, label: trans.__('Rename symbol'), icon: renameIcon } @@ -126,7 +131,7 @@ export class RenameCM extends CodeMirrorIntegration { outcome = await this.apply_edit(workspaceEdit); } catch (error) { this.status_message.set(this.trans.__('Rename failed: %1', error)); - return outcome; + return; } try { @@ -142,7 +147,7 @@ export class RenameCM extends CodeMirrorIntegration { status = this.trans._n( 'Renamed %2 in %3 place', 'Renamed %2 in %3 places', - outcome.appliedChanges, + outcome.appliedChanges!, change_text, outcome.appliedChanges ); @@ -179,11 +184,11 @@ export class RenameCM extends CodeMirrorIntegration { * and provides a nice error message to the user. */ static ux_workaround_for_rope_limitation( - error: any, + error: Error, diagnostics_feature: DiagnosticsCM, editor: CodeMirrorVirtualEditor, rename_feature: RenameCM - ): string { + ): string | null { let has_index_error = false; try { has_index_error = error.message.includes('IndexError'); diff --git a/packages/jupyterlab-lsp/src/features/signature.spec.ts b/packages/jupyterlab-lsp/src/features/signature.spec.ts index 0cb7064c6..10eddacbb 100644 --- a/packages/jupyterlab-lsp/src/features/signature.spec.ts +++ b/packages/jupyterlab-lsp/src/features/signature.spec.ts @@ -10,7 +10,7 @@ describe('Signature', () => { const split = extractLead( ['This function does foo', '', 'But there are more details'], 1 - ); + )!; expect(split.lead).to.equal('This function does foo'); expect(split.remainder).to.equal('But there are more details'); }); @@ -36,7 +36,7 @@ describe('Signature', () => { 'But there are more details' ], 2 - ); + )!; expect(split.lead).to.equal('This function does foo,\nand it does bar'); expect(split.remainder).to.equal('But there are more details'); }); @@ -66,7 +66,7 @@ describe('Signature', () => { parameters: [ { label: 'text', - documentation: null + documentation: undefined } ], activeParameter: 0 @@ -91,7 +91,7 @@ describe('Signature', () => { parameters: [ { label: 'text', - documentation: null + documentation: undefined } ], activeParameter: 0 @@ -116,7 +116,7 @@ describe('Signature', () => { parameters: [ { label: 'text', - documentation: null + documentation: undefined } ], activeParameter: 0 diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index 07e02c2f3..472926a6e 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -68,7 +68,7 @@ export function extractLead(lines: string[], size: number): ISplit | null { */ export function signatureToMarkdown( item: lsProtocol.SignatureInformation, - language: string, + language: string = '', codeHighlighter: ( source: string, variable: string, @@ -78,7 +78,7 @@ export function signatureToMarkdown( activeParameterFallback?: number | null, maxLinesBeforeCollapse: number = 4 ): string { - const activeParameter: number | null = + const activeParameter: number | undefined | null = typeof item.activeParameter !== 'undefined' ? item.activeParameter : activeParameterFallback; @@ -135,7 +135,7 @@ export function signatureToMarkdown( '\n\n' + item.parameters .filter(parameter => parameter.documentation) - .map(parameter => '- ' + getMarkdown(parameter.documentation)) + .map(parameter => '- ' + getMarkdown(parameter.documentation!)) .join('\n'); } if (details) { @@ -207,7 +207,7 @@ export class SignatureCM extends CodeMirrorIntegration { } else { // otherwise, update the signature as the active parameter could have changed, // or the server may want us to close the tooltip - this.requestSignature(newRootPosition, previousPosition).catch( + this.requestSignature(newRootPosition, previousPosition)?.catch( this.console.warn ); } @@ -219,7 +219,7 @@ export class SignatureCM extends CodeMirrorIntegration { protected get_markup_for_signature_help( response: lsProtocol.SignatureHelp, - language: string + language: string = '' ): lsProtocol.MarkupContent { let signatures = new Array(); @@ -403,7 +403,7 @@ export class SignatureCM extends CodeMirrorIntegration { return; } - this.requestSignature(root_position, previousPosition).catch( + this.requestSignature(root_position, previousPosition)?.catch( this.console.warn ); } @@ -473,7 +473,10 @@ export const SIGNATURE_PLUGIN: JupyterFrontEndPlugin = { renderMimeRegistry: IRenderMimeRegistry, codeMirror: ICodeMirror ) => { - const settings = new FeatureSettings(settingRegistry, FEATURE_ID); + const settings = new FeatureSettings( + settingRegistry, + FEATURE_ID + ); const labIntegration = new SignatureLabIntegration( app, settings, diff --git a/packages/jupyterlab-lsp/src/index.ts b/packages/jupyterlab-lsp/src/index.ts index e2f0305ba..b525c6ee6 100644 --- a/packages/jupyterlab-lsp/src/index.ts +++ b/packages/jupyterlab-lsp/src/index.ts @@ -104,7 +104,9 @@ export class FeatureManager implements ILSPFeatureManager { } this.command_manager_registered.connect( (feature_manager, command_manager) => { - command_manager.add(options.feature.commands); + if (options.feature.commands) { + command_manager.add(options.feature.commands); + } } ); } @@ -114,7 +116,7 @@ export class FeatureManager implements ILSPFeatureManager { if (options.feature.settings && options.feature.settings.ready) { options.feature.settings.ready .then(() => { - if (!options.feature.settings.composite.disable) { + if (!options.feature?.settings?.composite.disable) { this._register(options); } else { console.log('Skipping ', options.feature.id, 'as disabled'); @@ -163,7 +165,7 @@ export class LSPExtension implements ILSPExtension { private code_overrides_manager: ILSPCodeOverridesManager, public console: ILSPLogConsole, public translator: ITranslator, - public user_console: ILoggerRegistry, + public user_console: ILoggerRegistry | null, status_bar: IStatusBar | null ) { const trans = (translator || nullTranslator).load('jupyterlab_lsp'); @@ -198,7 +200,7 @@ export class LSPExtension implements ILSPExtension { this.setting_registry .load(plugin.id) .then(settings => { - const options = settings.composite as LanguageServer; + const options = settings.composite as Required; // Store the initial server settings, to be sent asynchronously // when the servers are initialized. const initial_configuration = (options.language_servers || @@ -248,7 +250,7 @@ export class LSPExtension implements ILSPExtension { } private updateOptions(settings: ISettingRegistry.ISettings) { - const options = settings.composite as LanguageServer; + const options = settings.composite as Required; const languageServerSettings = (options.language_servers || {}) as TLanguageServerConfigurations; diff --git a/packages/jupyterlab-lsp/src/manager.ts b/packages/jupyterlab-lsp/src/manager.ts index ff2b70508..b4bbf423e 100644 --- a/packages/jupyterlab-lsp/src/manager.ts +++ b/packages/jupyterlab-lsp/src/manager.ts @@ -34,7 +34,7 @@ export class LanguageServerManager implements ILanguageServerManager { this._baseUrl = options.baseUrl || PageConfig.getBaseUrl(); this._retries = options.retries || 2; this._retriesInterval = options.retriesInterval || 10000; - this._statusCode = null; + this._statusCode = -1; this._configuration = {}; this.console = options.console; this.fetchSessions().catch(console.warn); @@ -87,9 +87,9 @@ export class LanguageServerManager implements ILanguageServerManager { ) { // most things speak language // if language is not known, it is guessed based on MIME type earlier - // so some language should be available by now (which can be not obvious, e.g. "plain" for txt documents) - const lowerCaseLanguage = options.language.toLocaleLowerCase(); - return spec.languages.some( + // so some language should be available by now (which can be not so obvious, e.g. "plain" for txt documents) + const lowerCaseLanguage = options.language!.toLocaleLowerCase(); + return spec.languages!.some( language => language.toLocaleLowerCase() == lowerCaseLanguage ); } diff --git a/packages/jupyterlab-lsp/src/overrides/maps.ts b/packages/jupyterlab-lsp/src/overrides/maps.ts index 4ef4126a0..ee7e51be1 100644 --- a/packages/jupyterlab-lsp/src/overrides/maps.ts +++ b/packages/jupyterlab-lsp/src/overrides/maps.ts @@ -28,7 +28,11 @@ export class ReversibleOverridesMap extends OverridesMap { } get reverse(): OverridesMap { - return this.type(this.overrides.map(override => override.reverse)); + return this.type( + this.overrides + .filter(override => override.reverse != null) + .map(override => override.reverse!) + ); } type(overrides: ICodeOverride[]) { diff --git a/packages/jupyterlab-lsp/src/tokens.ts b/packages/jupyterlab-lsp/src/tokens.ts index 2790c00bc..2703fdac7 100644 --- a/packages/jupyterlab-lsp/src/tokens.ts +++ b/packages/jupyterlab-lsp/src/tokens.ts @@ -186,7 +186,7 @@ export interface ILSPVirtualEditorManager { */ findBestImplementation( editors: CodeEditor.IEditor[] - ): IVirtualEditorType; + ): IVirtualEditorType | null; } /** diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts index 5c1a4eaa2..c1167d769 100644 --- a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts +++ b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts @@ -14,14 +14,14 @@ describe('rpy2 IPython overrides', () => { overrides.filter(override => override.scope == 'cell') ); it('overrides cell magic', () => { - let override = cell_magics.override_for(R_CELL_MAGIC); + let override = cell_magics.override_for(R_CELL_MAGIC)!; expect(override).to.equal( 'rpy2.ipython.rmagic.RMagics.R("""print(1)\n""", "")' ); let reverse = cell_magics.reverse.override_for(override); expect(reverse).to.equal(R_CELL_MAGIC); - override = cell_magics.override_for('%%R -i x -o y\nsome\ncode'); + override = cell_magics.override_for('%%R -i x -o y\nsome\ncode')!; expect(override).to.equal( 'y = rpy2.ipython.rmagic.RMagics.R("""some\ncode""", "", x)' ); @@ -37,7 +37,7 @@ describe('rpy2 IPython overrides', () => { it('works with other Rdevice', () => { let line = '%Rdevice svg'; - let override = line_magics.override_for(line); + let override = line_magics.override_for(line)!; expect(override).to.equal( 'rpy2.ipython.rmagic.RMagics.Rdevice(" svg", "")' ); @@ -53,19 +53,19 @@ describe('rpy2 IPython overrides', () => { it('works with the short form arguments, inputs and outputs', () => { let line = '%R -i x'; - let override = line_magics.override_for(line); + let override = line_magics.override_for(line)!; expect(override).to.equal('rpy2.ipython.rmagic.RMagics.R("", "", x)'); let reverse = line_magics.reverse.override_for(override); expect(reverse).to.equal(line); line = '%R -o x'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal('x = rpy2.ipython.rmagic.RMagics.R("", "")'); reverse = line_magics.reverse.override_for(override); expect(reverse).to.equal(line); line = '%R -i x command()'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal( 'rpy2.ipython.rmagic.RMagics.R(" command()", "", x)' ); @@ -73,7 +73,7 @@ describe('rpy2 IPython overrides', () => { expect(reverse).to.equal(line); line = '%R -i x -w 800 -h 400 command()'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal( 'rpy2.ipython.rmagic.RMagics.R(" command()", "-w 800 -h 400", x)' ); @@ -81,13 +81,13 @@ describe('rpy2 IPython overrides', () => { expect(reverse).to.equal(line); line = '%R -i x -o y -i z command()'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal( 'y = rpy2.ipython.rmagic.RMagics.R(" command()", "", x, z)' ); line = '%R -i x -i z -o y -o w command()'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal( 'y, w = rpy2.ipython.rmagic.RMagics.R(" command()", "", x, z)' ); @@ -103,14 +103,14 @@ describe('rpy2 IPython overrides', () => { it('works with the long form arguments', () => { let line = '%R --input x'; - let override = line_magics.override_for(line); + let override = line_magics.override_for(line)!; expect(override).to.equal('rpy2.ipython.rmagic.RMagics.R("", "", x)'); let reverse = line_magics.reverse.override_for(override); // TODO: make this preserve the long form expect(reverse).to.equal('%R -i x'); line = '%R --width 800 --height 400 command()'; - override = line_magics.override_for(line); + override = line_magics.override_for(line)!; expect(override).to.equal( 'rpy2.ipython.rmagic.RMagics.R(" command()", "--width 800 --height 400")' ); diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython/overrides.spec.ts b/packages/jupyterlab-lsp/src/transclusions/ipython/overrides.spec.ts index be9e0bf48..d485b2ada 100644 --- a/packages/jupyterlab-lsp/src/transclusions/ipython/overrides.spec.ts +++ b/packages/jupyterlab-lsp/src/transclusions/ipython/overrides.spec.ts @@ -39,7 +39,7 @@ describe('Default IPython overrides', () => { overrides.filter(override => override.scope == 'cell') ); it('overrides cell magics', () => { - let override = cell_magics_map.override_for(CELL_MAGIC_EXISTS); + let override = cell_magics_map.override_for(CELL_MAGIC_EXISTS)!; expect(override).to.equal( 'get_ipython().run_cell_magic("MAGIC", "", """some text\n""")' ); @@ -51,7 +51,7 @@ describe('Default IPython overrides', () => { it('works for empty cells', () => { // those are not correct syntax, but will happen when users are in the process of writing const cell_magic_with_args = '%%MAGIC\n'; - let override = cell_magics_map.override_for(cell_magic_with_args); + let override = cell_magics_map.override_for(cell_magic_with_args)!; expect(override).to.equal( 'get_ipython().run_cell_magic("MAGIC", "", """""")' ); @@ -62,7 +62,7 @@ describe('Default IPython overrides', () => { it('escapes arguments in the first line', () => { const cell_magic_with_args = '%%MAGIC "arg in quotes"\ntext'; - let override = cell_magics_map.override_for(cell_magic_with_args); + let override = cell_magics_map.override_for(cell_magic_with_args)!; expect(override).to.equal( 'get_ipython().run_cell_magic("MAGIC", " \\"arg in quotes\\"", """text""")' ); @@ -72,7 +72,7 @@ describe('Default IPython overrides', () => { }); it('escapes docstrings properly', () => { - let override = cell_magics_map.override_for(CELL_MAGIC_WITH_DOCSTRINGS); + let override = cell_magics_map.override_for(CELL_MAGIC_WITH_DOCSTRINGS)!; expect(override).to.equal( 'get_ipython().run_cell_magic("MAGIC", "", """' + ESCAPED_TEXT_WITH_DOCSTRINGS + @@ -97,7 +97,7 @@ describe('Default IPython overrides', () => { overrides.filter(override => override.scope == 'line') ); it('overrides line magics', () => { - let override = line_magics_map.override_for(LINE_MAGIC_WITH_SPACE); + let override = line_magics_map.override_for(LINE_MAGIC_WITH_SPACE)!; expect(override).to.equal( 'get_ipython().run_line_magic("MAGIC", " line = dd")' ); @@ -134,7 +134,7 @@ describe('Default IPython overrides', () => { it('escapes arguments', () => { let line_magic_with_args = '%MAGIC "arg"'; - let override = line_magics_map.override_for(line_magic_with_args); + let override = line_magics_map.override_for(line_magic_with_args)!; expect(override).to.equal( 'get_ipython().run_line_magic("MAGIC", " \\"arg\\"")' ); @@ -143,7 +143,7 @@ describe('Default IPython overrides', () => { expect(reverse).to.equal(line_magic_with_args); line_magic_with_args = '%MAGIC "arg\\"'; - override = line_magics_map.override_for(line_magic_with_args); + override = line_magics_map.override_for(line_magic_with_args)!; expect(override).to.equal( 'get_ipython().run_line_magic("MAGIC", " \\"arg\\\\\\"")' ); @@ -153,8 +153,8 @@ describe('Default IPython overrides', () => { }); it('overrides shell commands', () => { - let override = line_magics_map.override_for('!ls -o'); - expect(override).to.equal('get_ipython().getoutput("ls -o")'); + let override = line_magics_map.override_for('!ls -o')!; + expect(override).to.equal('get_ipython().getoutput("ls -o")')!; let reverse = line_magics_map.reverse.override_for(override); expect(reverse).to.equal('!ls -o'); @@ -185,13 +185,13 @@ describe('Default IPython overrides', () => { ); it('overrides pinfo', () => { - let override = line_magics_map.override_for('?int'); + let override = line_magics_map.override_for('?int')!; expect(override).to.equal("get_ipython().run_line_magic('pinfo', 'int')"); let reverse = line_magics_map.reverse.override_for(override); expect(reverse).to.equal('?int'); - override = line_magics_map.override_for('int?'); + override = line_magics_map.override_for('int?')!; expect(override).to.equal( "get_ipython().run_line_magic('pinfo', 'int')" ); @@ -201,7 +201,7 @@ describe('Default IPython overrides', () => { }); it('overrides pinfo2', () => { - let override = line_magics_map.override_for('??int'); + let override = line_magics_map.override_for('??int')!; expect(override).to.equal( "get_ipython().run_line_magic('pinfo2', 'int')" ); @@ -209,7 +209,7 @@ describe('Default IPython overrides', () => { let reverse = line_magics_map.reverse.override_for(override); expect(reverse).to.equal('??int'); - override = line_magics_map.override_for('int??'); + override = line_magics_map.override_for('int??')!; expect(override).to.equal( "get_ipython().run_line_magic('pinfo2', 'int')" ); @@ -217,7 +217,7 @@ describe('Default IPython overrides', () => { reverse = line_magics_map.reverse.override_for(override); expect(reverse).to.equal('int??'); - override = line_magics_map.override_for('some_func??'); + override = line_magics_map.override_for('some_func??')!; expect(override).to.equal( "get_ipython().run_line_magic('pinfo2', 'some_func')" ); diff --git a/packages/jupyterlab-lsp/src/utils.ts b/packages/jupyterlab-lsp/src/utils.ts index 98682b26c..412f6e2c3 100644 --- a/packages/jupyterlab-lsp/src/utils.ts +++ b/packages/jupyterlab-lsp/src/utils.ts @@ -103,7 +103,7 @@ export class DefaultMap extends Map { get_or_create(k: K, ...args: any[]): V { if (this.has(k)) { - return super.get(k); + return super.get(k)!; } else { let v = this.default_factory(k, ...args); this.set(k, v); diff --git a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts index 613b3827c..d430aa10d 100644 --- a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts @@ -59,7 +59,7 @@ class DocDispatcher implements CodeMirror.Doc { // TODO: edgecase: from and to in different cells let ce_editor = this.virtual_editor.virtual_document.get_editor_at_source_line(from); - let cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor); + let cm_editor = this.virtual_editor.ce_editor_to_cm_editor.get(ce_editor)!; let notebook_map = this.virtual_editor; return cm_editor .getDoc() @@ -73,7 +73,8 @@ class DocDispatcher implements CodeMirror.Doc { getCursor(start?: string): CodeMirror.Position { let active_editor = this.adapter.activeEditor as CodeMirrorEditor; if (active_editor == null) { - return; + // TODO + return undefined as any; } let cursor = active_editor.editor .getDoc() @@ -81,7 +82,7 @@ class DocDispatcher implements CodeMirror.Doc { return this.virtual_editor.transform_from_editor_to_root( active_editor, cursor - ); + )!; } } /** @@ -183,14 +184,14 @@ export class CodeMirrorVirtualEditor this.virtual_document.dispose(); // just to be sure - this.virtual_document = null; - this.code_extractors = null; + this.virtual_document = null as any; + this.code_extractors = null as any; this.isDisposed = true; // just to be sure - this.forEveryBlockEditor = null; - this._proxy = null; + this.forEveryBlockEditor = null as any; + this._proxy = null as any; } get_cursor_position(): IRootPosition { @@ -233,8 +234,14 @@ export class CodeMirrorVirtualEditor this.change.emit(change as IEditorChange); } - window_coords_to_root_position(coordinates: IWindowCoordinates) { - return this.coordsChar(coordinates, 'window') as IRootPosition; + window_coords_to_root_position( + coordinates: IWindowCoordinates + ): IRootPosition | null { + const position = this.coordsChar(coordinates, 'window'); + if (position.line === -1 && position.ch === -1) { + return null; + } + return position; } get_token_at(position: IRootPosition): CodeEditor.IToken { @@ -242,7 +249,7 @@ export class CodeMirrorVirtualEditor return { value: token.string, offset: token.start, - type: token.type + type: token.type || '' }; } @@ -250,7 +257,9 @@ export class CodeMirrorVirtualEditor return this.get_editor_at_root_line(position); } - transform_virtual_to_editor(position: IVirtualPosition): IEditorPosition { + transform_virtual_to_editor( + position: IVirtualPosition + ): IEditorPosition | null { return this.virtual_document.transform_virtual_to_editor(position); } @@ -316,12 +325,12 @@ export class CodeMirrorVirtualEditor let wrapped_handler = this._event_wrappers.get([eventName, handler]); this.forEveryBlockEditor(cm_editor => { - cm_editor.off(eventName as EventName2, wrapped_handler); + cm_editor.off(eventName as EventName2, wrapped_handler!); }); } find_ce_editor(cm_editor: CodeMirror.Editor): CodeEditor.IEditor { - return this.cm_editor_to_ce_editor.get(cm_editor); + return this.cm_editor_to_ce_editor.get(cm_editor)!; } transform_from_editor_to_root( @@ -332,7 +341,7 @@ export class CodeMirrorVirtualEditor this.console.warn('Editor not found in editor_to_source_line map'); return null; } - let shift = this.editor_to_source_line.get(editor); + let shift = this.editor_to_source_line.get(editor)!; return { ...(position as CodeMirror.Position), line: position.line + shift @@ -355,7 +364,7 @@ export class CodeMirrorVirtualEditor where: string, _class: string ): CodeMirror.LineHandle { - return undefined; + return undefined as any; } addLineWidget( @@ -363,7 +372,7 @@ export class CodeMirrorVirtualEditor node: HTMLElement, options?: CodeMirror.LineWidgetOptions ): CodeMirror.LineWidget { - return undefined; + return undefined as any; } addOverlay(mode: any, options?: any): void { @@ -402,6 +411,7 @@ export class CodeMirrorVirtualEditor object: { left: number; top: number }, mode?: 'window' | 'page' | 'local' ): IRootPosition { + let bestGuess: IRootPosition | null = null; for (let editor of this.adapter.editors) { // TODO: use some more intelligent strategy to determine editors to test let cm_editor = editor as CodeMirrorEditor; @@ -411,8 +421,19 @@ export class CodeMirrorVirtualEditor continue; } - return this.transform_from_editor_to_root(editor, pos as IEditorPosition); + bestGuess = this.transform_from_editor_to_root( + editor, + pos as IEditorPosition + ); + break; + } + if (bestGuess == null) { + return { + line: -1, + ch: -1 + } as IRootPosition; } + return bestGuess; } cursorCoords( @@ -427,7 +448,7 @@ export class CodeMirrorVirtualEditor where?: boolean | IRootPosition | null, mode?: 'window' | 'page' | 'local' ): { left: number; top: number; bottom: number } { - if (typeof where !== 'boolean') { + if (typeof where !== 'boolean' && where != null) { let editor = this.get_editor_at_root_line(where); return editor.cursorCoords(this.transform_from_root_to_editor(where)); } @@ -469,20 +490,17 @@ export class CodeMirrorVirtualEditor private get_editor_at_root_line(pos: IRootPosition): CodeMirror.Editor { let ce_editor = this.virtual_document.root.get_editor_at_source_line(pos); - return this.ce_editor_to_cm_editor.get(ce_editor); + return this.ce_editor_to_cm_editor.get(ce_editor)!; } getTokenAt(pos: IRootPosition, precise?: boolean): CodeMirror.Token { - if (pos === undefined) { - return; - } let editor = this.get_editor_at_root_line(pos); return editor.getTokenAt(this.transform_from_root_to_editor(pos)); } getTokenTypeAt(pos: IRootPosition): string { let ce_editor = this.virtual_document.get_editor_at_source_line(pos); - let cm_editor = this.ce_editor_to_cm_editor.get(ce_editor); + let cm_editor = this.ce_editor_to_cm_editor.get(ce_editor)!; return cm_editor.getTokenTypeAt(this.transform_from_root_to_editor(pos)); } @@ -537,9 +555,9 @@ export class CodeMirrorVirtualEditor forEveryBlockEditor( callback: (cm_editor: CodeMirror.Editor) => any, monitor_for_new_blocks = true, - on_editor_removed_callback: ( - cm_editor: CodeMirror.Editor - ) => any | null = null + on_editor_removed_callback: + | ((cm_editor: CodeMirror.Editor) => any) + | null = null ) { const editors_with_handlers = new Set(); @@ -594,7 +612,7 @@ export class CodeMirrorVirtualEditor * @param cm_editor */ find_editor(cm_editor: CodeMirror.Editor) { - let ce_editor = this.cm_editor_to_ce_editor.get(cm_editor); + let ce_editor = this.cm_editor_to_ce_editor.get(cm_editor)!; return { index: this.adapter.get_editor_index(ce_editor), node: this.adapter.get_editor_wrapper(ce_editor) diff --git a/packages/jupyterlab-lsp/src/virtual/console.ts b/packages/jupyterlab-lsp/src/virtual/console.ts index f94e0ec17..3aa4b7e31 100644 --- a/packages/jupyterlab-lsp/src/virtual/console.ts +++ b/packages/jupyterlab-lsp/src/virtual/console.ts @@ -35,7 +35,7 @@ class FloatingConsole implements ILogConsoleImplementation { // likely to be replaced by JupyterLab console: https://github.com/jupyterlab/jupyterlab/pull/6833#issuecomment-543016425 private readonly element: HTMLElement; - constructor(scope: string[] = ['LSP'], element: HTMLElement = null) { + constructor(scope: string[] = ['LSP'], element: HTMLElement | null = null) { if (element) { this.element = element; } else { @@ -215,7 +215,7 @@ class ConsoleSingleton { this.implementation = new BrowserConsole(); } - this.verbosity = composite.loggingLevel; + this.verbosity = composite.loggingLevel!; this.changed.emit(); } } diff --git a/packages/jupyterlab-lsp/src/virtual/document.spec.ts b/packages/jupyterlab-lsp/src/virtual/document.spec.ts index 19f3869a7..46da25adc 100644 --- a/packages/jupyterlab-lsp/src/virtual/document.spec.ts +++ b/packages/jupyterlab-lsp/src/virtual/document.spec.ts @@ -91,7 +91,7 @@ describe('VirtualDocument', () => { it('joins non-standalone fragments together', () => { let { cell_code_kept, foreign_document_map } = document.extract_foreign_code( - { value: R_LINE_MAGICS, ce_editor: null }, + { value: R_LINE_MAGICS, ce_editor: null as any }, { line: 0, column: 0 @@ -104,7 +104,7 @@ describe('VirtualDocument', () => { let { virtual_document: r_document } = foreign_document_map.get( foreign_document_map.keys().next().value - ); + )!; expect(r_document.language).to.equal('r'); expect(r_document.value).to.equal('df = data.frame()\n\n\nggplot(df)\n'); }); @@ -154,7 +154,7 @@ describe('VirtualDocument', () => { let editor_position = document.transform_virtual_to_editor({ line: 0, ch: 0 - } as IVirtualPosition); + } as IVirtualPosition)!; expect(editor_position.line).to.equal(0); expect(editor_position.ch).to.equal(0); @@ -162,7 +162,7 @@ describe('VirtualDocument', () => { editor_position = document.transform_virtual_to_editor({ line: 4, ch: 0 - } as IVirtualPosition); + } as IVirtualPosition)!; expect(editor_position.line).to.equal(0); expect(editor_position.ch).to.equal(0); }); @@ -198,7 +198,7 @@ describe('VirtualDocument', () => { // because it checks R source position, rather than checking root source positions. let editor_position = - foreign_document.transform_virtual_to_editor(virtual_r_1_1); + foreign_document.transform_virtual_to_editor(virtual_r_1_1)!; expect(editor_position.line).to.equal(1); expect(editor_position.ch).to.equal(4); @@ -207,7 +207,7 @@ describe('VirtualDocument', () => { editor_position = foreign_document.transform_virtual_to_editor({ line: 3, ch: 0 - } as IVirtualPosition); + } as IVirtualPosition)!; // 0th editor line is 'test line in Python 2\n' expect(editor_position.line).to.equal(1); // 1st editor lines is '%R 1st test line in R line magic 2' @@ -219,7 +219,7 @@ describe('VirtualDocument', () => { editor_position = foreign_document.transform_virtual_to_editor({ line: 6, ch: 36 - } as IVirtualPosition); + } as IVirtualPosition)!; // 0th editor line is 'test line in Python 3\n' expect(editor_position.line).to.equal(1); // 1st editor line is '%R -i imported_variable 1st test line in R line magic 3' @@ -231,7 +231,7 @@ describe('VirtualDocument', () => { editor_position = foreign_document.transform_virtual_to_editor({ line: 12, ch: 36 - } as IVirtualPosition); + } as IVirtualPosition)!; // 0th editor line is '%%R -i imported_variable\n' expect(editor_position.line).to.equal(1); // 1st editor line is '1st test line in R cell magic 2' diff --git a/packages/jupyterlab-lsp/src/virtual/document.ts b/packages/jupyterlab-lsp/src/virtual/document.ts index 99613f664..07575f7ce 100644 --- a/packages/jupyterlab-lsp/src/virtual/document.ts +++ b/packages/jupyterlab-lsp/src/virtual/document.ts @@ -32,7 +32,7 @@ interface IVirtualLine { /** * Where does the virtual line belongs to in the source document? */ - source_line: number; + source_line: number | null; editor: CodeEditor.IEditor; } @@ -132,7 +132,7 @@ export namespace VirtualDocument { foreign_code_extractors: IForeignCodeExtractorsRegistry; overrides_registry: ICodeOverridesRegistry; path: string; - file_extension: string; + file_extension: string | undefined; console: ILSPLogConsole; /** * Notebooks or any other aggregates of documents are not supported @@ -218,9 +218,9 @@ export class VirtualDocument { public changed: Signal; public path: string; - public file_extension: string; + public file_extension: string | undefined; public has_lsp_supported_file: boolean; - public parent?: VirtualDocument; + public parent?: VirtualDocument | null; private readonly options: VirtualDocument.IOptions; public update_manager: UpdateManager; @@ -251,7 +251,7 @@ export class VirtualDocument { this.source_lines = new Map(); this.foreign_documents = new Map(); this.overrides_registry = options.overrides_registry; - this.standalone = options.standalone; + this.standalone = options.standalone || false; this.instance_id = VirtualDocument.instances_count; VirtualDocument.instances_count += 1; this.unused_standalone_documents = new DefaultMap( @@ -289,14 +289,15 @@ export class VirtualDocument { this.unused_standalone_documents.clear(); this.virtual_lines.clear(); - // just to be sure - this.cell_magics_overrides = null; - this.document_info = null; - this.foreign_extractors = null; - this.foreign_extractors_registry = null; - this.line_magics_overrides = null; - this.line_blocks = null; - this.overrides_registry = null; + // just to be sure - if anything is accessed after disposal (it should not) we + // will get alterted by errors in the console AND this will limit memory leaks + this.cell_magics_overrides = null as any; + this.document_info = null as any; + this.foreign_extractors = null as any; + this.foreign_extractors_registry = null as any; + this.line_magics_overrides = null as any; + this.line_blocks = null as any; + this.overrides_registry = null as any; } /** @@ -415,7 +416,7 @@ export class VirtualDocument { } is_within_foreign(source_position: ISourcePosition): boolean { - let source_line = this.source_lines.get(source_position.line); + let source_line = this.source_lines.get(source_position.line)!; let source_position_ce: CodeEditor.IPosition = { line: source_line.editor_line, @@ -432,7 +433,7 @@ export class VirtualDocument { virtual_position_at_document( source_position: ISourcePosition ): IVirtualPosition { - let source_line = this.source_lines.get(source_position.line); + let source_line = this.source_lines.get(source_position.line)!; let virtual_line = source_line.virtual_line; // position inside the cell (block) @@ -474,7 +475,7 @@ export class VirtualDocument { // if not standalone, try to append to existing document let foreign_exists = this.foreign_documents.has(extractor.language); if (!extractor.standalone && foreign_exists) { - foreign_document = this.foreign_documents.get(extractor.language); + foreign_document = this.foreign_documents.get(extractor.language)!; this.unused_documents.delete(foreign_document); } else { // if standalone, try to re-use existing connection to the server @@ -482,7 +483,7 @@ export class VirtualDocument { extractor.language ); if (extractor.standalone && unused_standalone.length > 0) { - foreign_document = unused_standalone.pop(); + foreign_document = unused_standalone.pop()!; this.unused_documents.delete(foreign_document); } else { // if (previous document does not exists) or (extractor produces standalone documents @@ -521,8 +522,14 @@ export class VirtualDocument { for (let result of results) { if (result.foreign_code !== null) { + // result.range should only be null if result.foregin_code is null + if (result.range === null) { + this.console.warn( + 'Failure in foreign code extraction: `range` is null but `foreign_code` is not!' + ); + continue; + } let foreign_document = this.choose_foreign_document(extractor); - foreign_document_map.set(result.range, { virtual_line: foreign_document.last_virtual_line, virtual_document: foreign_document, @@ -538,7 +545,7 @@ export class VirtualDocument { ce_editor: block.ce_editor }, foreign_shift, - result.virtual_shift + result.virtual_shift! ); } if (result.host_code != null) { @@ -748,11 +755,7 @@ export class VirtualDocument { } transform_source_to_editor(pos: ISourcePosition): IEditorPosition { - if (pos == null) { - this.console.warn('no position available'); - return; - } - let source_line = this.source_lines.get(pos.line); + let source_line = this.source_lines.get(pos.line)!; let editor_line = source_line.editor_line; let editor_shift = source_line.editor_shift; return { @@ -764,17 +767,35 @@ export class VirtualDocument { } as IEditorPosition; } + /** + Can be null because some lines are added as padding/anchors + to the virtual document and those do not exist in the source document + and thus they are absent in the editor. + */ transform_virtual_to_editor( virtual_position: IVirtualPosition - ): IEditorPosition { + ): IEditorPosition | null { let source_position = this.transform_virtual_to_source(virtual_position); + if (source_position == null) { + return null; + } return this.transform_source_to_editor(source_position); } - transform_virtual_to_source(position: IVirtualPosition): ISourcePosition { + /** + Can be null because some lines are added as padding/anchors + to the virtual document and those do not exist in the source document. + */ + transform_virtual_to_source( + position: IVirtualPosition + ): ISourcePosition | null { + const line = this.virtual_lines.get(position.line)!.source_line; + if (line == null) { + return null; + } return { ch: position.ch, - line: this.virtual_lines.get(position.line).source_line + line: line } as ISourcePosition; } @@ -786,24 +807,16 @@ export class VirtualDocument { } get_editor_at_virtual_line(pos: IVirtualPosition): CodeEditor.IEditor { - if (pos == null) { - this.console.warn('no position available'); - return; - } let line = pos.line; // tolerate overshot by one (the hanging blank line at the end) if (!this.virtual_lines.has(line)) { line -= 1; } - return this.virtual_lines.get(line).editor; + return this.virtual_lines.get(line)!.editor; } get_editor_at_source_line(pos: ISourcePosition): CodeEditor.IEditor { - if (pos == null) { - this.console.warn('no position available'); - return; - } - return this.source_lines.get(pos.line).editor; + return this.source_lines.get(pos.line)!.editor; } /** diff --git a/packages/jupyterlab-lsp/src/virtual/editor.ts b/packages/jupyterlab-lsp/src/virtual/editor.ts index f7361ff5c..6d4d9863d 100644 --- a/packages/jupyterlab-lsp/src/virtual/editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/editor.ts @@ -100,7 +100,7 @@ export interface IVirtualEditor { */ window_coords_to_root_position( coordinates: IWindowCoordinates - ): IRootPosition; + ): IRootPosition | null; /** * Get the token at given root source position. @@ -152,7 +152,7 @@ export class VirtualEditorManager implements ILSPVirtualEditorManager { findBestImplementation( editors: CodeEditor.IEditor[] - ): IVirtualEditorType { + ): IVirtualEditorType | null { // for now, we check if all editors are of the same type, // but it could be good enough if majority of the editors // had the requested type diff --git a/packages/lsp-ws-connection/src/test/connection.test.ts b/packages/lsp-ws-connection/src/test/connection.test.ts index d2ca67c92..91c2beea4 100644 --- a/packages/lsp-ws-connection/src/test/connection.test.ts +++ b/packages/lsp-ws-connection/src/test/connection.test.ts @@ -81,12 +81,13 @@ class MockSocket implements EventTarget { /** * Sends a synthetic event to the client code, for example to imitate a server response */ - public dispatchEvent = (event: Event) => { + public dispatchEvent = (event: Event): boolean => { const listeners: Listener[] = this.listeners[event.type]; if (!listeners) { return false; } listeners.forEach(listener => listener.call(null, event)); + return true; }; constructor(url: string, protocols?: string[]) { diff --git a/packages/lsp-ws-connection/src/test/mock-connection.ts b/packages/lsp-ws-connection/src/test/mock-connection.ts index 40e38a347..4813eaba4 100644 --- a/packages/lsp-ws-connection/src/test/mock-connection.ts +++ b/packages/lsp-ws-connection/src/test/mock-connection.ts @@ -14,12 +14,13 @@ export class MockConnection implements ILspConnection { /** * Sends a synthetic event to the client code, for example to imitate a server response */ - public dispatchEvent = (event: MessageEvent) => { + public dispatchEvent = (event: MessageEvent): boolean => { const listeners = this.listeners[event.type]; if (!listeners) { return false; } listeners.forEach(listener => listener.call(null, event.data)); + return true; }; public sendInitialize = sinon.stub(); diff --git a/packages/lsp-ws-connection/src/types.ts b/packages/lsp-ws-connection/src/types.ts index 282c497fe..6bb0ff1ac 100644 --- a/packages/lsp-ws-connection/src/types.ts +++ b/packages/lsp-ws-connection/src/types.ts @@ -22,6 +22,7 @@ export type AnyLocation = | lsProtocol.Location | lsProtocol.Location[] | lsProtocol.LocationLink[] + | undefined | null; export type AnyCompletion = diff --git a/packages/lsp-ws-connection/src/ws-connection.ts b/packages/lsp-ws-connection/src/ws-connection.ts index 5e8a69d59..1138e2eb9 100644 --- a/packages/lsp-ws-connection/src/ws-connection.ts +++ b/packages/lsp-ws-connection/src/ws-connection.ts @@ -88,7 +88,7 @@ export class LspWsConnection this.serverCapabilities = registerServerCapability( this.serverCapabilities, capabilityRegistration - ); + )!; } catch (err) { console.error(err); } diff --git a/packages/tsconfigbase.json b/packages/tsconfigbase.json index 3c15fd7c5..398bfd68b 100644 --- a/packages/tsconfigbase.json +++ b/packages/tsconfigbase.json @@ -16,6 +16,7 @@ "sourceMap": true, "target": "es2017", "skipLibCheck": true, + "strictNullChecks": true, "types": [] } } From 49d1d22bd8c73d7a9e834b2b9be49e34ef3bee3b Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 26 Dec 2021 22:26:22 +0000 Subject: [PATCH 2/8] Enable `noImplicitThis` --- packages/jupyterlab-lsp/src/features/signature.ts | 5 +---- packages/tsconfigbase.json | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/signature.ts b/packages/jupyterlab-lsp/src/features/signature.ts index 472926a6e..92217cecc 100644 --- a/packages/jupyterlab-lsp/src/features/signature.ts +++ b/packages/jupyterlab-lsp/src/features/signature.ts @@ -123,10 +123,7 @@ export function signatureToMarkdown( } } else { if (item.documentation.kind !== 'markdown') { - this.console.warn( - 'Unknown MarkupContent kind:', - item.documentation.kind - ); + console.warn('Unknown MarkupContent kind:', item.documentation.kind); } details += item.documentation.value; } diff --git a/packages/tsconfigbase.json b/packages/tsconfigbase.json index 398bfd68b..eb261a588 100644 --- a/packages/tsconfigbase.json +++ b/packages/tsconfigbase.json @@ -17,6 +17,7 @@ "target": "es2017", "skipLibCheck": true, "strictNullChecks": true, + "noImplicitThis": true, "types": [] } } From 3ab3a8b8b16c1784cdbe4c597222d2190af7e985 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 26 Dec 2021 22:41:41 +0000 Subject: [PATCH 3/8] Fix hover character comparison logic, finish null position handling --- .../src/editor_integration/codemirror.ts | 11 ++++++++--- packages/jupyterlab-lsp/src/features/hover.ts | 2 +- packages/jupyterlab-lsp/src/features/jump_to.ts | 6 ++++++ .../jupyterlab-lsp/src/virtual/codemirror_editor.ts | 3 +++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts index 7743c8386..5b209bc1b 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/codemirror.ts @@ -186,14 +186,19 @@ export abstract class CodeMirrorIntegration }; } - protected position_from_mouse(event: MouseEvent): IRootPosition { - return this.virtual_editor.coordsChar( + protected position_from_mouse(event: MouseEvent): IRootPosition | null { + const position = this.virtual_editor.coordsChar( { left: event.clientX, top: event.clientY }, 'window' - ) as IRootPosition; + ); + if (position.line === -1 && position.ch === -1) { + return null; + } else { + return position as IRootPosition; + } } public transform_virtual_position_to_root_position( diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 9d3827198..3e7fe70ea 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -416,7 +416,7 @@ export class HoverCM extends CodeMirrorIntegration { } if ( - this.last_hover_character && + !this.last_hover_character || !is_equal(root_position, this.last_hover_character) ) { let virtual_position = diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 3c72b80aa..97edfd26f 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -63,6 +63,12 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { 'mousedown', (virtual_editor, event: MouseEvent) => { let root_position = this.position_from_mouse(event); + if (root_position == null) { + this.console.warn( + 'Could not retrieve root position from mouse event to jump to definition' + ); + return; + } let document = virtual_editor.document_at_root_position(root_position); let virtual_position = virtual_editor.root_position_to_virtual_position(root_position); diff --git a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts index d430aa10d..97917631a 100644 --- a/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts +++ b/packages/jupyterlab-lsp/src/virtual/codemirror_editor.ts @@ -407,6 +407,9 @@ export class CodeMirrorVirtualEditor } } + /** + * @note returns {line: -1, ch: -1} if position is outside of all editors + */ coordsChar( object: { left: number; top: number }, mode?: 'window' | 'page' | 'local' From dab96cefdafdd747a7f562cf6eb177bdbd55463e Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 27 Dec 2021 01:30:38 +0000 Subject: [PATCH 4/8] Only fetch position on actual user-commanded jump --- .../jupyterlab-lsp/src/features/jump_to.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 97edfd26f..22c11eae6 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -62,19 +62,20 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { this.editor_handlers.set( 'mousedown', (virtual_editor, event: MouseEvent) => { - let root_position = this.position_from_mouse(event); - if (root_position == null) { - this.console.warn( - 'Could not retrieve root position from mouse event to jump to definition' - ); - return; - } - let document = virtual_editor.document_at_root_position(root_position); - let virtual_position = - virtual_editor.root_position_to_virtual_position(root_position); - const { button } = event; if (button === 0 && getModifierState(event, this.modifierKey)) { + let root_position = this.position_from_mouse(event); + if (root_position == null) { + this.console.warn( + 'Could not retrieve root position from mouse event to jump to definition' + ); + return; + } + let document = + virtual_editor.document_at_root_position(root_position); + let virtual_position = + virtual_editor.root_position_to_virtual_position(root_position); + this.connection .getDefinition(virtual_position, document.document_info, false) .then(targets => { From 1e835b52aadfe9d25a27ee3e7e345331f528de45 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 27 Dec 2021 01:49:42 +0000 Subject: [PATCH 5/8] Handle an warning showing up on restoring workspace --- packages/jupyterlab-lsp/src/command_manager.ts | 14 ++++++++++++-- packages/jupyterlab-lsp/src/positioning.ts | 4 ++++ packages/jupyterlab-lsp/src/virtual/document.ts | 8 ++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/jupyterlab-lsp/src/command_manager.ts b/packages/jupyterlab-lsp/src/command_manager.ts index b758a7e96..79ccf7208 100644 --- a/packages/jupyterlab-lsp/src/command_manager.ts +++ b/packages/jupyterlab-lsp/src/command_manager.ts @@ -6,7 +6,7 @@ import { IDocumentWidget } from '@jupyterlab/docregistry'; import { WidgetAdapter } from './adapters/adapter'; import { LSPConnection } from './connection'; import { IFeatureCommand, IFeatureEditorIntegration } from './feature'; -import { IRootPosition, IVirtualPosition } from './positioning'; +import { IRootPosition, IVirtualPosition, PositionError } from './positioning'; import { ILSPAdapterManager, ILSPLogConsole } from './tokens'; import { VirtualDocument } from './virtual/document'; import { IVirtualEditor } from './virtual/editor'; @@ -184,7 +184,17 @@ export class ContextCommandManager extends LSPCommandManager { } } if (context == null) { - context = this.current_adapter?.context_from_active_document(); + try { + context = this.current_adapter?.context_from_active_document(); + } catch (e) { + if (e instanceof PositionError) { + this.console.log( + 'Could not get context from active document: it is expected when restoring workspace with open files' + ); + } else { + throw e; + } + } } return context; } diff --git a/packages/jupyterlab-lsp/src/positioning.ts b/packages/jupyterlab-lsp/src/positioning.ts index 352673e93..cdbc36964 100644 --- a/packages/jupyterlab-lsp/src/positioning.ts +++ b/packages/jupyterlab-lsp/src/positioning.ts @@ -64,3 +64,7 @@ export function offset_at_position( } return offset; } + +export class PositionError extends Error { + // no-op +} diff --git a/packages/jupyterlab-lsp/src/virtual/document.ts b/packages/jupyterlab-lsp/src/virtual/document.ts index 07575f7ce..218023105 100644 --- a/packages/jupyterlab-lsp/src/virtual/document.ts +++ b/packages/jupyterlab-lsp/src/virtual/document.ts @@ -13,7 +13,8 @@ import { ICodeOverridesRegistry } from '../overrides/tokens'; import { IEditorPosition, ISourcePosition, - IVirtualPosition + IVirtualPosition, + PositionError } from '../positioning'; import { ILSPLogConsole } from '../tokens'; import { DefaultMap, until_ready } from '../utils'; @@ -433,7 +434,10 @@ export class VirtualDocument { virtual_position_at_document( source_position: ISourcePosition ): IVirtualPosition { - let source_line = this.source_lines.get(source_position.line)!; + let source_line = this.source_lines.get(source_position.line); + if (source_line == null) { + throw new PositionError('Source line not mapped to virtual position'); + } let virtual_line = source_line.virtual_line; // position inside the cell (block) From b18afc2c53122e46f45a77fa60e247deaf3b452d Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 27 Dec 2021 02:23:33 +0000 Subject: [PATCH 6/8] Enable `strictBindCallApply` --- .../jupyterlab-lsp/src/features/diagnostics/listing.tsx | 7 +++++-- packages/tsconfigbase.json | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx b/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx index 963f47069..1a3583ef7 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx +++ b/packages/jupyterlab-lsp/src/features/diagnostics/listing.tsx @@ -40,7 +40,7 @@ export class DiagnosticsDatabase extends Map< IEditorDiagnostic[] > { get all(): Array { - return [].concat.apply([], this.values()); + return [].concat.apply([], this.values() as any); } } @@ -316,7 +316,10 @@ export class DiagnosticsListing extends VDomRenderer { }); } ); - let flattened: IDiagnosticsRow[] = [].concat.apply([], by_document); + let flattened: IDiagnosticsRow[] = ([] as IDiagnosticsRow[]).concat.apply( + [], + by_document + ); this._diagnostics = new Map(flattened.map(row => [row.key, row])); let sorted_column = this.columns.filter( diff --git a/packages/tsconfigbase.json b/packages/tsconfigbase.json index eb261a588..c7ae1d488 100644 --- a/packages/tsconfigbase.json +++ b/packages/tsconfigbase.json @@ -18,6 +18,7 @@ "skipLibCheck": true, "strictNullChecks": true, "noImplicitThis": true, + "strictBindCallApply": true, "types": [] } } From 1b9731e555bd8c8590cfb16bbd3901dfee44339f Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 27 Dec 2021 10:58:50 +0000 Subject: [PATCH 7/8] Enable `noFallthroughCasesInSwitch` and disallow `allowUnusedLabels` --- packages/tsconfigbase.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/tsconfigbase.json b/packages/tsconfigbase.json index c7ae1d488..18d6fc441 100644 --- a/packages/tsconfigbase.json +++ b/packages/tsconfigbase.json @@ -19,6 +19,8 @@ "strictNullChecks": true, "noImplicitThis": true, "strictBindCallApply": true, + "allowUnusedLabels": false, + "noFallthroughCasesInSwitch": true, "types": [] } } From 1b857b535f3de5157121b5f3b97881a5489a7c90 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 27 Dec 2021 11:12:53 +0000 Subject: [PATCH 8/8] Enable `noImplicitReturns` --- packages/jupyterlab-lsp/src/adapters/adapter.ts | 2 ++ .../src/editor_integration/editor_adapter.ts | 1 + .../src/features/diagnostics/diagnostics.ts | 1 + packages/jupyterlab-lsp/src/features/hover.ts | 1 + packages/jupyterlab-lsp/src/features/jump_to.ts | 10 ++++++++-- packages/tsconfigbase.json | 1 + 6 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/jupyterlab-lsp/src/adapters/adapter.ts b/packages/jupyterlab-lsp/src/adapters/adapter.ts index ecd5558d5..68f39c610 100644 --- a/packages/jupyterlab-lsp/src/adapters/adapter.ts +++ b/packages/jupyterlab-lsp/src/adapters/adapter.ts @@ -647,6 +647,8 @@ export abstract class WidgetAdapter { connection, virtual_document }; + } else { + return undefined; } } diff --git a/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts b/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts index 213130292..8aa299856 100644 --- a/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts +++ b/packages/jupyterlab-lsp/src/editor_integration/editor_adapter.ts @@ -88,6 +88,7 @@ export class EditorAdapter> { this.console.error(e); } this.invalidateLastChange(); + return undefined; } public invalidateLastChange() { diff --git a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts index 3f948c8de..75017e326 100644 --- a/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts +++ b/packages/jupyterlab-lsp/src/features/diagnostics/diagnostics.ts @@ -106,6 +106,7 @@ class DiagnosticsPanel { return column; } } + return undefined; }; /** Columns Menu **/ diff --git a/packages/jupyterlab-lsp/src/features/hover.ts b/packages/jupyterlab-lsp/src/features/hover.ts index 3e7fe70ea..1e7e31862 100644 --- a/packages/jupyterlab-lsp/src/features/hover.ts +++ b/packages/jupyterlab-lsp/src/features/hover.ts @@ -494,6 +494,7 @@ export class HoverCM extends CodeMirrorIntegration { ) { this.console.warn(e); } + return undefined; } }; diff --git a/packages/jupyterlab-lsp/src/features/jump_to.ts b/packages/jupyterlab-lsp/src/features/jump_to.ts index 22c11eae6..e061947ab 100644 --- a/packages/jupyterlab-lsp/src/features/jump_to.ts +++ b/packages/jupyterlab-lsp/src/features/jump_to.ts @@ -94,7 +94,7 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { get_uri_and_range(location_or_locations: AnyLocation) { if (location_or_locations == null) { - return; + return undefined; } // some language servers appear to return a single object const locations = Array.isArray(location_or_locations) @@ -105,7 +105,7 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { // (like when there are multiple definitions or usages) // could use the showHints() or completion frontend as a reference if (locations.length === 0) { - return; + return undefined; } this.console.log( @@ -125,6 +125,12 @@ export class CMJumpToDefinition extends CodeMirrorIntegration { uri: location_or_link.uri, range: location_or_link.range }; + } else { + this.console.warn( + 'Returned jump location is incorrect (no uri or targetUri):', + location_or_link + ); + return undefined; } } diff --git a/packages/tsconfigbase.json b/packages/tsconfigbase.json index 18d6fc441..3689fec8e 100644 --- a/packages/tsconfigbase.json +++ b/packages/tsconfigbase.json @@ -21,6 +21,7 @@ "strictBindCallApply": true, "allowUnusedLabels": false, "noFallthroughCasesInSwitch": true, + "noImplicitReturns": true, "types": [] } }