From 4d5f30dee038f02408c190c7dec890ccadb44ece Mon Sep 17 00:00:00 2001 From: TypeScript Bot Date: Fri, 8 Nov 2019 14:07:46 -0800 Subject: [PATCH] Cherry-pick PR #34906 into release-3.7 (#35006) Component commits: dfa4bc0d75 Use empty object for invalid package json contents instead of undefined Fixes #34726 4d035ba5f8 Behave as if package json doesnt exist in case of invalid json in package json --- src/server/packageJsonCache.ts | 6 +-- src/server/project.ts | 3 +- src/services/utilities.ts | 9 ++-- .../unittests/tsserver/packageJsonInfo.ts | 17 ++++++ ...ort_filteredByInvalidPackageJson_direct.ts | 53 +++++++++++++++++++ ...portSuggestionsCache_invalidPackageJson.ts | 37 +++++++++++++ 6 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_filteredByInvalidPackageJson_direct.ts create mode 100644 tests/cases/fourslash/server/importSuggestionsCache_invalidPackageJson.ts diff --git a/src/server/packageJsonCache.ts b/src/server/packageJsonCache.ts index d47432341fbe3..290711a821721 100644 --- a/src/server/packageJsonCache.ts +++ b/src/server/packageJsonCache.ts @@ -9,7 +9,7 @@ namespace ts.server { } export function createPackageJsonCache(project: Project): PackageJsonCache { - const packageJsons = createMap(); + const packageJsons = createMap(); const directoriesWithoutPackageJson = createMap(); return { addOrUpdate, @@ -18,7 +18,7 @@ namespace ts.server { directoriesWithoutPackageJson.set(getDirectoryPath(fileName), true); }, getInDirectory: directory => { - return packageJsons.get(combinePaths(directory, "package.json")); + return packageJsons.get(combinePaths(directory, "package.json")) || undefined; }, directoryHasPackageJson, searchDirectoryAndAncestors: directory => { @@ -39,7 +39,7 @@ namespace ts.server { function addOrUpdate(fileName: Path) { const packageJsonInfo = createPackageJsonInfo(fileName, project); - if (packageJsonInfo) { + if (packageJsonInfo !== undefined) { packageJsons.set(fileName, packageJsonInfo); directoriesWithoutPackageJson.delete(getDirectoryPath(fileName)); } diff --git a/src/server/project.ts b/src/server/project.ts index e3a9aedc30371..b0d2c427aa7d9 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1471,7 +1471,8 @@ namespace ts.server { case Ternary.True: const packageJsonFileName = combinePaths(directory, "package.json"); watchPackageJsonFile(packageJsonFileName); - result.push(Debug.assertDefined(packageJsonCache.getInDirectory(directory))); + const info = packageJsonCache.getInDirectory(directory); + if (info) result.push(info); } if (rootPath && rootPath === toPath(directory)) { return true; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index d0e75c5522fc9..55dc95d3fc2a7 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2210,7 +2210,7 @@ namespace ts { return packageJsons; } - export function createPackageJsonInfo(fileName: string, host: LanguageServiceHost): PackageJsonInfo | undefined { + export function createPackageJsonInfo(fileName: string, host: LanguageServiceHost): PackageJsonInfo | false | undefined { if (!host.readFile) { return undefined; } @@ -2218,11 +2218,10 @@ namespace ts { type PackageJsonRaw = Record | undefined>; const dependencyKeys = ["dependencies", "devDependencies", "optionalDependencies", "peerDependencies"] as const; const stringContent = host.readFile(fileName); - const content = stringContent && tryParseJson(stringContent) as PackageJsonRaw; - if (!content) { - return undefined; - } + if (!stringContent) return undefined; + const content = tryParseJson(stringContent) as PackageJsonRaw; + if (!content) return false; const info: Pick = {}; for (const key of dependencyKeys) { const dependencies = content[key]; diff --git a/src/testRunner/unittests/tsserver/packageJsonInfo.ts b/src/testRunner/unittests/tsserver/packageJsonInfo.ts index aad479fccfbcc..20be7ce56e3ab 100644 --- a/src/testRunner/unittests/tsserver/packageJsonInfo.ts +++ b/src/testRunner/unittests/tsserver/packageJsonInfo.ts @@ -71,6 +71,23 @@ namespace ts.projectSystem { assert.lengthOf(project.getPackageJsonsVisibleToFile("/a.ts" as Path), 1); assert.lengthOf(project.getPackageJsonsVisibleToFile("/src/b.ts" as Path), 2); }); + + it("handles errors in json parsing of package.json", () => { + const packageJsonContent = `{ "mod" }`; + const { project, host } = setup([tsConfig, { path: packageJson.path, content: packageJsonContent }]); + project.getPackageJsonsVisibleToFile("/src/whatever/blah.ts" as Path); + const packageJsonInfo = project.packageJsonCache.getInDirectory("/" as Path)!; + assert.isUndefined(packageJsonInfo); + + host.writeFile(packageJson.path, packageJson.content); + project.getPackageJsonsVisibleToFile("/src/whatever/blah.ts" as Path); + const packageJsonInfo2 = project.packageJsonCache.getInDirectory("/" as Path)!; + assert.ok(packageJsonInfo2); + assert.ok(packageJsonInfo2.dependencies); + assert.ok(packageJsonInfo2.devDependencies); + assert.ok(packageJsonInfo2.peerDependencies); + assert.ok(packageJsonInfo2.optionalDependencies); + }); }); function setup(files: readonly File[] = [tsConfig, packageJson]) { diff --git a/tests/cases/fourslash/completionsImport_filteredByInvalidPackageJson_direct.ts b/tests/cases/fourslash/completionsImport_filteredByInvalidPackageJson_direct.ts new file mode 100644 index 0000000000000..5bd121ed37f7a --- /dev/null +++ b/tests/cases/fourslash/completionsImport_filteredByInvalidPackageJson_direct.ts @@ -0,0 +1,53 @@ +/// + +//@noEmit: true + +//@Filename: /package.json +////{ +//// "mod" +//// "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 + }, + { + name: "ReactFake", + hasAction: true, + source: "/node_modules/fake-react/index", + sortText: completion.SortText.AutoImportSuggestions + } + ], + preferences: { + includeCompletionsForModuleExports: true + } +}); diff --git a/tests/cases/fourslash/server/importSuggestionsCache_invalidPackageJson.ts b/tests/cases/fourslash/server/importSuggestionsCache_invalidPackageJson.ts new file mode 100644 index 0000000000000..ad1de44aea3af --- /dev/null +++ b/tests/cases/fourslash/server/importSuggestionsCache_invalidPackageJson.ts @@ -0,0 +1,37 @@ +/// + +// @Filename: /jsconfig.json +////{ +//// "compilerOptions": { +//// "module": "commonjs", +//// }, +////} + +// @Filename: /node_modules/@types/node/index.d.ts +////declare module 'fs' { +//// export function readFile(): void; +////} +////declare module 'util' { +//// export function promisify(): void; +////} + +// @Filename: /package.json +////{ "mod" } + +// @Filename: /a.js +//// +////readF/**/ + +goTo.marker(""); +verify.completions({ + includes: { + name: "readFile", + source: "fs", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }, + preferences: { + includeCompletionsForModuleExports: true, + includeInsertTextCompletions: true, + }, +}); \ No newline at end of file