From 51fad385aeb2940dcb6db1a51eed048dbe8c8b48 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 25 May 2022 16:33:23 -0700 Subject: [PATCH] Turn off string filtering for completions (#10137) * Turn off string filtering for completions * Run test with jedi and without * Using Ian's setting name instead. Changed wording * Update message again and add link to kernel setting --- news/2 Fixes/8893.md | 1 + package.json | 8 +- .../pythonKernelCompletionProvider.ts | 5 - src/kernels/kernel.base.ts | 4 +- src/platform/common/configSettings.ts | 1 + src/platform/common/types.ts | 1 + .../completionProvider.vscode.test.ts | 442 ++++++++++-------- 7 files changed, 248 insertions(+), 214 deletions(-) create mode 100644 news/2 Fixes/8893.md diff --git a/news/2 Fixes/8893.md b/news/2 Fixes/8893.md new file mode 100644 index 00000000000..616cc486562 --- /dev/null +++ b/news/2 Fixes/8893.md @@ -0,0 +1 @@ +Fix to provide autocomplete inside of quoted strings. This fix also enabled a setting to allow the use of Jedi for completions in a kernel, but should be used with caution. Jedi can hang the kernel preventing exeuction from happening. diff --git a/package.json b/package.json index dbd2e13789e..87cb5a810d4 100644 --- a/package.json +++ b/package.json @@ -1985,7 +1985,7 @@ }, "jupyter.pythonCompletionTriggerCharacters": { "type": "string", - "default": ".%", + "default": ".%'\"", "description": "Characters which trigger auto completion on a python jupyter kernel.", "scope": "machine" }, @@ -2000,6 +2000,12 @@ "default": false, "description": "Add PYTHONNOUSERSITE to kernels before starting. This prevents global/user site-packages from being used in the PYTHONPATH of the kernel.", "scope": "machine" + }, + "jupyter.enableExtendedKernelCompletions": { + "type": "boolean", + "default": false, + "markdownDescription": "Enables Jedi support for extended IntelliSense completions in running Jupyter kernels (see this [setting](https://ipython.readthedocs.io/en/stable/config/options/terminal.html?highlight=use_jedi#configtrait-Completer.use_jedi)). This can greatly impact notebook cell execution performance. Use with caution.", + "scope": "machine" } } }, diff --git a/src/intellisense/pythonKernelCompletionProvider.ts b/src/intellisense/pythonKernelCompletionProvider.ts index d96d8bc3a94..3e8d5d6338d 100644 --- a/src/intellisense/pythonKernelCompletionProvider.ts +++ b/src/intellisense/pythonKernelCompletionProvider.ts @@ -281,11 +281,6 @@ export function filterCompletions( allowStringFilter && (triggerCharacter == "'" || triggerCharacter == '"' || positionInsideString(line, position)); - // If inside of a string, filter out everything except file names - if (insideString) { - result = result.filter((r) => r.itemText.includes('.') || r.itemText.endsWith('/')); - } - traceInfoIfCI(`Jupyter completions filtering applied: ${insideString} on ${line}`); // Update magics to have a much lower sort order than other strings. diff --git a/src/kernels/kernel.base.ts b/src/kernels/kernel.base.ts index 2a9833ee8af..1e9fc13cbf3 100644 --- a/src/kernels/kernel.base.ts +++ b/src/kernels/kernel.base.ts @@ -623,7 +623,9 @@ export abstract class BaseKernel implements IKernel { if (file) { result.push(`__vsc_ipynb_file__ = "${file.replace(/\\/g, '\\\\')}"`); } - result.push(CodeSnippets.DisableJedi); + if (!this.configService.getSettings(undefined).enableExtendedKernelCompletions) { + result.push(CodeSnippets.DisableJedi); + } // For Python notebook initialize matplotlib // Wrap this startup code in try except as it might fail diff --git a/src/platform/common/configSettings.ts b/src/platform/common/configSettings.ts index 24b1515a1e5..df2b3cbc980 100644 --- a/src/platform/common/configSettings.ts +++ b/src/platform/common/configSettings.ts @@ -101,6 +101,7 @@ export class JupyterSettings implements IWatchableJupyterSettings { public logKernelOutputSeparately: boolean = false; public poetryPath: string = ''; public excludeUserSitePackages: boolean = false; + public enableExtendedKernelCompletions: boolean = false; public variableTooltipFields: IVariableTooltipFields = { python: { diff --git a/src/platform/common/types.ts b/src/platform/common/types.ts index 00342ce2e2c..716e7e51b99 100644 --- a/src/platform/common/types.ts +++ b/src/platform/common/types.ts @@ -115,6 +115,7 @@ export interface IJupyterSettings { readonly logKernelOutputSeparately: boolean; readonly poetryPath: string; readonly excludeUserSitePackages: boolean; + readonly enableExtendedKernelCompletions: boolean; } export interface IVariableTooltipFields { diff --git a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts index 72114458c40..95f03d15bcf 100644 --- a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts +++ b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts @@ -16,6 +16,7 @@ import { TextDocument, Uri, workspace, + WorkspaceConfiguration, WorkspaceEdit } from 'vscode'; import { IVSCodeNotebook } from '../../../../platform/common/application/types'; @@ -41,222 +42,249 @@ import { import { Settings } from '../../../../platform/common/constants'; /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ -suite('DataScience - VSCode Intellisense Notebook - (Code Completion via Jupyter) (slow)', function () { - let api: IExtensionTestApi; - const disposables: IDisposable[] = []; - let vscodeNotebook: IVSCodeNotebook; - let completionProvider: PythonKernelCompletionProvider; - this.timeout(120_000); - let previousPythonCompletionTriggerCharactersValue: string | undefined; - suiteSetup(async function () { - traceInfo(`Start Suite Code Completion via Jupyter`); - this.timeout(120_000); - const jupyterConfig = workspace.getConfiguration('jupyter', undefined); - previousPythonCompletionTriggerCharactersValue = jupyterConfig.get('pythonCompletionTriggerCharacters'); - await jupyterConfig.update('pythonCompletionTriggerCharacters', '.%"\'', ConfigurationTarget.Global); - api = await initialize(); - if (IS_REMOTE_NATIVE_TEST()) { - // https://github.com/microsoft/vscode-jupyter/issues/6331 - return this.skip(); - } - await startJupyterServer(); - await prewarmNotebooks(); - sinon.restore(); - vscodeNotebook = api.serviceContainer.get(IVSCodeNotebook); - completionProvider = api.serviceContainer.get(PythonKernelCompletionProvider); - traceInfo(`Start Suite (Completed) Code Completion via Jupyter`); - }); - // Use same notebook without starting kernel in every single test (use one for whole suite). - setup(async function () { - traceInfo(`Start Test ${this.currentTest?.title}`); - sinon.restore(); - await startJupyterServer(); - await createEmptyPythonNotebook(disposables, Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'tmp'))); // TODO, can't do this on web tests - setIntellisenseTimeout(30000); - traceInfo(`Start Test (completed) ${this.currentTest?.title}`); - }); - teardown(async function () { - sinon.restore(); - traceInfo(`Ended Test ${this.currentTest?.title}`); - setIntellisenseTimeout(Settings.IntellisenseTimeout); - await closeNotebooksAndCleanUpAfterTests(disposables); - traceInfo(`Ended Test (completed) ${this.currentTest?.title}`); - }); - suiteTeardown(async () => { - await workspace - .getConfiguration('jupyter', undefined) - .update( - 'pythonCompletionTriggerCharacters', - previousPythonCompletionTriggerCharactersValue, - ConfigurationTarget.Global - ); - await closeNotebooksAndCleanUpAfterTests(disposables); - }); - /** - * Test completions. - * @param {string} cellCode e.g. `df.` - * @param {string} itemToExistInCompletion E.g. `Name`, `Age` - * @param {string} textToFilterCompletions The text typed to filter the list, e.g. `N`. - * @param {string} itemToExistInCompletionAfterFilter The filtered list, e.g. if user types `N`, then `Age` will not show up, but `Name` will. - */ - async function testCompletions( - cellCode: string, - triggerCharacter: string | undefined, - itemToNotExistInCompletion?: string, - itemToExistInCompletion?: string, - textToFilterCompletions?: string, - itemToExistInCompletionAfterFilter?: string - ) { - await insertCodeCell('%pip install pandas', { - index: 0 - }); - const cell = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(0)!; +[true, false].forEach((useJedi) => { + suite( + `DataScience - VSCode Intellisense Notebook - (Code Completion via Jupyter) (slow) ${ + useJedi ? 'withJedi' : 'withoutJedi' + }`, + function () { + let api: IExtensionTestApi; + const disposables: IDisposable[] = []; + let vscodeNotebook: IVSCodeNotebook; + let completionProvider: PythonKernelCompletionProvider; + this.timeout(120_000); + let previousPythonCompletionTriggerCharactersValue: string | undefined; + let jupyterConfig: WorkspaceConfiguration; + let previousJediSetting: boolean | undefined; - await runCell(cell); + suiteSetup(async function () { + traceInfo(`Start Suite Code Completion via Jupyter`); + this.timeout(120_000); + jupyterConfig = workspace.getConfiguration('jupyter', undefined); + previousPythonCompletionTriggerCharactersValue = jupyterConfig.get( + 'pythonCompletionTriggerCharacters' + ); + previousJediSetting = jupyterConfig.get('enableExtendedKernelCompletions'); + await jupyterConfig.update('enableExtendedKernelCompletions', useJedi, ConfigurationTarget.Global); + await jupyterConfig.update('pythonCompletionTriggerCharacters', '.%"\'', ConfigurationTarget.Global); + api = await initialize(); + if (IS_REMOTE_NATIVE_TEST()) { + // https://github.com/microsoft/vscode-jupyter/issues/6331 + return this.skip(); + } + await startJupyterServer(); + await prewarmNotebooks(); + sinon.restore(); + vscodeNotebook = api.serviceContainer.get(IVSCodeNotebook); + completionProvider = + api.serviceContainer.get(PythonKernelCompletionProvider); + traceInfo(`Start Suite (Completed) Code Completion via Jupyter`); + }); + // Use same notebook without starting kernel in every single test (use one for whole suite). + setup(async function () { + traceInfo(`Start Test ${this.currentTest?.title}`); + sinon.restore(); + await startJupyterServer(); + await createEmptyPythonNotebook(disposables, Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'tmp'))); // TODO, can't do this on web tests + setIntellisenseTimeout(30000); + traceInfo(`Start Test (completed) ${this.currentTest?.title}`); + }); + teardown(async function () { + sinon.restore(); + traceInfo(`Ended Test ${this.currentTest?.title}`); + setIntellisenseTimeout(Settings.IntellisenseTimeout); + await closeNotebooksAndCleanUpAfterTests(disposables); + traceInfo(`Ended Test (completed) ${this.currentTest?.title}`); + }); + suiteTeardown(async () => { + await workspace + .getConfiguration('jupyter', undefined) + .update( + 'pythonCompletionTriggerCharacters', + previousPythonCompletionTriggerCharactersValue, + ConfigurationTarget.Global + ); + await jupyterConfig.update( + 'enableExtendedKernelCompletions', + previousJediSetting, + ConfigurationTarget.Global + ); + await closeNotebooksAndCleanUpAfterTests(disposables); + }); + /** + * Test completions. + * @param {string} cellCode e.g. `df.` + * @param {string} itemToExistInCompletion E.g. `Name`, `Age` + * @param {string} textToFilterCompletions The text typed to filter the list, e.g. `N`. + * @param {string} itemToExistInCompletionAfterFilter The filtered list, e.g. if user types `N`, then `Age` will not show up, but `Name` will. + */ + async function testCompletions( + cellCode: string, + triggerCharacter: string | undefined, + itemToNotExistInCompletion?: string, + itemToExistInCompletion?: string, + textToFilterCompletions?: string, + itemToExistInCompletionAfterFilter?: string + ) { + await insertCodeCell('%pip install pandas', { + index: 0 + }); + const cell = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(0)!; - const namesCsvPath = path.join(__dirname, 'names.csv').replace(/\\/g, '/').replace('out', 'src'); + await runCell(cell); - // Wait till execution count changes and status is success. - await waitForExecutionCompletedSuccessfully(cell); - await insertCodeCell(`import pandas as pd\ndf = pd.read_csv("${namesCsvPath}")\n`, { - index: 1 - }); - const cell2 = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(1)!; + const namesCsvPath = path.join(__dirname, 'names.csv').replace(/\\/g, '/').replace('out', 'src'); - await runCell(cell2); + // Wait till execution count changes and status is success. + await waitForExecutionCompletedSuccessfully(cell); + await insertCodeCell(`import pandas as pd\ndf = pd.read_csv("${namesCsvPath}")\n`, { + index: 1 + }); + const cell2 = vscodeNotebook.activeNotebookEditor?.notebook.cellAt(1)!; - // Wait till execution count changes and status is success. - await waitForExecutionCompletedSuccessfully(cell2); - const cell3 = await insertCodeCell('import os\nprint(os.getcwd())\n'); - await runCell(cell3); + await runCell(cell2); - // Wait till execution count changes and status is success. - await waitForExecutionCompletedSuccessfully(cell3); - traceInfo(`last cell output: ${getCellOutputs(cell3)}`); + // Wait till execution count changes and status is success. + await waitForExecutionCompletedSuccessfully(cell2); + const cell3 = await insertCodeCell('import os\nprint(os.getcwd())\n'); + await runCell(cell3); - // Now add the cell to check intellisense. - await insertCodeCell(cellCode); - const cell4 = vscodeNotebook.activeNotebookEditor!.notebook.cellAt(3); + // Wait till execution count changes and status is success. + await waitForExecutionCompletedSuccessfully(cell3); + traceInfo(`last cell output: ${getCellOutputs(cell3)}`); - const token = new CancellationTokenSource().token; - // If we're testing string completions, ensure the cursor position is inside the string quotes. - let position = new Position( - 0, - cellCode.includes('"') || cellCode.includes("'") ? cellCode.length - 1 : cellCode.length - ); - let context: CompletionContext = { - triggerKind: triggerCharacter ? CompletionTriggerKind.TriggerCharacter : CompletionTriggerKind.Invoke, - triggerCharacter - }; - traceInfo('Get completions in test'); - let completions = await completionProvider.provideCompletionItems(cell4.document, position, token, context); - await sleep(500); - // Ask a second time as Jupyter can sometimes not be ready - traceInfo('Get completions second time in test'); - completions = await completionProvider.provideCompletionItems(cell4.document, position, token, context); - let items = completions.map((item) => item.label); - assert.isOk(items.length); - if (itemToExistInCompletion) { - assert.ok( - items.find((item) => - typeof item === 'string' - ? item.includes(itemToExistInCompletion) - : item.label.includes(itemToExistInCompletion) - ) - ); - } else { - return; - } - if (itemToNotExistInCompletion) { - assert.isUndefined( - items.find((item) => - typeof item === 'string' - ? item.includes(itemToNotExistInCompletion) - : item.label.includes(itemToNotExistInCompletion) - ) - ); - } - // Make sure it is skipping items that are already provided by pylance (no dupes) - // Pylance isn't returning them right now: https://github.com/microsoft/vscode-jupyter/issues/8842 - // assert.notOk( - // items.find((item) => (typeof item === 'string' ? item.includes('Name') : item.label.includes('Name'))) - // ); + // Now add the cell to check intellisense. + await insertCodeCell(cellCode); + const cell4 = vscodeNotebook.activeNotebookEditor!.notebook.cellAt(3); - if (!textToFilterCompletions || !itemToExistInCompletionAfterFilter) { - return; - } - // Add some text after the . and make sure we still get completions - const edit = new WorkspaceEdit(); - edit.insert(cell4.document.uri, new Position(cellCode.length, 0), textToFilterCompletions); - await workspace.applyEdit(edit); - position = new Position(0, cellCode.length + textToFilterCompletions.length); - completions = await completionProvider.provideCompletionItems(cell4.document, position, token, context); - items = completions.map((item) => item.label); - assert.isOk(items.length); - assert.isUndefined( - // Since we've filtered the completion the old item will no longer exist. - items.find((item) => - typeof item === 'string' - ? item.includes(itemToExistInCompletion) - : item.label.includes(itemToExistInCompletion) - ) - ); - assert.ok( - items.find((item) => - typeof item === 'string' - ? item.includes(itemToExistInCompletionAfterFilter) - : item.label.includes(itemToExistInCompletionAfterFilter) - ) - ); - } - test('Dataframe completions', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions('df.', '.', fileName, 'Age', 'N', 'Name'); - }); - test('Dataframe column completions', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions('df.Name.', '.', fileName, 'add_prefix', 'add_s', 'add_suffix'); - }); - test('Dataframe assignment completions', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions('var_name = df.', '.', fileName, 'Age', 'N', 'Name'); - }); - test('Dataframe assignment column completions', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions(fileName.substring(0, 1), fileName); - }); - test('File path completions with double quotes', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions(`"${fileName.substring(0, 1)}"`, undefined, fileName); - }); - test('File path completions with single quotes', async () => { - const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); - await testCompletions(`'${fileName.substring(0, 1)}'`, undefined, fileName); - }); - test('Provider is registered', async () => { - await insertCodeCell('print(1)', { - index: 0 - }); - let stubCalled = false; - const stub = sinon.stub(completionProvider, 'provideCompletionItems'); - stub.callsFake( - async ( - _document: TextDocument, - _position: Position, - _token: CancellationToken, - _context: CompletionContext - ) => { - stubCalled = true; - return []; + const token = new CancellationTokenSource().token; + // If we're testing string completions, ensure the cursor position is inside the string quotes. + let position = new Position( + 0, + cellCode.includes('"') || cellCode.includes("'") ? cellCode.length - 1 : cellCode.length + ); + let context: CompletionContext = { + triggerKind: triggerCharacter + ? CompletionTriggerKind.TriggerCharacter + : CompletionTriggerKind.Invoke, + triggerCharacter + }; + traceInfo('Get completions in test'); + let completions = await completionProvider.provideCompletionItems( + cell4.document, + position, + token, + context + ); + await sleep(500); + // Ask a second time as Jupyter can sometimes not be ready + traceInfo('Get completions second time in test'); + completions = await completionProvider.provideCompletionItems(cell4.document, position, token, context); + let items = completions.map((item) => item.label); + assert.isOk(items.length); + if (itemToExistInCompletion) { + assert.ok( + items.find((item) => + typeof item === 'string' + ? item.includes(itemToExistInCompletion) + : item.label.includes(itemToExistInCompletion) + ) + ); + } else { + return; + } + if (itemToNotExistInCompletion) { + assert.isUndefined( + items.find((item) => + typeof item === 'string' + ? item.includes(itemToNotExistInCompletion) + : item.label.includes(itemToNotExistInCompletion) + ) + ); + } + // Make sure it is skipping items that are already provided by pylance (no dupes) + // Pylance isn't returning them right now: https://github.com/microsoft/vscode-jupyter/issues/8842 + // assert.notOk( + // items.find((item) => (typeof item === 'string' ? item.includes('Name') : item.label.includes('Name'))) + // ); + + if (!textToFilterCompletions || !itemToExistInCompletionAfterFilter) { + return; + } + // Add some text after the . and make sure we still get completions + const edit = new WorkspaceEdit(); + edit.insert(cell4.document.uri, new Position(cellCode.length, 0), textToFilterCompletions); + await workspace.applyEdit(edit); + position = new Position(0, cellCode.length + textToFilterCompletions.length); + completions = await completionProvider.provideCompletionItems(cell4.document, position, token, context); + items = completions.map((item) => item.label); + assert.isOk(items.length); + assert.isUndefined( + // Since we've filtered the completion the old item will no longer exist. + items.find((item) => + typeof item === 'string' + ? item.includes(itemToExistInCompletion) + : item.label.includes(itemToExistInCompletion) + ) + ); + assert.ok( + items.find((item) => + typeof item === 'string' + ? item.includes(itemToExistInCompletionAfterFilter) + : item.label.includes(itemToExistInCompletionAfterFilter) + ) + ); } - ); - await insertCodeCell('a.', { index: 1 }); - const cell2 = vscodeNotebook.activeNotebookEditor!.notebook.cellAt(1); + test('Dataframe completions', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions('df.', '.', fileName, 'Age', 'N', 'Name'); + }); + test('Dataframe column completions', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions('df.Name.', '.', fileName, 'add_prefix', 'add_s', 'add_suffix'); + }); + test('Dataframe assignment completions', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions('var_name = df.', '.', fileName, 'Age', 'N', 'Name'); + }); + test('Dataframe assignment column completions', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions(fileName.substring(0, 1), fileName); + }); + test('File path completions with double quotes', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions(`"${fileName.substring(0, 1)}"`, undefined, fileName); + }); + test('File path completions with single quotes', async () => { + const fileName = path.basename(vscodeNotebook.activeNotebookEditor!.notebook.uri.fsPath); + await testCompletions(`'${fileName.substring(0, 1)}'`, undefined, fileName); + }); + test('Provider is registered', async () => { + await insertCodeCell('print(1)', { + index: 0 + }); + let stubCalled = false; + const stub = sinon.stub(completionProvider, 'provideCompletionItems'); + stub.callsFake( + async ( + _document: TextDocument, + _position: Position, + _token: CancellationToken, + _context: CompletionContext + ) => { + stubCalled = true; + return []; + } + ); + await insertCodeCell('a.', { index: 1 }); + const cell2 = vscodeNotebook.activeNotebookEditor!.notebook.cellAt(1); - const position = new Position(0, 2); - traceInfo('Get completions in test'); - // Executing the command `vscode.executeCompletionItemProvider` to simulate triggering completion - await commands.executeCommand('vscode.executeCompletionItemProvider', cell2.document.uri, position); - assert.ok(stubCalled, 'Completion provider not registered'); - }); + const position = new Position(0, 2); + traceInfo('Get completions in test'); + // Executing the command `vscode.executeCompletionItemProvider` to simulate triggering completion + await commands.executeCommand('vscode.executeCompletionItemProvider', cell2.document.uri, position); + assert.ok(stubCalled, 'Completion provider not registered'); + }); + } + ); });