Proposal: If there’s a package.json, only auto-import things in it, more or less (microsoft#31893)

* Move package.json related utils to utilities

* Add failing test

* Make first test pass

* Don’t filter when there’s no package.json, fix scoped package imports

* Use type acquisition as a heuristic for whether a JS project is using node core

* Make same fix in getCompletionDetails

* Fix re-exporting

* Change JS node core module heuristic to same-file utilization

* Remove unused method

* Remove other unused method

* Remove unused triple-slash ref

* Update comment

* Refactor findAlias to forEachAlias to reduce iterations

* Really fix re-exporting

* Use getModuleSpecifier instead of custom hack

* Fix offering auto imports to paths within node modules

* Rename things and make comments better

* Add another reexport test

* Inline `symbolHasBeenSeen`

* Simplify forEachAlias to findAlias

* Add note that symbols is mutated

* Symbol order doesn’t matter here

* Style nits

* Add test with nested package.jsons

* Fix and add tests for export * re-exports

Author: andrewbranch

src/compiler/utilities.ts

@@ -7468,7 +7468,7 @@ namespace ts {
     export function getDirectoryPath(path: Path): Path;
     /**
      * Returns the path except for its basename. Semantics align with NodeJS's `path.dirname`
-     * except that we support URL's as well.
+     * except that we support URLs as well.
      *
      * ```ts
      * getDirectoryPath("/path/to/file.ext") === "/path/to"

src/harness/fourslash.ts

@@ -798,7 +798,7 @@ namespace FourSlash {
                         const name = typeof include === "string" ? include : include.name;
                         const found = nameToEntries.get(name);
                         if (!found) throw this.raiseError(`No completion ${name} found`);
-                        assert(found.length === 1); // Must use 'exact' for multiple completions with same name
+                        assert(found.length === 1, `Must use 'exact' for multiple completions with same name: '${name}'`);
                         this.verifyCompletionEntry(ts.first(found), include);
                     }
                 }

src/services/codefixes/importFixes.ts

@@ -283,13 +283,25 @@ namespace ts.codefix {
         preferences: UserPreferences,
     ): ReadonlyArray<FixAddNewImport | FixUseImportType> {
         const isJs = isSourceFileJS(sourceFile);
+        const { allowsImporting } = createLazyPackageJsonDependencyReader(sourceFile, host);
         const choicesForEachExportingModule = flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) =>
             moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences, program.redirectTargetsMap)
             .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.
                 exportedSymbolIsTypeOnly && isJs ? { kind: ImportFixKind.ImportType, moduleSpecifier, position: Debug.assertDefined(position) } : { kind: ImportFixKind.AddNew, moduleSpecifier, importKind }));
-        // Sort to keep the shortest paths first
-        return sort(choicesForEachExportingModule, (a, b) => a.moduleSpecifier.length - b.moduleSpecifier.length);
+
+        // Sort by presence in package.json, then shortest paths first
+        return sort(choicesForEachExportingModule, (a, b) => {
+            const allowsImportingA = allowsImporting(a.moduleSpecifier);
+            const allowsImportingB = allowsImporting(b.moduleSpecifier);
+            if (allowsImportingA && !allowsImportingB) {
+                return -1;
+            }
+            if (allowsImportingB && !allowsImportingA) {
+                return 1;
+            }
+            return a.moduleSpecifier.length - b.moduleSpecifier.length;
+        });
     }
 
     function getFixesForAddImport(
@@ -380,7 +392,8 @@ namespace ts.codefix {
         // "default" is a keyword and not a legal identifier for the import, so we don't expect it here
         Debug.assert(symbolName !== InternalSymbolName.Default);
 
-        const fixes = arrayFrom(flatMapIterator(getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program).entries(), ([_, exportInfos]) =>
+        const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program, preferences, host);
+        const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
             getFixForImport(exportInfos, symbolName, symbolToken.getStart(sourceFile), program, sourceFile, host, preferences)));
         return { fixes, symbolName };
     }
@@ -393,14 +406,16 @@ namespace ts.codefix {
         sourceFile: SourceFile,
         checker: TypeChecker,
         program: Program,
+        preferences: UserPreferences,
+        host: LanguageServiceHost
     ): ReadonlyMap<ReadonlyArray<SymbolExportInfo>> {
         // For each original symbol, keep all re-exports of that symbol together so we can call `getCodeActionsForImport` on the whole group at once.
         // Maps symbol id to info for modules providing that symbol (original export + re-exports).
         const originalSymbolToExportInfos = createMultiMap<SymbolExportInfo>();
         function addSymbol(moduleSymbol: Symbol, exportedSymbol: Symbol, importKind: ImportKind): void {
             originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { moduleSymbol, importKind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exportedSymbol, checker) });
         }
-        forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => {
+        forEachExternalModuleToImportFrom(checker, host, preferences, program.redirectTargetsMap, sourceFile, program.getSourceFiles(), moduleSymbol => {
             cancellationToken.throwIfCancellationRequested();
 
             const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions());
@@ -561,12 +576,44 @@ namespace ts.codefix {
         return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning));
     }
 
-    export function forEachExternalModuleToImportFrom(checker: TypeChecker, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
+    export function forEachExternalModuleToImportFrom(checker: TypeChecker, host: LanguageServiceHost, preferences: UserPreferences, redirectTargetsMap: RedirectTargetsMap, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
+        const { allowsImporting } = createLazyPackageJsonDependencyReader(from, host);
+        const compilerOptions = host.getCompilationSettings();
+        const getCanonicalFileName = hostGetCanonicalFileName(host);
         forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => {
-            if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
+            if (sourceFile === undefined && allowsImporting(stripQuotes(module.getName()))) {
                 cb(module);
             }
+            else if (sourceFile && sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
+                const moduleSpecifier = getNodeModulesPackageNameFromFileName(sourceFile.fileName);
+                if (!moduleSpecifier || allowsImporting(moduleSpecifier)) {
+                    cb(module);
+                }
+            }
         });
+
+        function getNodeModulesPackageNameFromFileName(importedFileName: string): string | undefined {
+            const specifier = moduleSpecifiers.getModuleSpecifier(
+                compilerOptions,
+                from,
+                toPath(from.fileName, /*basePath*/ undefined, getCanonicalFileName),
+                importedFileName,
+                host,
+                allSourceFiles,
+                preferences,
+                redirectTargetsMap);
+
+            // Paths here are not node_modules, so we don’t care about them;
+            // returning anything will trigger a lookup in package.json.
+            if (!pathIsRelative(specifier) && !isRootedDiskPath(specifier)) {
+                const components = getPathComponents(getPackageNameFromTypesPackageName(specifier)).slice(1);
+                // Scoped packages
+                if (startsWith(components[0], "@")) {
+                    return `${components[0]}/${components[1]}`;
+                }
+                return components[0];
+            }
+        }
     }
 
     function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) {
@@ -620,4 +667,69 @@ namespace ts.codefix {
         // Need `|| "_"` to ensure result isn't empty.
         return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
     }
+
+    function createLazyPackageJsonDependencyReader(fromFile: SourceFile, host: LanguageServiceHost) {
+        const packageJsonPaths = findPackageJsons(getDirectoryPath(fromFile.fileName), host);
+        const dependencyIterator = readPackageJsonDependencies(host, packageJsonPaths);
+        let seenDeps: Map<true> | undefined;
+        let usesNodeCoreModules: boolean | undefined;
+        return { allowsImporting };
+
+        function containsDependency(dependency: string) {
+            if ((seenDeps || (seenDeps = createMap())).has(dependency)) {
+                return true;
+            }
+            let packageName: string | void;
+            while (packageName = dependencyIterator.next().value) {
+                seenDeps.set(packageName, true);
+                if (packageName === dependency) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        function allowsImporting(moduleSpecifier: string): boolean {
+            if (!packageJsonPaths.length) {
+                return true;
+            }
+
+            // If we’re in JavaScript, it can be difficult to tell whether the user wants to import
+            // from Node core modules or not. We can start by seeing if the user is actually using
+            // any node core modules, as opposed to simply having @types/node accidentally as a
+            // dependency of a dependency.
+            if (isSourceFileJS(fromFile) && JsTyping.nodeCoreModules.has(moduleSpecifier)) {
+                if (usesNodeCoreModules === undefined) {
+                    usesNodeCoreModules = consumesNodeCoreModules(fromFile);
+                }
+                if (usesNodeCoreModules) {
+                    return true;
+                }
+            }
+
+            return containsDependency(moduleSpecifier)
+                || containsDependency(getTypesPackageName(moduleSpecifier));
+        }
+    }
+
+    function *readPackageJsonDependencies(host: LanguageServiceHost, packageJsonPaths: string[]) {
+        type PackageJson = Record<typeof dependencyKeys[number], Record<string, string> | undefined>;
+        const dependencyKeys = ["dependencies", "devDependencies", "optionalDependencies"] as const;
+        for (const fileName of packageJsonPaths) {
+            const content = readJson(fileName, { readFile: host.readFile ? host.readFile.bind(host) : sys.readFile }) as PackageJson;
+            for (const key of dependencyKeys) {
+                const dependencies = content[key];
+                if (!dependencies) {
+                    continue;
+                }
+                for (const packageName in dependencies) {
+                    yield packageName;
+                }
+            }
+        }
+    }
+
+    function consumesNodeCoreModules(sourceFile: SourceFile): boolean {
+        return some(sourceFile.imports, ({ text }) => JsTyping.nodeCoreModules.has(text));
+    }
 }