Use empty object for invalid package json contents instead of undefin…

…ed (#34906) * Use empty object for invalid package json contents instead of undefined Fixes #34726 * Behave as if package json doesnt exist in case of invalid json in package json
microsoft · Nov 4, 2019 · 47ec514 · 47ec514
1 parent 9a3ec5f
commit 47ec514
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 9 deletions.
diff --git 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<PackageJsonInfo>();
+        const packageJsons = createMap<PackageJsonInfo | false>();
         const directoriesWithoutPackageJson = createMap<true>();
         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
@@ -1474,7 +1474,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
@@ -2210,19 +2210,18 @@ 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;
         }
 
         type PackageJsonRaw = Record<typeof dependencyKeys[number], Record<string, string> | 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<PackageJsonInfo, typeof dependencyKeys[number]> = {};
         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
@@ -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
@@ -0,0 +1,53 @@
+/// <reference path="fourslash.ts" />
+
+//@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
@@ -0,0 +1,37 @@
+/// <reference path="../fourslash.ts" />
+
+// @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,
+    },
+});