From 253de7c622bd6f44e2afab5f937db093ac3dbb36 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Tue, 17 Aug 2021 13:39:03 +0200 Subject: [PATCH 1/2] Sort auto-imports by import symbol type --- .../src/analyzer/importStatementUtils.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/importStatementUtils.ts b/packages/pyright-internal/src/analyzer/importStatementUtils.ts index e38420dbcc1d..3d3e14309abf 100644 --- a/packages/pyright-internal/src/analyzer/importStatementUtils.ts +++ b/packages/pyright-internal/src/analyzer/importStatementUtils.ts @@ -129,6 +129,18 @@ export function getTopLevelImports(parseTree: ModuleNode, includeImplicitImports return localImports; } +// Return import symbol type to allow sorting similar to isort +// CONSTANT_VARIABLE, CamelCaseClass, variable_or_function +function _getImportSymbolNameType(symbolName: string): number { + if (SymbolNameUtils.isConstantName(symbolName)) { + return 0; + } + if (SymbolNameUtils.isTypeAliasName(symbolName)) { + return 1; + } + return 2; +} + export function getTextEditsForAutoImportSymbolAddition( symbolName: string, importStatement: ImportStatement, @@ -145,9 +157,15 @@ export function getTextEditsForAutoImportSymbolAddition( // Make sure we're not attempting to auto-import a symbol that // already exists in the import list. if (!importStatement.node.imports.some((importAs) => importAs.name.value === symbolName)) { - // Do our best to insert the new symbol in alphabetical order. + // Insert new symbol by import symbol type and then alphabetical order. + // Match isort default behavior. + const symbolNameType = _getImportSymbolNameType(symbolName); for (const curImport of importStatement.node.imports) { - if (curImport.name.value > symbolName) { + const curImportNameType = _getImportSymbolNameType(curImport.name.value); + if ( + (curImportNameType === symbolNameType && curImport.name.value > symbolName) || + curImportNameType > symbolNameType + ) { break; } From 91d551b203eca4621f05da2a2a7bf0dcbef1a778 Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Wed, 18 Aug 2021 00:14:06 +0200 Subject: [PATCH 2/2] Add test --- ...letions.autoimport.fromImport.fourslash.ts | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 packages/pyright-internal/src/tests/fourslash/completions.autoimport.fromImport.fourslash.ts diff --git a/packages/pyright-internal/src/tests/fourslash/completions.autoimport.fromImport.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/completions.autoimport.fromImport.fourslash.ts new file mode 100644 index 000000000000..af54cc6a390c --- /dev/null +++ b/packages/pyright-internal/src/tests/fourslash/completions.autoimport.fromImport.fourslash.ts @@ -0,0 +1,100 @@ +/// + +// @filename: test.py +//// from lib import ( +//// [|/*import1*/|]MY_CONST_VAR[|/*import2*/|], +//// ClsOther[|/*import3*/|], +//// my_afunc[|/*import4*/|], +//// ) +//// +//// [|A_CONST/*marker1*/|] +//// [|MY_CONST_VAR2/*marker2*/|] +//// [|Cls/*marker3*/|] +//// [|ClsList/*marker4*/|] +//// [|my_func/*marker5*/|] + +// @filename: lib.py +//// A_CONST = 1 +//// MY_CONST_VAR = 2 +//// MY_CONST_VAR2 = 3 +//// class Cls: ... +//// class ClsOther: ... +//// ClsOtherList = list[ClsOther] +//// def my_afunc(): ... +//// def my_func(): ... + +{ + const import1Range = helper.getPositionRange('import1'); + const import2Range = helper.getPositionRange('import2'); + const import3Range = helper.getPositionRange('import3'); + const import4Range = helper.getPositionRange('import4'); + const marker1Range = helper.getPositionRange('marker1'); + const marker2Range = helper.getPositionRange('marker2'); + const marker3Range = helper.getPositionRange('marker3'); + const marker4Range = helper.getPositionRange('marker4'); + const marker5Range = helper.getPositionRange('marker5'); + + // @ts-ignore + await helper.verifyCompletion('included', 'markdown', { + marker1: { + completions: [ + { + label: 'A_CONST', + kind: Consts.CompletionItemKind.Constant, + documentation: '```\nfrom lib import A_CONST\n```', + detail: 'Auto-import', + textEdit: { range: marker1Range, newText: 'A_CONST' }, + additionalTextEdits: [{ range: import1Range, newText: 'A_CONST,\n ' }], + }, + ], + }, + marker2: { + completions: [ + { + label: 'MY_CONST_VAR2', + kind: Consts.CompletionItemKind.Constant, + documentation: '```\nfrom lib import MY_CONST_VAR2\n```', + detail: 'Auto-import', + textEdit: { range: marker2Range, newText: 'MY_CONST_VAR2' }, + additionalTextEdits: [{ range: import2Range, newText: ',\n MY_CONST_VAR2' }], + }, + ], + }, + marker3: { + completions: [ + { + label: 'Cls', + kind: Consts.CompletionItemKind.Class, + documentation: '```\nfrom lib import Cls\n```', + detail: 'Auto-import', + textEdit: { range: marker3Range, newText: 'Cls' }, + additionalTextEdits: [{ range: import2Range, newText: ',\n Cls' }], + }, + ], + }, + marker4: { + completions: [ + { + label: 'ClsOtherList', + kind: Consts.CompletionItemKind.Variable, + documentation: '```\nfrom lib import ClsOtherList\n```', + detail: 'Auto-import', + textEdit: { range: marker4Range, newText: 'ClsOtherList' }, + additionalTextEdits: [{ range: import3Range, newText: ',\n ClsOtherList' }], + }, + ], + }, + marker5: { + completions: [ + { + label: 'my_func', + kind: Consts.CompletionItemKind.Function, + documentation: '```\nfrom lib import my_func\n```', + detail: 'Auto-import', + textEdit: { range: marker5Range, newText: 'my_func' }, + additionalTextEdits: [{ range: import4Range, newText: ',\n my_func' }], + }, + ], + }, + }); +}