Filter auto imports by symbol flags before resolving module specifiers (microsoft#45168)

* Filter auto imports by symbol flags before resolving module specifiers

* Don’t filter types out of import statement completions

Author: andrewbranch

src/services/codefixes/importFixes.ts

@@ -231,11 +231,11 @@ namespace ts.codefix {
         function getInfoWithChecker(checker: TypeChecker, isFromPackageJson: boolean): SymbolExportInfo | undefined {
             const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
             if (defaultInfo && skipAlias(defaultInfo.symbol, checker) === symbol) {
-                return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, isTypeOnly: isTypeOnlySymbol(symbol, checker), isFromPackageJson };
+                return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson };
             }
             const named = checker.tryGetMemberInModuleExportsAndProperties(symbol.name, moduleSymbol);
             if (named && skipAlias(named, checker) === symbol) {
-                return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, isTypeOnly: isTypeOnlySymbol(symbol, checker), isFromPackageJson };
+                return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson };
             }
         }
     }
@@ -256,12 +256,12 @@ namespace ts.codefix {
 
             const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
             if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target) === symbolName) && skipAlias(defaultInfo.symbol, checker) === exportedSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
-                result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, isTypeOnly: isTypeOnlySymbol(defaultInfo.symbol, checker), isFromPackageJson });
+                result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson });
             }
 
             for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) {
                 if (exported.name === symbolName && skipAlias(exported, checker) === exportedSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
-                    result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, isTypeOnly: isTypeOnlySymbol(exported, checker), isFromPackageJson });
+                    result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson });
                 }
             }
         });
@@ -294,10 +294,6 @@ namespace ts.codefix {
         return result && { ...result, computedWithoutCacheCount };
     }
 
-    function isTypeOnlySymbol(s: Symbol, checker: TypeChecker): boolean {
-        return !(skipAlias(s, checker).flags & SymbolFlags.Value);
-    }
-
     function isTypeOnlyPosition(sourceFile: SourceFile, position: number) {
         return isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
     }
@@ -395,9 +391,9 @@ namespace ts.codefix {
         });
     }
 
