diff --git a/src/cli.ts b/src/cli.ts index 334a4967..a1abe429 100644 --- a/src/cli.ts +++ b/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), 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() diff --git a/src/utils.ts b/src/utils.ts index 700dce8c..f661af31 100644 --- a/src/utils.ts +++ b/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}"`, + ) } } diff --git a/test/cli.test.ts b/test/cli.test.ts index 62805a95..d220ebee 100644 --- a/test/cli.test.ts +++ b/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',