Store symbol table map key in CachedSymbolExportInfo (#45289)

andrewbranch · web-flow · commit f80bc3f5f079 · 2021-08-04T16:31:47.000-07:00
* Store symbol table map key in CachedSymbolExportInfo

* Remove debug assertion

* Filter out known symbols (again) and private identifiers
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -581,6 +581,7 @@ namespace ts {
             getEmitResolver,
             getExportsOfModule: getExportsOfModuleAsArray,
             getExportsAndPropertiesOfModule,
+            forEachExportAndPropertyOfModule,
             getSymbolWalker: createGetSymbolWalker(
                 getRestTypeOfSignature,
                 getTypePredicateOfSignature,
@@ -3532,6 +3533,24 @@ namespace ts {
             return exports;
         }
 
+        function forEachExportAndPropertyOfModule(moduleSymbol: Symbol, cb: (symbol: Symbol, key: __String) => void): void {
+            const exports = getExportsOfModule(moduleSymbol);
+            exports.forEach((symbol, key) => {
+                if (!isReservedMemberName(key)) {
+                    cb(symbol, key);
+                }
+            });
+            const exportEquals = resolveExternalModuleSymbol(moduleSymbol);
+            if (exportEquals !== moduleSymbol) {
+                const type = getTypeOfSymbol(exportEquals);
+                if (shouldTreatPropertiesOfExternalModuleAsExports(type)) {
+                    getPropertiesOfType(type).forEach(symbol => {
+                        cb(symbol, symbol.escapedName);
+                    });
+                }
+            }
+        }
+
         function tryGetMemberInModuleExports(memberName: __String, moduleSymbol: Symbol): Symbol | undefined {
             const symbolTable = getExportsOfModule(moduleSymbol);
             if (symbolTable) {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -4225,6 +4225,7 @@ namespace ts {
         getExportsOfModule(moduleSymbol: Symbol): Symbol[];
         /** Unlike `getExportsOfModule`, this includes properties of an `export =` value. */
         /* @internal */ getExportsAndPropertiesOfModule(moduleSymbol: Symbol): Symbol[];
+        /* @internal */ forEachExportAndPropertyOfModule(moduleSymbol: Symbol, cb: (symbol: Symbol, key: __String) => void): void;
         getJsxIntrinsicTagNamesAt(location: Node): Symbol[];
         isOptionalParameter(node: ParameterDeclaration): boolean;
         getAmbientModules(): Symbol[];
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -3289,6 +3289,10 @@ namespace ts {
         return startsWith(symbol.escapedName as string, "__@");
     }
 
+    export function isPrivateIdentifierSymbol(symbol: Symbol): boolean {
+        return startsWith(symbol.escapedName as string, "__#");
+    }
+
     /**
      * Includes the word "Symbol" with unicode escapes
      */
diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -1934,6 +1934,7 @@ namespace ts.Completions {
                 !!importCompletionNode,
                 context => {
                     exportInfo.forEach(sourceFile.path, (info, symbolName, isFromAmbientModule) => {
+                        if (!isIdentifierText(symbolName, getEmitScriptTarget(host.getCompilationSettings()))) return;
                         if (!detailsEntryId && isStringANonContextualKeyword(symbolName)) return;
                         // `targetFlags` should be the same for each `info`
                         if (!isTypeOnlyLocation && !importCompletionNode && !(info[0].targetFlags & SymbolFlags.Value)) return;
diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts
@@ -29,6 +29,7 @@ namespace ts {
         // Used to rehydrate `symbol` and `moduleSymbol` when transient
         id: number;
         symbolName: string;
+        symbolTableKey: __String;
         moduleName: string;
         moduleFile: SourceFile | undefined;
 
@@ -44,7 +45,7 @@ namespace ts {
     export interface ExportInfoMap {
         isUsableByFile(importingFile: Path): boolean;
         clear(): void;
-        add(importingFile: Path, symbol: Symbol, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, scriptTarget: ScriptTarget, checker: TypeChecker): void;
+        add(importingFile: Path, symbol: Symbol, key: __String, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, scriptTarget: ScriptTarget, checker: TypeChecker): void;
         get(importingFile: Path, importedName: string, symbol: Symbol, moduleName: string, checker: TypeChecker): readonly SymbolExportInfo[] | undefined;
         forEach(importingFile: Path, action: (info: readonly SymbolExportInfo[], name: string, isFromAmbientModule: boolean) => void): void;
         releaseSymbols(): void;
@@ -71,13 +72,19 @@ namespace ts {
                 symbols.clear();
                 usableByFileName = undefined;
             },
-            add: (importingFile, symbol, moduleSymbol, moduleFile, exportKind, isFromPackageJson, scriptTarget, checker) => {
+            add: (importingFile, symbol, symbolTableKey, moduleSymbol, moduleFile, exportKind, isFromPackageJson, scriptTarget, checker) => {
                 if (importingFile !== usableByFileName) {
                     cache.clear();
                     usableByFileName = importingFile;
                 }
                 const isDefault = exportKind === ExportKind.Default;
-                const importedName = getNameForExportedSymbol(isDefault && getLocalSymbolForExportDefault(symbol) || symbol, scriptTarget);
+                const namedSymbol = isDefault && getLocalSymbolForExportDefault(symbol) || symbol;
+                // A re-export merged with an export from a module augmentation can result in `symbol`
+                // being an external module symbol; the name it is re-exported by will be `symbolTableKey`
+                // (which comes from the keys of `moduleSymbol.exports`.)
+                const importedName = isExternalModuleSymbol(namedSymbol)
+                    ? unescapeLeadingUnderscores(symbolTableKey)
+                    : getNameForExportedSymbol(namedSymbol, scriptTarget);
                 const moduleName = stripQuotes(moduleSymbol.name);
                 const id = exportInfoId++;
                 const storedSymbol = symbol.flags & SymbolFlags.Transient ? undefined : symbol;
@@ -86,6 +93,7 @@ namespace ts {
 
                 exportInfo.add(key(importedName, symbol, moduleName, checker), {
                     id,
+                    symbolTableKey,
                     symbolName: importedName,
                     moduleName,
                     moduleFile,
@@ -160,12 +168,10 @@ namespace ts {
             const moduleSymbol = info.moduleSymbol || cachedModuleSymbol || Debug.checkDefined(info.moduleFile
                 ? checker.getMergedSymbol(info.moduleFile.symbol)
                 : checker.tryFindAmbientModule(info.moduleName));
-            const symbolName = exportKind === ExportKind.Default
-                ? InternalSymbolName.Default
-                : info.symbolName;
             const symbol = info.symbol || cachedSymbol || Debug.checkDefined(exportKind === ExportKind.ExportEquals
                 ? checker.resolveExternalModuleSymbol(moduleSymbol)
-                : checker.tryGetMemberInModuleExportsAndProperties(symbolName, moduleSymbol));
+                : checker.tryGetMemberInModuleExportsAndProperties(unescapeLeadingUnderscores(info.symbolTableKey), moduleSymbol),
+                `Could not find symbol '${info.symbolName}' by key '${info.symbolTableKey}' in module ${moduleSymbol.name}`);
             symbols.set(id, [symbol, moduleSymbol]);
             return {
                 symbol,
@@ -330,30 +336,32 @@ namespace ts {
             const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
             // Note: I think we shouldn't actually see resolved module symbols here, but weird merges
             // can cause it to happen: see 'completionsImport_mergedReExport.ts'
-            if (defaultInfo && !checker.isUndefinedSymbol(defaultInfo.symbol) && !isExternalModuleSymbol(defaultInfo.symbol)) {
+            if (defaultInfo && isImportableSymbol(defaultInfo.symbol, checker)) {
                 cache.add(
                     importingFile.path,
                     defaultInfo.symbol,
+                    defaultInfo.exportKind === ExportKind.Default ? InternalSymbolName.Default : InternalSymbolName.ExportEquals,
                     moduleSymbol,
                     moduleFile,
                     defaultInfo.exportKind,
                     isFromPackageJson,
                     scriptTarget,
                     checker);
             }
-            for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) {
-                if (exported !== defaultInfo?.symbol && !isKnownSymbol(exported) && !isExternalModuleSymbol(exported) && addToSeen(seenExports, exported)) {
+            checker.forEachExportAndPropertyOfModule(moduleSymbol, (exported, key) => {
+                if (exported !== defaultInfo?.symbol && isImportableSymbol(exported, checker) && addToSeen(seenExports, exported)) {
                     cache.add(
                         importingFile.path,
                         exported,
+                        key,
                         moduleSymbol,
                         moduleFile,
                         ExportKind.Named,
                         isFromPackageJson,
                         scriptTarget,
                         checker);
                 }
-            }
+            });
         });
 
         host.log?.(`getExportInfoMap: done in ${timestamp() - start} ms`);
@@ -368,6 +376,10 @@ namespace ts {
         return info && { symbol, exportKind, ...info };
     }
 
+    function isImportableSymbol(symbol: Symbol, checker: TypeChecker) {
+        return !checker.isUndefinedSymbol(symbol) && !checker.isUnknownSymbol(symbol) && !isKnownSymbol(symbol) && !isPrivateIdentifierSymbol(symbol);
+    }
+
     function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly exportKind: ExportKind } | undefined {
         const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol);
         if (exportEquals !== moduleSymbol) return { symbol: exportEquals, exportKind: ExportKind.ExportEquals };
diff --git a/tests/cases/fourslash/server/completionsImport_mergedReExport.ts b/tests/cases/fourslash/server/completionsImport_mergedReExport.ts
@@ -30,26 +30,29 @@
 
 verify.completions({
   marker: "",
+  includes: [{
+    name: "Config",
+    source: "/node_modules/@jest/types/index",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }],
   preferences: {
     includeCompletionsForModuleExports: true,
   },
 });
 
 edit.insert("o");
 
-// Should not crash
 verify.completions({
   marker: "",
+  includes: [{
+    name: "Config",
+    source: "@jest/types",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }],
   preferences: {
     includeCompletionsForModuleExports: true,
+    allowIncompleteCompletions: true,
   },
 });
-
-// Because of the way `Config` is merged, we are actually not including it
-// in completions here, though it would be better if we could. The `exports`
-// of "@jest/types/index" would contain an alias symbol named `Config` without
-// the merge from ts-jest, but with the merge, the `exports` contains the merge
-// of `namespace Config` and the "@jest/types/Config" module symbol. This is
-// unexpected (to me) and difficult to work with, and might be wrong? My
-// expectation would have been to preserve the export alias symbol, but let it
-// *resolve* to the merge of the SourceFile and the namespace declaration.
