From f071b877b57d8d306797bed7df6762409d500d76 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 3 Nov 2020 16:44:28 -0800 Subject: [PATCH] Module abbreviation infrastructure, remove unused mappings on completion resolve, other (#1135) Rollup of: - Verify `package-lock.json` state in checks to avoid lerna modifying things. - Add infrastructure for module abbreviations in completions. - Removed unused auto-import mapping from `completion/resolve` handler. - Add `isInImport` to completion data for checking the completion context. - Allow completion verification in fourslash tests to be overridden. --- build/checkLockIndent.js | 45 ++ package-lock.json | 20 +- package.json | 6 +- .../pyright-internal/src/analyzer/program.ts | 31 +- .../pyright-internal/src/analyzer/service.ts | 14 +- .../src/analyzer/sourceFile.ts | 15 +- .../src/backgroundAnalysisBase.ts | 44 +- packages/pyright-internal/src/common/core.ts | 15 + .../src/languageServerBase.ts | 1 + .../src/languageService/autoImporter.ts | 17 + .../src/languageService/completionProvider.ts | 416 ++++++++++-------- .../src/tests/fourslash/fourslash.ts | 8 +- .../src/tests/harness/fourslash/testState.ts | 36 +- packages/vscode-pyright/src/extension.ts | 5 +- 14 files changed, 406 insertions(+), 267 deletions(-) create mode 100644 build/checkLockIndent.js diff --git a/build/checkLockIndent.js b/build/checkLockIndent.js new file mode 100644 index 000000000..c5c442eb0 --- /dev/null +++ b/build/checkLockIndent.js @@ -0,0 +1,45 @@ +/* eslint-disable @typescript-eslint/no-var-requires */ +//@ts-check + +// Lerna doesn't do a good job preserving the indention in lock files. +// Check that the lock files are still indented correctly, otherwise +// the change will cause problems with merging and the updateDeps script. + +const detectIndent = require('detect-indent'); +const fsExtra = require('fs-extra'); +const util = require('util'); +const glob = util.promisify(require('glob')); + +async function findPackageLocks() { + const lernaFile = await fsExtra.readFile('lerna.json', 'utf-8'); + + /** @type {{ packages: string[] }} */ + const lernaConfig = JSON.parse(lernaFile); + + const matches = await Promise.all(lernaConfig.packages.map((pattern) => glob(pattern + '/package-lock.json'))); + return ['package-lock.json'].concat(...matches); +} + +async function main() { + const locks = await findPackageLocks(); + + let ok = true; + + for (const filepath of locks) { + const input = await fsExtra.readFile(filepath, 'utf-8'); + const indent = detectIndent(input); + + if (indent.indent !== ' ') { + ok = false; + console.error(`${filepath} has invalid indent "${indent.indent}"`); + } + } + + if (!ok) { + console.error('Lerna may have modified package-lock.json during bootstrap.'); + console.error('You may need to revert any package-lock changes and rerun install:all.'); + process.exit(1); + } +} + +main(); diff --git a/package-lock.json b/package-lock.json index ce867d0de..d06465bb4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4121,9 +4121,9 @@ "dev": true }, "detect-indent": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/detect-indent/-/detect-indent-5.0.0.tgz", - "integrity": "sha1-OHHMCmoALow+Wzz38zYmRnXwa50=", + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/detect-indent/-/detect-indent-6.0.0.tgz", + "integrity": "sha512-oSyFlqaTHCItVRGK5RmrmjB+CmaMOW7IaNA/kdxqhoa6d17j/5ce9O9eWXmV/KEdRwqpQA+Vqe8a8Bsybu4YnA==", "dev": true }, "dezalgo": { @@ -10413,6 +10413,12 @@ "write-file-atomic": "^2.4.2" }, "dependencies": { + "detect-indent": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/detect-indent/-/detect-indent-5.0.0.tgz", + "integrity": "sha1-OHHMCmoALow+Wzz38zYmRnXwa50=", + "dev": true + }, "make-dir": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-2.1.0.tgz", @@ -10459,6 +10465,14 @@ "pify": "^3.0.0", "sort-keys": "^2.0.0", "write-file-atomic": "^2.0.0" + }, + "dependencies": { + "detect-indent": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/detect-indent/-/detect-indent-5.0.0.tgz", + "integrity": "sha1-OHHMCmoALow+Wzz38zYmRnXwa50=", + "dev": true + } } } } diff --git a/package.json b/package.json index f6991ffcb..daf785223 100644 --- a/package.json +++ b/package.json @@ -10,13 +10,14 @@ "build:extension:dev": "cd packages/vscode-pyright && npm run webpack", "build:cli:dev": "cd packages/pyright && npm run webpack", "watch:extension": "cd packages/vscode-pyright && npm run webpack-dev", - "check": "npm run check:syncpack && npm run check:eslint && npm run check:prettier", + "check": "npm run check:syncpack && npm run check:eslint && npm run check:prettier && npm run check:lockindent", "check:syncpack": "syncpack list-mismatches", "fix:syncpack": "syncpack fix-mismatches --indent \" \" && npm run install:all", "check:eslint": "eslint .", "fix:eslint": "eslint --fix .", "check:prettier": "prettier -c .", - "fix:prettier": "prettier --write ." + "fix:prettier": "prettier --write .", + "check:lockindent": "node ./build/checkLockIndent.js" }, "devDependencies": { "@types/fs-extra": "^9.0.2", @@ -25,6 +26,7 @@ "@types/yargs": "^15.0.9", "@typescript-eslint/eslint-plugin": "^4.6.0", "@typescript-eslint/parser": "^4.6.0", + "detect-indent": "^6.0.0", "eslint": "^7.12.0", "eslint-config-prettier": "^6.14.0", "eslint-plugin-simple-import-sort": "^5.0.3", diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index 30e7b30cf..1b87148b9 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -45,10 +45,11 @@ import { AutoImporter, AutoImportResult, buildModuleSymbolsMap, + getAutoImportCandidatesForAbbr, ModuleSymbolMap, } from '../languageService/autoImporter'; import { CallHierarchyProvider } from '../languageService/callHierarchyProvider'; -import { CompletionResults } from '../languageService/completionProvider'; +import { AbbreviationMap, CompletionResults } from '../languageService/completionProvider'; import { IndexOptions, IndexResults, WorkspaceSymbolCallback } from '../languageService/documentSymbolProvider'; import { HoverResults } from '../languageService/hoverProvider'; import { ReferenceCallback, ReferencesResult } from '../languageService/referencesProvider'; @@ -433,16 +434,16 @@ export class Program { }); } - indexWorkspace(callback: (path: string, results: IndexResults) => void, token: CancellationToken) { + indexWorkspace(callback: (path: string, results: IndexResults) => void, token: CancellationToken): number { if (!this._configOptions.indexing) { - return; + return 0; } - let count = 0; return this._runEvaluatorWithCancellationToken(token, () => { // Go through all workspace files to create indexing data. // This will cause all files in the workspace to be parsed and bound. But // _handleMemoryHighUsage will make sure we don't OOM + let count = 0; for (const sourceFileInfo of this._sourceFileList) { if (!this._isUserCode(sourceFileInfo)) { continue; @@ -453,7 +454,7 @@ export class Program { if (results) { if (++count > 2000) { this._console.warn(`Workspace indexing has hit its upper limit: 2000 files`); - return; + return count; } callback(sourceFileInfo.sourceFile.getFilePath(), results); @@ -461,6 +462,8 @@ export class Program { this._handleMemoryHighUsage(); } + + return count; }); } @@ -955,7 +958,7 @@ export class Program { filePath: string, range: Range, similarityLimit: number, - nameMap: Map | undefined, + nameMap: AbbreviationMap | undefined, libraryMap: Map | undefined, token: CancellationToken ): AutoImportResult[] { @@ -1002,13 +1005,10 @@ export class Program { const currentScope = getScopeForNode(currentNode); if (currentScope) { - const translatedWord = nameMap?.get(writtenWord); - if (translatedWord) { - // No filter is needed since we only do exact match. - const exactMatch = 1; - results.push( - ...autoImporter.getAutoImportCandidates(translatedWord, exactMatch, writtenWord, token) - ); + const info = nameMap?.get(writtenWord); + if (info) { + // No scope filter is needed since we only do exact match. + results.push(...getAutoImportCandidatesForAbbr(autoImporter, writtenWord, info, token)); } results.push( @@ -1354,6 +1354,7 @@ export class Program { position: Position, workspacePath: string, format: MarkupKind, + nameMap: AbbreviationMap | undefined, libraryMap: Map | undefined, token: CancellationToken ): Promise { @@ -1378,6 +1379,7 @@ export class Program { this._evaluator!, format, this._createSourceMapper(execEnv, /* mapCompiled */ true), + nameMap, libraryMap, () => this._buildModuleSymbolsMap(sourceFileInfo, !!libraryMap, token), token @@ -1416,7 +1418,6 @@ export class Program { filePath: string, completionItem: CompletionItem, format: MarkupKind, - libraryMap: Map | undefined, token: CancellationToken ) { return this._runEvaluatorWithCancellationToken(token, () => { @@ -1435,8 +1436,6 @@ export class Program { this._evaluator!, format, this._createSourceMapper(execEnv, /* mapCompiled */ true), - libraryMap, - () => this._buildModuleSymbolsMap(sourceFileInfo, !!libraryMap, token), completionItem, token ); diff --git a/packages/pyright-internal/src/analyzer/service.ts b/packages/pyright-internal/src/analyzer/service.ts index 6c43b3836..7221c78f0 100644 --- a/packages/pyright-internal/src/analyzer/service.ts +++ b/packages/pyright-internal/src/analyzer/service.ts @@ -47,7 +47,7 @@ import { } from '../common/pathUtils'; import { DocumentRange, Position, Range } from '../common/textRange'; import { timingStats } from '../common/timing'; -import { CompletionResults } from '../languageService/completionProvider'; +import { AbbreviationMap, CompletionResults } from '../languageService/completionProvider'; import { IndexResults, WorkspaceSymbolCallback } from '../languageService/documentSymbolProvider'; import { HoverResults } from '../languageService/hoverProvider'; import { ReferenceCallback } from '../languageService/referencesProvider'; @@ -226,7 +226,7 @@ export class AnalyzerService { filePath: string, range: Range, similarityLimit: number, - nameMap: Map | undefined, + nameMap: AbbreviationMap | undefined, token: CancellationToken ) { return this._program.getAutoImports( @@ -295,6 +295,7 @@ export class AnalyzerService { position: Position, workspacePath: string, format: MarkupKind, + nameMap: AbbreviationMap | undefined, token: CancellationToken ): Promise { return this._program.getCompletionsForPosition( @@ -302,6 +303,7 @@ export class AnalyzerService { position, workspacePath, format, + nameMap, this._backgroundAnalysisProgram.getIndexing(filePath), token ); @@ -313,13 +315,7 @@ export class AnalyzerService { format: MarkupKind, token: CancellationToken ) { - this._program.resolveCompletionItem( - filePath, - completionItem, - format, - this._backgroundAnalysisProgram.getIndexing(filePath), - token - ); + this._program.resolveCompletionItem(filePath, completionItem, format, token); } performQuickAction( diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index be693b5c6..f31f7f7a6 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -33,7 +33,7 @@ import { DocumentRange, getEmptyRange, Position, TextRange } from '../common/tex import { TextRangeCollection } from '../common/textRangeCollection'; import { timingStats } from '../common/timing'; import { ModuleSymbolMap } from '../languageService/autoImporter'; -import { CompletionResults } from '../languageService/completionProvider'; +import { AbbreviationMap, CompletionResults } from '../languageService/completionProvider'; import { CompletionItemData, CompletionProvider } from '../languageService/completionProvider'; import { DefinitionProvider } from '../languageService/definitionProvider'; import { DocumentHighlightProvider } from '../languageService/documentHighlightProvider'; @@ -790,6 +790,7 @@ export class SourceFile { evaluator: TypeEvaluator, format: MarkupKind, sourceMapper: SourceMapper, + nameMap: AbbreviationMap | undefined, libraryMap: Map | undefined, moduleSymbolsCallback: () => ModuleSymbolMap, token: CancellationToken @@ -818,8 +819,11 @@ export class SourceFile { evaluator, format, sourceMapper, - libraryMap, - moduleSymbolsCallback, + { + nameMap, + libraryMap, + getModuleSymbolsMap: moduleSymbolsCallback, + }, token ); @@ -833,8 +837,6 @@ export class SourceFile { evaluator: TypeEvaluator, format: MarkupKind, sourceMapper: SourceMapper, - libraryMap: Map | undefined, - moduleSymbolsCallback: () => ModuleSymbolMap, completionItem: CompletionItem, token: CancellationToken ) { @@ -856,8 +858,7 @@ export class SourceFile { evaluator, format, sourceMapper, - libraryMap, - moduleSymbolsCallback, + undefined, token ); diff --git a/packages/pyright-internal/src/backgroundAnalysisBase.ts b/packages/pyright-internal/src/backgroundAnalysisBase.ts index c20e08023..b1564e631 100644 --- a/packages/pyright-internal/src/backgroundAnalysisBase.ts +++ b/packages/pyright-internal/src/backgroundAnalysisBase.ts @@ -49,25 +49,7 @@ export class BackgroundAnalysisBase { this._worker = worker; // global channel to communicate from BG channel to main thread. - worker.on('message', (msg: AnalysisResponse) => { - switch (msg.requestType) { - case 'log': { - const logData = msg.data as LogData; - this.log(logData.level, logData.message); - break; - } - - case 'analysisResult': { - // Change in diagnostics due to host such as file closed rather than - // analyzing files. - this._onAnalysisCompletion(convertAnalysisResults(msg.data)); - break; - } - - default: - debug.fail(`${msg.requestType} is not expected`); - } - }); + worker.on('message', (msg: AnalysisResponse) => this.onMessage(msg)); // this will catch any exception thrown from background thread, // print log and ignore exception @@ -76,6 +58,26 @@ export class BackgroundAnalysisBase { }); } + protected onMessage(msg: AnalysisResponse) { + switch (msg.requestType) { + case 'log': { + const logData = msg.data as LogData; + this.log(logData.level, logData.message); + break; + } + + case 'analysisResult': { + // Change in diagnostics due to host such as file closed rather than + // analyzing files. + this._onAnalysisCompletion(convertAnalysisResults(msg.data)); + break; + } + + default: + debug.fail(`${msg.requestType} is not expected`); + } + } + setCompletionCallback(callback?: AnalysisCompleteCallback) { this._onAnalysisCompletion = callback ?? nullCallback; } @@ -530,7 +532,7 @@ export interface AnalysisRequest { port?: MessagePort; } -interface AnalysisResponse { - requestType: 'log' | 'analysisResult' | 'analysisPaused' | 'indexResult' | 'analysisDone'; +export interface AnalysisResponse { + requestType: 'log' | 'telemetry' | 'analysisResult' | 'analysisPaused' | 'indexResult' | 'analysisDone'; data: any; } diff --git a/packages/pyright-internal/src/common/core.ts b/packages/pyright-internal/src/common/core.ts index 26b3e932e..5ad810855 100644 --- a/packages/pyright-internal/src/common/core.ts +++ b/packages/pyright-internal/src/common/core.ts @@ -127,3 +127,18 @@ export function isDebugMode() { const argv = process.execArgv.join(); return argv.includes('inspect') || argv.includes('debug'); } + +interface Thenable { + then( + onfulfilled?: (value: T) => TResult | Thenable, + onrejected?: (reason: any) => TResult | Thenable + ): Thenable; + then( + onfulfilled?: (value: T) => TResult | Thenable, + onrejected?: (reason: any) => void + ): Thenable; +} + +export function isThenable(v: any): v is Thenable { + return typeof v?.then === 'function'; +} diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index dfec9c912..ce8758093 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -891,6 +891,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface { position, workspacePath, this._completionDocFormat, + undefined, token ); } diff --git a/packages/pyright-internal/src/languageService/autoImporter.ts b/packages/pyright-internal/src/languageService/autoImporter.ts index c6ae9f56b..3be1dc4a2 100644 --- a/packages/pyright-internal/src/languageService/autoImporter.ts +++ b/packages/pyright-internal/src/languageService/autoImporter.ts @@ -114,6 +114,23 @@ export function buildModuleSymbolsMap(files: SourceFileInfo[], token: Cancellati return moduleSymbolMap; } +export interface AbbreviationInfo { + importFrom?: string; + importName: string; +} + +export function getAutoImportCandidatesForAbbr( + autoImporter: AutoImporter, + abbr: string | undefined, + abbrInfo: AbbreviationInfo, + token: CancellationToken +) { + const exactMatch = 1; + return autoImporter + .getAutoImportCandidates(abbrInfo.importName, exactMatch, abbr, token) + .filter((r) => r.source === abbrInfo.importFrom && r.name === abbrInfo.importName); +} + export interface AutoImportResult { name: string; symbol?: Symbol; diff --git a/packages/pyright-internal/src/languageService/completionProvider.ts b/packages/pyright-internal/src/languageService/completionProvider.ts index 69c18323b..ce305c57c 100644 --- a/packages/pyright-internal/src/languageService/completionProvider.ts +++ b/packages/pyright-internal/src/languageService/completionProvider.ts @@ -60,7 +60,6 @@ import { getMembersForModule, isProperty, makeTypeVarsConcrete, - specializeType, } from '../analyzer/typeUtils'; import { throwIfCancellationRequested } from '../common/cancellationUtils'; import { ConfigOptions } from '../common/configOptions'; @@ -85,7 +84,13 @@ import { StringNode, } from '../parser/parseNodes'; import { ParseResults } from '../parser/parser'; -import { AutoImporter, ModuleSymbolMap } from './autoImporter'; +import { + AbbreviationInfo, + AutoImporter, + AutoImportResult, + getAutoImportCandidatesForAbbr, + ModuleSymbolMap, +} from './autoImporter'; import { IndexResults } from './documentSymbolProvider'; const _keywords: string[] = [ @@ -179,6 +184,7 @@ export interface CompletionItemData { position: Position; autoImportText?: string; symbolLabel?: string; + isInImport?: boolean; } // MemberAccessInfo attempts to gather info for unknown types @@ -193,11 +199,40 @@ export interface CompletionResults { memberAccessInfo?: MemberAccessInfo; } +export type AbbreviationMap = Map; + +export interface AutoImportMaps { + nameMap?: AbbreviationMap; + libraryMap?: Map; + getModuleSymbolsMap: () => ModuleSymbolMap; +} + interface RecentCompletionInfo { label: string; autoImportText: string; } +interface Edits { + textEdit?: TextEdit; + additionalTextEdits?: TextEditAction[]; +} + +interface SymbolDetail { + isInImport?: boolean; + autoImportSource?: string; + autoImportAlias?: string; + objectThrough?: ObjectType; + edits?: Edits; +} + +interface CompletionDetail { + isInImport?: boolean; + typeDetail?: string; + documentation?: string; + autoImportText?: string; + edits?: Edits; +} + // We'll use a somewhat-arbitrary cutoff value here to determine // whether it's sufficiently similar. const similarityLimit = 0.25; @@ -224,8 +259,7 @@ export class CompletionProvider { private _evaluator: TypeEvaluator, private _format: MarkupKind, private _sourceMapper: SourceMapper, - private _libraryMap: Map | undefined, - private _moduleSymbolsCallback: () => ModuleSymbolMap, + private _autoImportMaps: AutoImportMaps | undefined, private _cancellationToken: CancellationToken ) {} @@ -513,7 +547,7 @@ export class CompletionProvider { const methodSignature = this._printMethodSignature(decl.node) + ':'; const textEdit = TextEdit.replace(range, methodSignature); - this._addSymbol(name, symbol, partialName.value, completionList, undefined, textEdit); + this._addSymbol(name, symbol, partialName.value, completionList, { edits: { textEdit } }); } } }); @@ -603,7 +637,7 @@ export class CompletionProvider { const objectThrough: ObjectType | undefined = isObject(specializedLeftType) ? specializedLeftType : undefined; - this._addSymbolsForSymbolTable(symbolTable, (_) => true, priorWord, objectThrough, completionList); + this._addSymbolsForSymbolTable(symbolTable, (_) => true, priorWord, false, objectThrough, completionList); // If we don't know this type, look for a module we should stub if (!leftType || isUnknown(leftType) || isUnbound(leftType)) { @@ -965,46 +999,49 @@ export class CompletionProvider { } private _getAutoImportCompletions(priorWord: string, completionList: CompletionList) { - const moduleSymbolMap = this._moduleSymbolsCallback(); + if (!this._autoImportMaps) { + return; + } + + const moduleSymbolMap = this._autoImportMaps.getModuleSymbolsMap(); + const excludes = completionList.items.filter((i) => !i.data?.autoImport).map((i) => i.label); const autoImporter = new AutoImporter( this._configOptions.findExecEnvironment(this._filePath), this._importResolver, this._parseResults, this._position, - completionList.items.filter((i) => !i.data?.autoImport).map((i) => i.label), + excludes, moduleSymbolMap, - this._libraryMap + this._autoImportMaps.libraryMap ); - for (const result of autoImporter.getAutoImportCandidates( - priorWord, - similarityLimit, - undefined, - this._cancellationToken - )) { + const results: AutoImportResult[] = []; + const info = this._autoImportMaps.nameMap?.get(priorWord); + if (info && priorWord.length > 1 && !excludes.some((e) => e === priorWord)) { + results.push(...getAutoImportCandidatesForAbbr(autoImporter, priorWord, info, this._cancellationToken)); + } + + results.push( + ...autoImporter.getAutoImportCandidates(priorWord, similarityLimit, undefined, this._cancellationToken) + ); + + for (const result of results) { if (result.symbol) { - this._addSymbol( - result.name, - result.symbol, - priorWord, - completionList, - result.source, - undefined, - result.edits - ); + this._addSymbol(result.name, result.symbol, priorWord, completionList, { + autoImportSource: result.source, + autoImportAlias: result.alias, + edits: { additionalTextEdits: result.edits }, + }); } else { this._addNameToCompletionList( - result.name, + result.alias ?? result.name, result.kind ?? CompletionItemKind.Module, priorWord, completionList, - undefined, - '', - result.source - ? `\`\`\`\nfrom ${result.source} import ${result.name}\n\`\`\`` - : `\`\`\`\nimport ${result.name}\n\`\`\``, - undefined, - result.edits + { + autoImportText: this._getAutoImportText(result.name, result.source, result.alias), + edits: { additionalTextEdits: result.edits }, + } ); } } @@ -1040,6 +1077,7 @@ export class CompletionProvider { return !importFromNode.imports.find((imp) => imp.name.value === name); }, priorWord, + true, undefined, completionList ); @@ -1120,7 +1158,14 @@ export class CompletionProvider { let scope = AnalyzerNodeInfo.getScope(curNode); if (scope) { while (scope) { - this._addSymbolsForSymbolTable(scope.symbolTable, () => true, priorWord, undefined, completionList); + this._addSymbolsForSymbolTable( + scope.symbolTable, + () => true, + priorWord, + false, + undefined, + completionList + ); scope = scope.parent; } @@ -1144,6 +1189,7 @@ export class CompletionProvider { .some((decl) => decl.type === DeclarationType.Variable); }, priorWord, + false, undefined, completionList ); @@ -1162,6 +1208,7 @@ export class CompletionProvider { symbolTable: SymbolTable, includeSymbolCallback: (name: string) => boolean, priorWord: string, + isInImport: boolean, objectThrough: ObjectType | undefined, completionList: CompletionList ) { @@ -1173,16 +1220,10 @@ export class CompletionProvider { // Don't add a symbol more than once. It may have already been // added from an inner scope's symbol table. if (!completionList.items.some((item) => item.label === name)) { - this._addSymbol( - name, - symbol, - priorWord, - completionList, - undefined, - undefined, - undefined, - objectThrough - ); + this._addSymbol(name, symbol, priorWord, completionList, { + objectThrough, + isInImport, + }); } } }); @@ -1193,10 +1234,7 @@ export class CompletionProvider { symbol: Symbol, priorWord: string, completionList: CompletionList, - autoImportSource?: string, - textEdit?: TextEdit, - additionalTextEdits?: TextEditAction[], - objectThrough?: ObjectType + detail: SymbolDetail ) { let primaryDecl = getLastTypedDeclaredForSymbol(symbol); if (!primaryDecl) { @@ -1235,11 +1273,11 @@ export class CompletionProvider { break; case DeclarationType.Function: { - const functionType = objectThrough - ? this._evaluator.bindFunctionToClassOrObject(objectThrough, type, false) + const functionType = detail.objectThrough + ? this._evaluator.bindFunctionToClassOrObject(detail.objectThrough, type, false) : type; if (functionType) { - if (isProperty(functionType) && objectThrough) { + if (isProperty(functionType) && detail.objectThrough) { const propertyType = this._evaluator.getGetterTypeFromProperty( functionType.classType, @@ -1339,161 +1377,165 @@ export class CompletionProvider { } } - let autoImportText: string | undefined; - if (autoImportSource) { - if (this._format === MarkupKind.Markdown) { - autoImportText = `\`\`\`\nfrom ${autoImportSource} import ${name}\n\`\`\``; - } else if (this._format === MarkupKind.PlainText) { - autoImportText = `from ${autoImportSource} import ${name}`; - } else { - fail(`Unsupported markup type: ${this._format}`); - } - } + const autoImportText = detail.autoImportSource + ? this._getAutoImportText(name, detail.autoImportSource, detail.autoImportAlias) + : undefined; - this._addNameToCompletionList( - name, - itemKind, - priorWord, - completionList, - undefined, - undefined, + this._addNameToCompletionList(detail.autoImportAlias ?? name, itemKind, priorWord, completionList, { autoImportText, - textEdit, - additionalTextEdits - ); + isInImport: detail.isInImport, + edits: detail.edits, + }); } else { // Does the symbol have no declaration but instead has a synthesized type? const synthesizedType = symbol.getSynthesizedType(); if (synthesizedType) { const itemKind: CompletionItemKind = CompletionItemKind.Variable; - this._addNameToCompletionList( - name, - itemKind, - priorWord, - completionList, - undefined, - undefined, - undefined, - textEdit, - additionalTextEdits - ); + this._addNameToCompletionList(name, itemKind, priorWord, completionList, { + isInImport: detail.isInImport, + edits: detail.edits, + }); } } } + private _getAutoImportText(importName: string, importFrom?: string, importAlias?: string) { + let autoImportText: string | undefined; + if (!importFrom) { + autoImportText = `import ${importName}`; + } else { + autoImportText = `from ${importFrom} import ${importName}`; + } + + if (importAlias) { + autoImportText = `${autoImportText} as ${importAlias}`; + } + + if (this._format === MarkupKind.Markdown) { + return `\`\`\`\n${autoImportText}\n\`\`\``; + } else if (this._format === MarkupKind.PlainText) { + return autoImportText; + } else { + fail(`Unsupported markup type: ${this._format}`); + } + } + private _addNameToCompletionList( name: string, itemKind: CompletionItemKind, filter: string, completionList: CompletionList, - typeDetail?: string, - documentation?: string, - autoImportText?: string, - textEdit?: TextEdit, - additionalTextEdits?: TextEditAction[] + detail?: CompletionDetail ) { - const similarity = StringUtils.computeCompletionSimilarity(filter, name); - - if (similarity > similarityLimit) { - const completionItem = CompletionItem.create(name); - completionItem.kind = itemKind; - - const completionItemData: CompletionItemData = { - workspacePath: this._workspacePath, - filePath: this._filePath, - position: this._position, - }; - completionItem.data = completionItemData; - - if (autoImportText) { - // Force auto-import entries to the end. - completionItem.sortText = this._makeSortText(SortCategory.AutoImport, name, autoImportText); - completionItemData.autoImportText = autoImportText; - completionItem.detail = 'Auto-import'; - } else if (SymbolNameUtils.isDunderName(name)) { - // Force dunder-named symbols to appear after all other symbols. - completionItem.sortText = this._makeSortText(SortCategory.DunderSymbol, name); - } else if (filter === '' && SymbolNameUtils.isPrivateOrProtectedName(name)) { - // Distinguish between normal and private symbols only if there is - // currently no filter text. Once we get a single character to filter - // upon, we'll no longer differentiate. - completionItem.sortText = this._makeSortText(SortCategory.PrivateSymbol, name); - } else { - completionItem.sortText = this._makeSortText(SortCategory.NormalSymbol, name); - } - - completionItemData.symbolLabel = name; - - if (this._format === MarkupKind.Markdown) { - let markdownString = ''; - - if (autoImportText) { - markdownString += autoImportText + '\n\n'; - } - - if (typeDetail) { - markdownString += '```python\n' + typeDetail + '\n```\n'; - } - - if (documentation) { - markdownString += '---\n'; - markdownString += convertDocStringToMarkdown(documentation); - } - - markdownString = markdownString.trimEnd(); - - if (markdownString) { - completionItem.documentation = { - kind: MarkupKind.Markdown, - value: markdownString, - }; - } - } else if (this._format === MarkupKind.PlainText) { - let plainTextString = ''; - - if (autoImportText) { - plainTextString += autoImportText + '\n\n'; - } - - if (typeDetail) { - plainTextString += typeDetail + '\n'; - } - - if (documentation) { - plainTextString += '\n' + convertDocStringToPlainText(documentation); - } - - plainTextString = plainTextString.trimEnd(); - - if (plainTextString) { - completionItem.documentation = { - kind: MarkupKind.PlainText, - value: plainTextString, - }; - } - } else { - fail(`Unsupported markup type: ${this._format}`); - } - - if (textEdit) { - completionItem.textEdit = textEdit; - } - - if (additionalTextEdits) { - completionItem.additionalTextEdits = additionalTextEdits.map((te) => { - const textEdit: TextEdit = { - range: { - start: { line: te.range.start.line, character: te.range.start.character }, - end: { line: te.range.end.line, character: te.range.end.character }, - }, - newText: te.replacementText, - }; - return textEdit; - }); - } - - completionList.items.push(completionItem); + // Auto importer already filtered out unnecessary ones. No need to do it again. + const similarity = detail?.autoImportText ? 1 : StringUtils.computeCompletionSimilarity(filter, name); + if (similarity <= similarityLimit) { + return; } + + const completionItem = CompletionItem.create(name); + completionItem.kind = itemKind; + + const completionItemData: CompletionItemData = { + workspacePath: this._workspacePath, + filePath: this._filePath, + position: this._position, + }; + + if (detail?.isInImport) { + completionItemData.isInImport = true; + } + + completionItem.data = completionItemData; + + if (detail?.autoImportText) { + // Force auto-import entries to the end. + completionItem.sortText = this._makeSortText(SortCategory.AutoImport, name, detail.autoImportText); + completionItemData.autoImportText = detail.autoImportText; + completionItem.detail = 'Auto-import'; + } else if (SymbolNameUtils.isDunderName(name)) { + // Force dunder-named symbols to appear after all other symbols. + completionItem.sortText = this._makeSortText(SortCategory.DunderSymbol, name); + } else if (filter === '' && SymbolNameUtils.isPrivateOrProtectedName(name)) { + // Distinguish between normal and private symbols only if there is + // currently no filter text. Once we get a single character to filter + // upon, we'll no longer differentiate. + completionItem.sortText = this._makeSortText(SortCategory.PrivateSymbol, name); + } else { + completionItem.sortText = this._makeSortText(SortCategory.NormalSymbol, name); + } + + completionItemData.symbolLabel = name; + + if (this._format === MarkupKind.Markdown) { + let markdownString = ''; + + if (detail?.autoImportText) { + markdownString += detail.autoImportText + '\n\n'; + } + + if (detail?.typeDetail) { + markdownString += '```python\n' + detail.typeDetail + '\n```\n'; + } + + if (detail?.documentation) { + markdownString += '---\n'; + markdownString += convertDocStringToMarkdown(detail.documentation); + } + + markdownString = markdownString.trimEnd(); + + if (markdownString) { + completionItem.documentation = { + kind: MarkupKind.Markdown, + value: markdownString, + }; + } + } else if (this._format === MarkupKind.PlainText) { + let plainTextString = ''; + + if (detail?.autoImportText) { + plainTextString += detail.autoImportText + '\n\n'; + } + + if (detail?.typeDetail) { + plainTextString += detail.typeDetail + '\n'; + } + + if (detail?.documentation) { + plainTextString += '\n' + convertDocStringToPlainText(detail.documentation); + } + + plainTextString = plainTextString.trimEnd(); + + if (plainTextString) { + completionItem.documentation = { + kind: MarkupKind.PlainText, + value: plainTextString, + }; + } + } else { + fail(`Unsupported markup type: ${this._format}`); + } + + if (detail?.edits?.textEdit) { + completionItem.textEdit = detail.edits.textEdit; + } + + if (detail?.edits?.additionalTextEdits) { + completionItem.additionalTextEdits = detail.edits.additionalTextEdits.map((te) => { + const textEdit: TextEdit = { + range: { + start: { line: te.range.start.line, character: te.range.start.character }, + end: { line: te.range.end.line, character: te.range.end.character }, + }, + newText: te.replacementText, + }; + return textEdit; + }); + } + + completionList.items.push(completionItem); } private _getRecentListIndex(name: string, autoImportText: string) { diff --git a/packages/pyright-internal/src/tests/fourslash/fourslash.ts b/packages/pyright-internal/src/tests/fourslash/fourslash.ts index 4d35a0015..feae40ddd 100644 --- a/packages/pyright-internal/src/tests/fourslash/fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/fourslash.ts @@ -131,6 +131,11 @@ declare namespace _ { kind?: DocumentHighlightKind; } + interface AbbreviationInfo { + importFrom?: string; + importName: string; + } + interface Fourslash { getDocumentHighlightKind(m?: Marker): DocumentHighlightKind | undefined; @@ -190,7 +195,8 @@ declare namespace _ { unknownMemberName?: string; }; }; - } + }, + abbrMap?: { [abbr: string]: AbbreviationInfo } ): Promise; verifySignature(map: { [marker: string]: { diff --git a/packages/pyright-internal/src/tests/harness/fourslash/testState.ts b/packages/pyright-internal/src/tests/harness/fourslash/testState.ts index 50d297a5f..18f4b496e 100644 --- a/packages/pyright-internal/src/tests/harness/fourslash/testState.ts +++ b/packages/pyright-internal/src/tests/harness/fourslash/testState.ts @@ -47,6 +47,7 @@ import { getStringComparer } from '../../../common/stringUtils'; import { DocumentRange, Position, Range as PositionRange, rangesAreEqual, TextRange } from '../../../common/textRange'; import { TextRangeCollection } from '../../../common/textRangeCollection'; import { LanguageServerInterface, WorkspaceServiceInstance } from '../../../languageServerBase'; +import { AbbreviationInfo } from '../../../languageService/autoImporter'; import { convertHoverResults } from '../../../languageService/hoverProvider'; import { ParseResults } from '../../../parser/parser'; import { Tokenizer } from '../../../parser/tokenizer'; @@ -94,6 +95,7 @@ export class TestState { readonly fs: vfs.TestFileSystem; readonly workspace: WorkspaceServiceInstance; readonly console: ConsoleInterface; + readonly rawConfigJson: any | undefined; // The current caret position in the active file currentCaretPosition = 0; @@ -124,14 +126,13 @@ export class TestState { for (const file of testData.files) { // if one of file is configuration file, set config options from the given json if (this._isConfig(file, ignoreCase)) { - let configJson: any; try { - configJson = JSON.parse(file.content); + this.rawConfigJson = JSON.parse(file.content); } catch (e) { throw new Error(`Failed to parse test ${file.fileName}: ${e.message}`); } - configOptions.initializeFromJson(configJson, 'basic', nullConsole); + configOptions.initializeFromJson(this.rawConfigJson, 'basic', nullConsole); this._applyTestConfigOptions(configOptions); } else { files[file.fileName] = new vfs.File(file.content, { meta: file.fileOptions, encoding: 'utf8' }); @@ -817,7 +818,8 @@ export class TestState { unknownMemberName?: string; }; }; - } + }, + abbrMap?: { [abbr: string]: AbbreviationInfo } ): Promise { this._analyze(); @@ -836,6 +838,7 @@ export class TestState { completionPosition, this.workspace.rootPath, docFormat, + abbrMap ? new Map(Object.entries(abbrMap)) : undefined, CancellationToken.None ); @@ -866,23 +869,16 @@ export class TestState { } const actual: CompletionItem = result.completionList.items[actualIndex]; - assert.equal(actual.label, expected.label); - assert.equal(actual.detail, expected.detail); - assert.equal(actual.kind, expected.kind); - if (expectedCompletions[i].documentation !== undefined) { + this.verifyCompletionItem(expected, actual); + + if (expected.documentation !== undefined) { if (actual.documentation === undefined) { - this.program.resolveCompletionItem( - filePath, - actual, - docFormat, - undefined, - CancellationToken.None - ); + this.program.resolveCompletionItem(filePath, actual, docFormat, CancellationToken.None); } if (MarkupContent.is(actual.documentation)) { - assert.equal(actual.documentation.value, expected.documentation); - assert.equal(actual.documentation.kind, docFormat); + assert.strictEqual(actual.documentation.value, expected.documentation); + assert.strictEqual(actual.documentation.kind, docFormat); } else { assert.fail( `Unexpected type of contents object "${actual.documentation}", should be MarkupContent.` @@ -1551,4 +1547,10 @@ export class TestState { } } } + + protected verifyCompletionItem(expected: _.FourSlashCompletionItem, actual: CompletionItem) { + assert.strictEqual(actual.label, expected.label); + assert.strictEqual(actual.detail, expected.detail); + assert.strictEqual(actual.kind, expected.kind); + } } diff --git a/packages/vscode-pyright/src/extension.ts b/packages/vscode-pyright/src/extension.ts index eb444ae66..fe59c9c44 100644 --- a/packages/vscode-pyright/src/extension.ts +++ b/packages/vscode-pyright/src/extension.ts @@ -37,6 +37,7 @@ import { } from 'vscode-languageclient/node'; import { Commands } from 'pyright-internal/commands/commands'; +import { isThenable } from 'pyright-internal/common/core'; import { FileBasedCancellationStrategy } from './cancellationUtils'; import { ProgressReporting } from './progress'; @@ -279,10 +280,6 @@ async function getPythonPathFromPythonExtension( return undefined; } -function isThenable(v: any): v is Thenable { - return typeof v?.then === 'function'; -} - function installPythonPathChangedListener( onDidChangeExecutionDetails: (callback: () => void) => void, scopeUri: Uri | undefined,