From c40ddb183e8c02fc27db6e1daf9dc8826da4f856 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Wed, 16 Oct 2019 17:02:35 -0400 Subject: [PATCH 1/2] Does not add a duplicate completion when offering an export which was re-declared as a global - fixes #32675 --- src/services/completions.ts | 10 ++++-- .../completionsRedeclareModuleAsGlobal.ts | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 1c3288a3282..35f03697dd0 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1000,6 +1000,7 @@ namespace ts.Completions { let completionKind = CompletionKind.None; let isNewIdentifierLocation = false; let keywordFilters = KeywordCompletionFilters.None; + // This also gets mutated in nested-functions after the return let symbols: Symbol[] = []; const symbolToOriginInfoMap: SymbolOriginInfoMap = []; const symbolToSortTextMap: SymbolSortTextMap = []; @@ -1342,9 +1343,12 @@ namespace ts.Completions { } const symbolId = getSymbolId(symbol); - symbols.push(symbol); - symbolToOriginInfoMap[symbolId] = origin; - symbolToSortTextMap[symbolId] = SortText.AutoImportSuggestions; + const existingSymbol = findLast(symbols, symbol => symbol.id === symbolId); + if (!existingSymbol) { + symbols.push(symbol); + symbolToOriginInfoMap[symbolId] = origin; + symbolToSortTextMap[symbolId] = SortText.AutoImportSuggestions; + } }); } filterGlobalCompletion(symbols); diff --git a/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts b/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts new file mode 100644 index 00000000000..4a17a0830ac --- /dev/null +++ b/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts @@ -0,0 +1,33 @@ +/// + +// @esModuleInterop: true, +// @target: esnext + +// @Filename: /myAssert.d.ts +////declare function assert(value:any, message?:string):void; +////export = assert; +////export as namespace assert; + +// @Filename: /ambient.d.ts +////import assert from './myAssert'; +//// +////type Assert = typeof assert; +//// +////declare global { +//// const assert: Assert; +////} + +// @Filename: /index.ts +/////// +////asser/**/; + +verify.completions({ + marker: "", + includes: [ + { + name: "assert", + sortText: completion.SortText.GlobalsOrKeywords + } + ], + preferences: { includeCompletionsForModuleExports: true, includeInsertTextCompletions: true } +}); From 85010fa6fefae471f6b70959deea81239195f46e Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Thu, 17 Oct 2019 12:12:41 -0400 Subject: [PATCH 2/2] Make sure that global module re-exports are short-cutted to be added to completions --- src/services/completions.ts | 13 +++++-------- .../fourslash/completionsRedeclareModuleAsGlobal.ts | 2 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 35f03697dd0..6f88dfea388 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1343,12 +1343,9 @@ namespace ts.Completions { } const symbolId = getSymbolId(symbol); - const existingSymbol = findLast(symbols, symbol => symbol.id === symbolId); - if (!existingSymbol) { - symbols.push(symbol); - symbolToOriginInfoMap[symbolId] = origin; - symbolToSortTextMap[symbolId] = SortText.AutoImportSuggestions; - } + symbols.push(symbol); + symbolToOriginInfoMap[symbolId] = origin; + symbolToSortTextMap[symbolId] = SortText.AutoImportSuggestions; }); } filterGlobalCompletion(symbols); @@ -1468,7 +1465,7 @@ namespace ts.Completions { } /** - * Gathers symbols that can be imported from other files, deduplicating along the way. Symbols can be “duplicates” + * Gathers symbols that can be imported from other files, de-duplicating along the way. Symbols can be "duplicates" * if re-exported from another module, e.g. `export { foo } from "./a"`. That syntax creates a fresh symbol, but * it’s just an alias to the first, and both have the same name, so we generally want to filter those aliases out, * if and only if the the first can be imported (it may be excluded due to package.json filtering in @@ -1552,7 +1549,7 @@ namespace ts.Completions { // Don't add another completion for `export =` of a symbol that's already global. // So in `declare namespace foo {} declare module "foo" { export = foo; }`, there will just be the global completion for `foo`. if (resolvedModuleSymbol !== moduleSymbol && - every(resolvedModuleSymbol.declarations, d => !!d.getSourceFile().externalModuleIndicator)) { + every(resolvedModuleSymbol.declarations, d => !!d.getSourceFile().externalModuleIndicator && !findAncestor(d, isGlobalScopeAugmentation))) { pushSymbol(resolvedModuleSymbol, moduleSymbol, /*skipFilter*/ true); } diff --git a/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts b/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts index 4a17a0830ac..3dd528cde17 100644 --- a/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts +++ b/tests/cases/fourslash/completionsRedeclareModuleAsGlobal.ts @@ -1,5 +1,7 @@ /// +// 32675 - if this fails there are two copies of assert in completions + // @esModuleInterop: true, // @target: esnext