CLI: one error: line for a flag with no path, an invalid schema, or a $ref that does not resolve

src/cli.ts

@@ -6,7 +6,8 @@ import {omit} from 'lodash'
 import {glob, isDynamicPattern} from 'tinyglobby'
 import {join, resolve, dirname} from 'path'
 import {resolveConfig} from 'prettier'
-import {compile, compileFiles, DEFAULT_OPTIONS, Options} from './index'
+import {JSONParserError, JSONParserErrorGroup} from '@apidevtools/json-schema-ref-parser'
+import {compile, compileFiles, DEFAULT_OPTIONS, Options, ValidationError} from './index'
 import {pathTransform, error, parseFileAsJSONSchema, justName, stripExtension} from './utils'
 
 // cwd and style are deliberately left out of the CLI defaults: processFile()
@@ -63,10 +64,18 @@ async function main(argv: minimist.ParsedArgs) {
   const argIn: string = argv._[0] || argv.input
   const argOut: string | undefined = argv._[1] || argv.output // the output can be omitted so this can be undefined
 
-  const ISGLOB = argIn && isDynamicPattern(argIn)
-  const ISDIR = !!argIn && isDir(argIn)
-
   try {
+    // A path flag with nothing after it (`json2ts schema.json -o`) parses as boolean `true`; `--output=` as ''
+    for (const flag of ['input', 'output'] as const) {
+      if (argv[flag] === true || argv[flag] === '') {
+        throw new UsageError(
+          `--${flag} (-${flag[0]}) needs a path after it, e.g. -${flag[0]} ${flag === 'input' ? 'schema.json' : 'schema.d.ts'}`,
+        )
+      }
+    }
+    const ISGLOB = argIn && isDynamicPattern(argIn)
+    const ISDIR = !!argIn && isDir(argIn)
+
     // Defend against unquoted glob expansion (or other shell mistakes) silently supplying extra
     // positional arguments. A positional that competes with an explicitly-passed --input/--output
     // flag for the same slot, or overflows past the two positional slots (input, output) this CLI
@@ -113,11 +122,35 @@ async function main(argv: minimist.ParsedArgs) {
       outputResult(await processFile(argIn, argOut, argv as Partial<Options>), argOut)
     }
   } catch (e) {
-    error(e instanceof UsageError ? e.message : e)
+    // The user's mistake, not the program's: print what to fix, without the stack.
+    // - UsageError: how the CLI was called
+    // - ValidationError: compile() already printed one line per rule the schema breaks
+    // - json-schema-ref-parser's errors: a $ref it could not resolve (a missing or
+    //   unparsable file, a pointer to nothing), one line each
+    if (e instanceof UsageError) {
+      error(e.message)
+    } else if (e instanceof JSONParserError || e instanceof JSONParserErrorGroup) {
+      refErrorLines(e).forEach(line => error(line))
+    } else if (!(e instanceof ValidationError)) {
+      error(e)
+    }
     process.exit(1)
   }
 }
 
+/**
+ * The message of each `$ref` error, naming the file it was found in when the message
+ * itself does not (a missing pointer says which pointer, not which file it is in). For
+ * the schema being compiled itself, `source` is its directory (a trailing slash), which
+ * says nothing: the user knows which schema they passed.
+ */
+function refErrorLines(e: JSONParserError | JSONParserErrorGroup): string[] {
+  const errors = e instanceof JSONParserErrorGroup ? e.errors : [e]
+  return errors.map(_ =>
+    _.source && !_.source.endsWith('/') && !_.message.includes(_.source) ? `${_.message} (in ${_.source})` : _.message,
+  )
+}
+
 // check if path is an existing directory
 function isDir(path: string): boolean {
   return existsSync(path) && lstatSync(path).isDirectory()

src/utils.ts

@@ -430,7 +430,9 @@ export function parseFileAsJSONSchema(filename: string | null, contents: string)
   try {
     return JSON.parse(contents)
   } catch {
-    throw new TypeError(`Error parsing JSON in file "${filename}"`)
+    throw new TypeError(
+      filename == null ? 'Error parsing JSON from standard input' : `Error parsing JSON in file "${filename}"`,
+    )
   }
 }
 

test/cli.test.ts

