Don't suggest import completions for /a/node_modules if we're in /b (#19928)

* Don't suggest import completions for /a/node_modules if we're in /b

* Remove test for delving into dependencies of dependencies
This commit is contained in:
Andy 2017-11-30 11:25:01 -08:00 committed by GitHub
parent 4c7b49156f
commit 75e5b13775
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 18 deletions

View file

@ -743,11 +743,7 @@ namespace ts.codefix {
const symbolIdActionMap = new ImportCodeActionMap();
const currentTokenMeaning = getMeaningFromLocation(symbolToken);
forEachExternalModule(checker, allSourceFiles, moduleSymbol => {
if (moduleSymbol === sourceFile.symbol) {
return;
}
forEachExternalModuleToImportFrom(checker, sourceFile, allSourceFiles, moduleSymbol => {
cancellationToken.throwIfCancellationRequested();
// check the default export
const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol);
@ -776,17 +772,35 @@ namespace ts.codefix {
return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning));
}
export function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
export function forEachExternalModuleToImportFrom(checker: TypeChecker, from: SourceFile, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol) => void) {
forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => {
if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
cb(module);
}
});
}
export function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) {
for (const ambient of checker.getAmbientModules()) {
cb(ambient);
cb(ambient, /*sourceFile*/ undefined);
}
for (const sourceFile of allSourceFiles) {
if (isExternalOrCommonJsModule(sourceFile)) {
cb(sourceFile.symbol);
cb(sourceFile.symbol, sourceFile);
}
}
}
/**
* Don't include something from a `node_modules` that isn't actually reachable by a global import.
* A relative import to node_modules is usually a bad idea.
*/
function isImportablePath(fromPath: string, toPath: string): boolean {
// If it's in a `node_modules` but is not reachable from here via a global import, don't bother.
const toNodeModules = forEachAncestorDirectory(toPath, ancestor => getBaseFileName(ancestor) === "node_modules" ? ancestor : undefined);
return toNodeModules === undefined || startsWith(fromPath, getDirectoryPath(toNodeModules));
}
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string {
return moduleSpecifierToValidIdentifier(removeFileExtension(getBaseFileName(moduleSymbol.name)), target);
}

View file

@ -1031,11 +1031,7 @@ namespace ts.Completions {
function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
const tokenTextLowerCase = tokenText.toLowerCase();
codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => {
if (moduleSymbol === sourceFile.symbol) {
return;
}
codefix.forEachExternalModuleToImportFrom(typeChecker, sourceFile, allSourceFiles, moduleSymbol => {
for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) {
let { name } = symbol;

View file

@ -0,0 +1,10 @@
/// <reference path="fourslash.ts" />
// @Filename: /unrelated/node_modules/@types/foo/index.d.ts
////export function foo() {}
// @Filename: /src/b.ts
////fo/**/;
goTo.marker("");
verify.not.completionListContains({ name: "foo", source: "/unrelated/node_modules/@types/foo/index" }, undefined, undefined, undefined, undefined, undefined, { includeExternalModuleExports: true });

View file

@ -18,8 +18,5 @@
//// "types": "bin/lib/libfile.d.ts"
//// }
verify.importFixAtPosition([
`import { f1 } from "package-name/node_modules/package-name2";
f1('');`
]);
// No fix because this accesses a nested node_modules
verify.not.codeFixAvailable();