From 528f873b2d2ed8b9c25fa0e31239c7084a677d55 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 11 Jun 2019 16:44:23 -0700 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20filter=20when=20there=E2=80=99s?= =?UTF-8?q?=20no=20package.json,=20fix=20scoped=20package=20imports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/codefixes/importFixes.ts | 54 ++++++++++++------- ...t_filteredByPackageJson_@typesImplicit.ts} | 4 +- ...Import_filteredByPackageJson_@typesOnly.ts | 44 +++++++++++++++ ...onsImport_filteredByPackageJson_ambient.ts | 30 +++++++++++ ...ionsImport_filteredByPackageJson_direct.ts | 46 ++++++++++++++++ .../importNameCodeFixNewImportNodeModules8.ts | 2 +- 6 files changed, 157 insertions(+), 23 deletions(-) rename tests/cases/fourslash/{completionsImport_filteredByPackageJson.ts => completionsImport_filteredByPackageJson_@typesImplicit.ts} (93%) create mode 100644 tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesOnly.ts create mode 100644 tests/cases/fourslash/completionsImport_filteredByPackageJson_ambient.ts create mode 100644 tests/cases/fourslash/completionsImport_filteredByPackageJson_direct.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9d34ae78e1c..87b61efff8b 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -571,37 +571,33 @@ namespace ts.codefix { const packageJsons = findPackageJsons(getDirectoryPath(from.fileName), host); const dependencyIterator = readPackageJsonDependencies(); forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => { - if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName) && packageJsonAllowsImporting(sourceFile.fileName)) { + if (sourceFile === undefined) { + const maybeSourceFile = module.valueDeclaration.getSourceFile(); + if (!maybeSourceFile || packageJsonAllowsImporting(maybeSourceFile.fileName)) { + cb(module); + } + } + else if (sourceFile && sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName) && packageJsonAllowsImporting(sourceFile.fileName)) { cb(module); } }); let seenDeps: Map | undefined; - function packageJsonAllowsImporting(fileName: string): Ternary { - const pathComponents = getPathComponents(getDirectoryPath(fileName)); - const nodeModulesIndex = pathComponents.lastIndexOf("node_modules"); - if (nodeModulesIndex === -1) { - return Ternary.Maybe; - } - const moduleSpecifier = pathComponents[nodeModulesIndex + 1] === "@types" - ? pathComponents[nodeModulesIndex + 2] - : pathComponents[nodeModulesIndex + 1]; - if (!moduleSpecifier) { - return Ternary.Maybe; - } - - if (seenDeps && seenDeps.has(moduleSpecifier)) { - return Ternary.True; + function packageJsonAllowsImporting(fileName: string): boolean { + const moduleSpecifier = getModuleSpecifierFromFileName(fileName); + if (!moduleSpecifier || !packageJsons.length || seenDeps && seenDeps.has(moduleSpecifier)) { + return true; } let packageName: string; while (packageName = dependencyIterator.next().value) { - (seenDeps || (seenDeps = createMap())).set(packageName, true); - if (packageName === moduleSpecifier) { - return Ternary.True; + const nameWithoutTypes = getPackageNameFromTypesPackageName(packageName); + (seenDeps || (seenDeps = createMap())).set(nameWithoutTypes, true); + if (nameWithoutTypes === moduleSpecifier) { + return true; } } - return Ternary.False; + return false; } type PackageJson = Record | undefined>; @@ -620,6 +616,24 @@ namespace ts.codefix { } } } + + function getModuleSpecifierFromFileName(fileName: string): string | undefined { + const pathComponents = getPathComponents(getDirectoryPath(fileName)); + const nodeModulesIndex = pathComponents.lastIndexOf("node_modules"); + if (nodeModulesIndex === -1) { + return undefined; + } + const firstComponent = pathComponents[nodeModulesIndex + 1]; + const moduleSpecifier = firstComponent && + firstComponent === "@types" ? pathComponents[nodeModulesIndex + 2] : + startsWith(firstComponent, "@") ? `${firstComponent}/${pathComponents[nodeModulesIndex + 2]}` : + firstComponent; + + if (!moduleSpecifier) { + return undefined; + } + return moduleSpecifier; + } } function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) { diff --git a/tests/cases/fourslash/completionsImport_filteredByPackageJson.ts b/tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesImplicit.ts similarity index 93% rename from tests/cases/fourslash/completionsImport_filteredByPackageJson.ts rename to tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesImplicit.ts index 1f6b1ac60eb..539a9cc8ff6 100644 --- a/tests/cases/fourslash/completionsImport_filteredByPackageJson.ts +++ b/tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesImplicit.ts @@ -14,7 +14,7 @@ //@Filename: /node_modules/@types/react/package.json ////{ -//// "name": "react" +//// "name": "@types/react" ////} //@Filename: /node_modules/@types/fake-react/index.d.ts @@ -22,7 +22,7 @@ //@Filename: /node_modules/@types/fake-react/package.json ////{ -//// "name": "fake-react" +//// "name": "@types/fake-react" ////} //@Filename: /src/index.ts diff --git a/tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesOnly.ts b/tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesOnly.ts new file mode 100644 index 00000000000..b0d2c01e3db --- /dev/null +++ b/tests/cases/fourslash/completionsImport_filteredByPackageJson_@typesOnly.ts @@ -0,0 +1,44 @@ +/// + +//@noEmit: true + +//@Filename: /package.json +////{ +//// "devDependencies": { +//// "@types/react": "*" +//// } +////} + +//@Filename: /node_modules/@types/react/index.d.ts +////export declare var React: any; + +//@Filename: /node_modules/@types/react/package.json +////{ +//// "name": "@types/react" +////} + +//@Filename: /node_modules/@types/fake-react/index.d.ts +////export declare var ReactFake: any; + +//@Filename: /node_modules/@types/fake-react/package.json +////{ +//// "name": "@types/fake-react" +////} + +//@Filename: /src/index.ts +////const x = Re/**/ + +verify.completions({ + marker: test.marker(""), + isNewIdentifierLocation: true, + includes: { + name: "React", + hasAction: true, + source: "/node_modules/@types/react/index", + sortText: completion.SortText.AutoImportSuggestions + }, + excludes: "ReactFake", + preferences: { + includeCompletionsForModuleExports: true + } +}); diff --git a/tests/cases/fourslash/completionsImport_filteredByPackageJson_ambient.ts b/tests/cases/fourslash/completionsImport_filteredByPackageJson_ambient.ts new file mode 100644 index 00000000000..3dcb9eb6690 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_filteredByPackageJson_ambient.ts @@ -0,0 +1,30 @@ +/// + +//@noEmit: true + +//@Filename: /package.json +////{ +//// "dependencies": { +//// } +////} + +//@Filename: /node_modules/@types/node/timers.d.ts +////declare module "timers" { +//// function setTimeout(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timeout; +////} + +//@Filename: /node_modules/@types/node/package.json +////{ +//// "name": "@types/node", +////} + +//@Filename: /src/index.ts +////setTimeo/**/ + +verify.completions({ + marker: test.marker(""), + exact: completion.globals, + preferences: { + includeCompletionsForModuleExports: true + } +}); diff --git a/tests/cases/fourslash/completionsImport_filteredByPackageJson_direct.ts b/tests/cases/fourslash/completionsImport_filteredByPackageJson_direct.ts new file mode 100644 index 00000000000..aa7845daed3 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_filteredByPackageJson_direct.ts @@ -0,0 +1,46 @@ +/// + +//@noEmit: true + +//@Filename: /package.json +////{ +//// "dependencies": { +//// "react": "*" +//// } +////} + +//@Filename: /node_modules/react/index.d.ts +////export declare var React: any; + +//@Filename: /node_modules/react/package.json +////{ +//// "name": "react", +//// "types": "./index.d.ts" +////} + +//@Filename: /node_modules/fake-react/index.d.ts +////export declare var ReactFake: any; + +//@Filename: /node_modules/fake-react/package.json +////{ +//// "name": "fake-react", +//// "types": "./index.d.ts" +////} + +//@Filename: /src/index.ts +////const x = Re/**/ + +verify.completions({ + marker: test.marker(""), + isNewIdentifierLocation: true, + includes: { + name: "React", + hasAction: true, + source: "/node_modules/react/index", + sortText: completion.SortText.AutoImportSuggestions + }, + excludes: "ReactFake", + preferences: { + includeCompletionsForModuleExports: true + } +}); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules8.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules8.ts index f048f0d30d2..acfddd587f7 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules8.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules8.ts @@ -3,7 +3,7 @@ //// [|f1/*0*/('');|] // @Filename: package.json -//// { "dependencies": { "package-name": "latest" } } +//// { "dependencies": { "@scope/package-name": "latest" } } // @Filename: node_modules/@scope/package-name/bin/lib/index.d.ts //// export function f1(text: string): string;