@@ -62,8 +62,9 @@ function cliFailTest(
   name: string,
   command: string,
   check: (output: {code?: number | string; stdout: string; stderr: string}) => void,
+  input?: string,
 ) {
-  spawners.push(() => results.set(name, run(command)))
+  spawners.push(() => results.set(name, run(command, input)))
   test(name, async () => {
     const {error, stdout, stderr} = await results.get(name)!
     expect(error).not.toBeNull()
@@ -423,6 +424,106 @@ suite('CLI', () => {
     },
   )
 
+  // A flag that should carry a path but has nothing after it (`-o` at the end of the line)
+  // used to reach path.resolve() as `true` and die with a TypeError and a stack trace
+  cliFailTest(
+    '-o with no path after it is a one-line usage error naming the flag',
+    'node dist/src/cli.js ./test/resources/ReferencedType.json -o',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toStartWith('error: --output (-o) needs a path after it')
+      expect(stderr.trimEnd().split('\n')).toHaveLength(1)
+    },
+  )
+
+  cliFailTest(
+    '--output= with an empty path is the same usage error',
+    'node dist/src/cli.js ./test/resources/ReferencedType.json --output=',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toStartWith('error: --output (-o) needs a path after it')
+    },
+  )
+
+  cliFailTest(
+    '-i with no path after it is a one-line usage error naming the flag',
+    'node dist/src/cli.js -i',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toStartWith('error: --input (-i) needs a path after it')
+      expect(stderr.trimEnd().split('\n')).toHaveLength(1)
+    },
+  )
+
+  // compile() prints one line per rule an invalid schema breaks; the CLI must not add a
+  // stack trace for the ValidationError it then throws (the lines say all there is to say)
+  cliFailTest(
+    'an invalid schema prints the rule it breaks and no stack trace',
+    'node dist/src/cli.js',
+    ({code, stdout, stderr}) => {
+      expect(code).toBe(1)
+      expect(stdout).toBe('')
+      expect(stderr).toStartWith('error: ')
+      expect(stderr).toContain('When minItems exists, minItems >= 0')
+      expect(stderr.trimEnd().split('\n')).toHaveLength(1)
+      expect(stderr).not.toMatch(/^\s+at /m)
+    },
+    '{"type": "array", "minItems": -1}',
+  )
+
+  cliFailTest(
+    'invalid JSON on stdin names standard input, not a file called "null"',
+    'node dist/src/cli.js',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toContain('Error parsing JSON from standard input')
+      expect(stderr).not.toContain('"null"')
+    },
+    '{"type": nope',
+  )
+
+  // A $ref to a file that does not exist: the resolver's message names the file, which is
+  // all the user needs; the error's stack and fields (code, source, toJSON…) are noise
+  cliFailTest(
+    'a $ref to a missing file is one error line naming the file',
+    'node dist/src/cli.js',
+    ({code, stdout, stderr}) => {
+      expect(code).toBe(1)
+      expect(stdout).toBe('')
+      expect(stderr).toStartWith('error: ')
+      expect(stderr).toContain('NoSuchRefTarget.json')
+      expect(stderr).not.toMatch(/^\s+at /m)
+      expect(stderr).not.toContain('ERESOLVER')
+    },
+    '{"properties": {"x": {"$ref": "./test/resources/NoSuchRefTarget.json"}}}',
+  )
+
+  // ...but a pointer into the schema itself is not suffixed with its directory
+  cliFailTest(
+    'a $ref to a missing pointer in the schema itself is the message alone',
+    'node dist/src/cli.js',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toBe('error: Missing $ref pointer "#/definitions/nope". Token "definitions" does not exist.\n')
+    },
+    '{"properties": {"x": {"$ref": "#/definitions/nope"}}}',
+  )
+
+  // ...and when the resolver's message does not name the file (a pointer into another
+  // file that does not exist there), the line says which file it is in
+  cliFailTest(
+    'a $ref to a missing pointer in another file names that file',
+    'node dist/src/cli.js',
+    ({code, stderr}) => {
+      expect(code).toBe(1)
+      expect(stderr).toStartWith('error: Missing $ref pointer "#/definitions/nope"')
+      // the resolver reports the path with forward slashes on every OS, so match the file name, not resolve()
+      expect(stderr).toMatch(/ \(in \S*test\/resources\/ReferencedType\.json\)\n$/)
+      expect(stderr.trimEnd().split('\n')).toHaveLength(1)
+    },
+    '{"properties": {"x": {"$ref": "./test/resources/ReferencedType.json#/definitions/nope"}}}',
+  )
+
   // ...while anything else that goes wrong still says where: the unreadable path here
   cliFailTest(
     'an unreadable input file reports the path it could not open',