Gracefully handle incomplete package.json files (#239)

olafurpg · web-flow · commit dfa4329a0b04 · 2023-03-16T11:54:28.000+01:00
* Gracefully handle incomplete package.json files

Previously, scip-typescript didn't emit the required
`scip-typescript npm PACKAGE_NAME PACKAGE_VERSION` prefix for global
symbols when a package.json file was missing either the `name` or
`version` field. Now, scip-typescript guarantees that all global symbols
have this prefix. When the `version` field is missing, we fallback to
the version `"HEAD"`.

* Empty commit to trigger ci

* ESlint
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,4 @@ node_modules
 .eslintcache
 yarn-error.log
 snapshots/output/**/*.scip
+tsconfig.tsbuildinfo
diff --git a/snapshots/input/invalid-package-json/package.json b/snapshots/input/invalid-package-json/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "invalid-package-json",
+  "version": "1.0.0",
+  "description": "Example TS/JS project",
+  "main": "src/main.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "private": true,
+  "packageManager": "pnpm@7.20.0"
+}
diff --git a/snapshots/input/invalid-package-json/packages/a/package.json b/snapshots/input/invalid-package-json/packages/a/package.json
@@ -0,0 +1,10 @@
+{
+  "name": "@example/a",
+  "description": "Example TS/JS project",
+  "main": "src/a.ts",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC"
+}
diff --git a/snapshots/input/invalid-package-json/packages/a/src/a.ts b/snapshots/input/invalid-package-json/packages/a/src/a.ts
@@ -0,0 +1,3 @@
+export function a(): string {
+  return ''
+}
diff --git a/snapshots/input/invalid-package-json/packages/a/tsconfig.json b/snapshots/input/invalid-package-json/packages/a/tsconfig.json
@@ -0,0 +1,9 @@
+{
+  "extends": "../../tsconfig.json",
+  "compilerOptions": {
+    "rootDir": ".",
+    "baseUrl": ".",
+    "outDir": "dist"
+  },
+  "include": ["src/*"]
+}
diff --git a/snapshots/input/invalid-package-json/packages/b/package.json b/snapshots/input/invalid-package-json/packages/b/package.json
@@ -0,0 +1,12 @@
+{
+  "description": "Example TS/JS project",
+  "main": "src/b.ts",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "@example/a": "1.0.0"
+  }
+}
diff --git a/snapshots/input/invalid-package-json/packages/b/src/b.ts b/snapshots/input/invalid-package-json/packages/b/src/b.ts
@@ -0,0 +1,5 @@
+import { a } from '@example/a'
+
+export function b() {
+  return a()
+}
diff --git a/snapshots/input/invalid-package-json/packages/b/tsconfig.json b/snapshots/input/invalid-package-json/packages/b/tsconfig.json
@@ -0,0 +1,11 @@
+{
+  "extends": "../../tsconfig.json",
+  "compilerOptions": {
+    "rootDir": ".",
+    "baseUrl": "./src",
+    "sourceRoot": "src",
+    "outDir": "dist"
+  },
+  "include": ["src/*"],
+  "references": [{ "path": "../a" }]
+}
diff --git a/snapshots/input/invalid-package-json/pnpm-lock.yaml b/snapshots/input/invalid-package-json/pnpm-lock.yaml
diff --git a/snapshots/input/invalid-package-json/pnpm-workspace.yaml b/snapshots/input/invalid-package-json/pnpm-workspace.yaml
@@ -0,0 +1,2 @@
+packages:
+  - 'packages/*'
diff --git a/snapshots/input/invalid-package-json/tsconfig.json b/snapshots/input/invalid-package-json/tsconfig.json
@@ -0,0 +1,24 @@
+{
+  "extends": "@sourcegraph/tsconfig",
+  "compilerOptions": {
+    "target": "es2020",
+    "module": "commonjs",
+    "allowJs": false,
+    "moduleResolution": "node",
+    "esModuleInterop": true,
+    "lib": ["esnext", "dom", "dom.iterable"],
+    "sourceMap": true,
+    "declaration": true,
+    "declarationMap": true,
+    "skipLibCheck": true,
+    "skipDefaultLibCheck": true,
+    "noErrorTruncation": true,
+    "importHelpers": true,
+    "resolveJsonModule": true,
+    "composite": true,
+    "outDir": "out",
+    "rootDir": "."
+  },
+  "include": [],
+  "exclude": ["out", "node_modules", "dist"]
+}
diff --git a/snapshots/output/invalid-package-json/packages/a/src/a.ts b/snapshots/output/invalid-package-json/packages/a/src/a.ts
@@ -0,0 +1,8 @@
+  export function a(): string {
+// definition @example/a HEAD src/`a.ts`/
+//documentation ```ts\nmodule "a.ts"\n```
+//                ^ definition @example/a HEAD src/`a.ts`/a().
+//                documentation ```ts\nfunction a(): string\n```
+    return ''
+  }
+  
diff --git a/src/FileIndexer.ts b/src/FileIndexer.ts
@@ -319,8 +319,8 @@ export class FileIndexer {
     }
     if (ts.isSourceFile(node)) {
       const package_ = this.packages.symbol(node.fileName)
-      if (!package_) {
-        return this.cached(node, ScipSymbol.empty())
+      if (package_.isEmpty()) {
+        return this.cached(node, ScipSymbol.anonymousPackage())
       }
       return this.cached(node, package_)
     }
diff --git a/src/Packages.ts b/src/Packages.ts
@@ -33,28 +33,35 @@ export class Packages {
         if (typeof name === 'string' && typeof version === 'string') {
           return this.cached(filePath, ScipSymbol.package(name, version))
         }
+        if (typeof name === 'string') {
+          // The version field is missing so we fallback to `"HEAD"`
+          return this.cached(filePath, ScipSymbol.package(name, 'HEAD'))
+        }
+        // Fallback to an anonymous package because we found a package.json but
+        // were unable to parse the name and version.
+        return this.cached(filePath, ScipSymbol.anonymousPackage())
       }
     } catch (error) {
       console.error(`error parsing ${packageJsonPath}`, error)
-      return this.cached(filePath, ScipSymbol.empty())
+      return this.cached(filePath, ScipSymbol.anonymousPackage())
     }
 
     if (filePath === this.projectRoot) {
-      return this.cached(filePath, ScipSymbol.empty())
+      // Don't look for package.json in a parent directory of the root.
+      return this.cached(filePath, ScipSymbol.anonymousPackage())
     }
 
     const dirname = path.dirname(filePath)
     if (dirname === filePath) {
-      return this.cached(filePath, ScipSymbol.empty())
+      // Avoid infinite recursion when `path.dirname(path) === path`
+      return this.cached(filePath, ScipSymbol.anonymousPackage())
     }
+
     const owner = this.symbol(dirname)
-    if (owner) {
-      return this.cached(
-        filePath,
-        ScipSymbol.global(owner, packageDescriptor(path.basename(filePath)))
-      )
-    }
-    return this.cached(filePath, ScipSymbol.empty())
+    return this.cached(
+      filePath,
+      ScipSymbol.global(owner, packageDescriptor(path.basename(filePath)))
+    )
   }
 
   private cached(filePath: string, sym: ScipSymbol): ScipSymbol {
diff --git a/src/ScipSymbol.ts b/src/ScipSymbol.ts
@@ -24,6 +24,10 @@ export class ScipSymbol {
     return new ScipSymbol(`scip-typescript npm ${name} ${version} `)
   }
 
+  public static anonymousPackage(): ScipSymbol {
+    return ScipSymbol.package('.', '.')
+  }
+
   public static global(
     owner: ScipSymbol,
     descriptor: scip.scip.Descriptor

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export function a(): string {`
	`2`	`+ return ''`
	`3`	`+}`