-    function getExistingImportDeclarations({ moduleSymbol, exportKind, isTypeOnly: exportedSymbolIsTypeOnly }: SymbolExportInfo, checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions): readonly FixAddToExistingImportInfo[] {
+    function getExistingImportDeclarations({ moduleSymbol, exportKind, targetFlags }: SymbolExportInfo, checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions): readonly FixAddToExistingImportInfo[] {
         // Can't use an es6 import for a type in JS.
-        if (exportedSymbolIsTypeOnly && isSourceFileJS(importingFile)) return emptyArray;
+        if (!(targetFlags & SymbolFlags.Value) && isSourceFileJS(importingFile)) return emptyArray;
         const importKind = getImportKind(importingFile, exportKind, compilerOptions);
         return mapDefined(importingFile.imports, (moduleSpecifier): FixAddToExistingImportInfo | undefined => {
             const i = importFromModuleSpecifier(moduleSpecifier);
@@ -462,7 +458,7 @@ namespace ts.codefix {
             computedWithoutCacheCount += computedWithoutCache ? 1 : 0;
             return moduleSpecifiers?.map((moduleSpecifier): FixAddNewImport | FixUseImportType =>
                 // `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types.
-                exportInfo.isTypeOnly && isJs && position !== undefined
+                !(exportInfo.targetFlags & SymbolFlags.Value) && isJs && position !== undefined
                     ? { kind: ImportFixKind.ImportType, moduleSpecifier, position, exportInfo }
                     : {
                         kind: ImportFixKind.AddNew,
@@ -547,7 +543,7 @@ namespace ts.codefix {
         if (!umdSymbol) return undefined;
         const symbol = checker.getAliasedSymbol(umdSymbol);
         const symbolName = umdSymbol.name;
-        const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, isTypeOnly: false, isFromPackageJson: false }];
+        const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false }];
         const useRequire = shouldUseRequire(sourceFile, program);
         const fixes = getImportFixes(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*preferTypeOnlyImport*/ false, useRequire, program, sourceFile, host, preferences);
         return { fixes, symbolName };
@@ -653,7 +649,7 @@ namespace ts.codefix {
                 !toFile && packageJsonFilter.allowsImportingAmbientModule(moduleSymbol, moduleSpecifierResolutionHost)
             ) {
                 const checker = program.getTypeChecker();
-                originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, isTypeOnly: isTypeOnlySymbol(exportedSymbol, checker), isFromPackageJson });
+                originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson });
             }
         }
         forEachExternalModuleToImportFrom(program, host, useAutoImportProvider, (moduleSymbol, sourceFile, program, isFromPackageJson) => {

src/services/completions.ts

@@ -1452,7 +1452,7 @@ namespace ts.Completions {
         const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
         const symbolToSortTextIdMap: SymbolSortTextIdMap = [];
         const seenPropertySymbols = new Map<SymbolId, true>();
-        const isTypeOnly = isTypeOnlyCompletion();
+        const isTypeOnlyLocation = isTypeOnlyCompletion();
         const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
             return createModuleSpecifierResolutionHost(isFromPackageJson ? host.getPackageJsonAutoImportProvider!()! : program, host);
         });
@@ -1506,7 +1506,7 @@ namespace ts.Completions {
             isJsxInitializer,
             insideJsDocTagTypeExpression,
             symbolToSortTextIdMap,
-            isTypeOnlyLocation: isTypeOnly,
+            isTypeOnlyLocation,
             isJsxIdentifierExpected,
             importCompletionNode,
             hasUnresolvedAutoImports,
@@ -1806,7 +1806,7 @@ namespace ts.Completions {
             const scopeNode = getScopeNode(contextToken, adjustedPosition, sourceFile) || sourceFile;
             isInSnippetScope = isSnippetScope(scopeNode);
 
-            const symbolMeanings = (isTypeOnly ? SymbolFlags.None : SymbolFlags.Value) | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias;
+            const symbolMeanings = (isTypeOnlyLocation ? SymbolFlags.None : SymbolFlags.Value) | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias;
 
             symbols = concatenate(symbols, typeChecker.getSymbolsInScope(scopeNode, symbolMeanings));
             Debug.assertEachIsDefined(symbols, "getSymbolsInScope() should all be defined");
@@ -1829,7 +1829,7 @@ namespace ts.Completions {
                 }
             }
             collectAutoImports();
-            if (isTypeOnly) {
+            if (isTypeOnlyLocation) {
                 keywordFilters = contextToken && isAssertionExpression(contextToken.parent)
                     ? KeywordCompletionFilters.TypeAssertionKeywords
                     : KeywordCompletionFilters.TypeKeywords;
@@ -1931,6 +1931,9 @@ namespace ts.Completions {
                 context => {
                     exportInfo.forEach(sourceFile.path, (info, symbolName, isFromAmbientModule) => {
                         if (!detailsEntryId && isStringANonContextualKeyword(symbolName)) return;
+                        // `targetFlags` should be the same for each `info`
+                        if (!isTypeOnlyLocation && !importCompletionNode && !(info[0].targetFlags & SymbolFlags.Value)) return;
+                        if (isTypeOnlyLocation && !(info[0].targetFlags & (SymbolFlags.Module | SymbolFlags.Type))) return;
                         const isCompletionDetailsMatch = detailsEntryId && some(info, i => detailsEntryId.source === stripQuotes(i.moduleSymbol.name));
                         if (isCompletionDetailsMatch || !detailsEntryId && charactersFuzzyMatchInString(symbolName, lowerCaseTokenText)) {
                             const defaultExportInfo = find(info, isImportableExportInfo);

src/services/exportInfoMap.ts

@@ -20,8 +20,7 @@ namespace ts {
         /** Set if `moduleSymbol` is an external module, not an ambient module */
         moduleFileName: string | undefined;
         exportKind: ExportKind;
-        /** If true, can't use an es6 import from a js file. */
-        isTypeOnly: boolean;
+        targetFlags: SymbolFlags;
         /** True if export was only found via the package.json AutoImportProvider (for telemetry). */
         isFromPackageJson: boolean;
     }
@@ -38,7 +37,7 @@ namespace ts {
         readonly moduleSymbol: Symbol | undefined;
         moduleFileName: string | undefined;
         exportKind: ExportKind;
-        isTypeOnly: boolean;
+        targetFlags: SymbolFlags;
         isFromPackageJson: boolean;
     }
 
@@ -92,7 +91,7 @@ namespace ts {
                     moduleFile,
                     moduleFileName: moduleFile?.fileName,
                     exportKind,
-                    isTypeOnly: !(skipAlias(symbol, checker).flags & SymbolFlags.Value),
+                    targetFlags: skipAlias(symbol, checker).flags,
                     isFromPackageJson,
                     symbol: storedSymbol,
                     moduleSymbol: storedModuleSymbol,
@@ -143,15 +142,15 @@ namespace ts {
 
         function rehydrateCachedInfo(info: CachedSymbolExportInfo): SymbolExportInfo {
             if (info.symbol && info.moduleSymbol) return info as SymbolExportInfo;
-            const { id, exportKind, isTypeOnly, isFromPackageJson, moduleFileName } = info;
+            const { id, exportKind, targetFlags, isFromPackageJson, moduleFileName } = info;
             const [cachedSymbol, cachedModuleSymbol] = symbols.get(id) || emptyArray;
             if (cachedSymbol && cachedModuleSymbol) {
                 return {
                     symbol: cachedSymbol,
                     moduleSymbol: cachedModuleSymbol,
                     moduleFileName,
                     exportKind,
-                    isTypeOnly,
+                    targetFlags,
                     isFromPackageJson,
                 };
             }
@@ -173,7 +172,7 @@ namespace ts {
                 moduleSymbol,
                 moduleFileName,
                 exportKind,
-                isTypeOnly,
+                targetFlags,
                 isFromPackageJson,
             };
         }

tests/cases/fourslash/importStatementCompletions1.ts

@@ -2,6 +2,7 @@
 
 // @Filename: /mod.ts
 //// export const foo = 0;
+//// export type Foo = number;
 
 // @Filename: /index0.ts
 //// [|import f/*0*/|]
@@ -32,6 +33,13 @@
       isSnippet: true,
       replacementSpan: test.ranges()[marker],
       sourceDisplay: "./mod",
+    }, {
+      name: "Foo",
+      source: "./mod",
+      insertText: `import { Foo$1 } from "./mod";`,
+      isSnippet: true,
+      replacementSpan: test.ranges()[marker],
+      sourceDisplay: "./mod",
     }],
     preferences: {
       includeCompletionsForImportStatements: true,