Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add exit codes to different flag validation errors #861

Merged
merged 11 commits into from
Dec 4, 2023
1 change: 1 addition & 0 deletions src/interfaces/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type CLIParseErrorOptions = {
input?: ParserInput
output?: ParserOutput
}
exit?: number
}

export type OutputArgs<T extends ParserInput['args']> = {[P in keyof T]: any}
Expand Down
25 changes: 13 additions & 12 deletions src/parser/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ export class CLIParseError extends CLIError {

constructor(options: CLIParseErrorOptions & {message: string}) {
options.message += '\nSee more help with --help'
super(options.message)
super(options.message, {exit: options.exit})
this.parse = options.parse
}
}

export class InvalidArgsSpecError extends CLIParseError {
public args: ArgInput

constructor({args, parse}: CLIParseErrorOptions & {args: ArgInput}) {
constructor({args, exit, parse}: CLIParseErrorOptions & {args: ArgInput}) {
let message = 'Invalid argument spec'
const namedArgs = Object.values(args).filter((a) => a.name)
if (namedArgs.length > 0) {
Expand All @@ -41,7 +41,7 @@ export class InvalidArgsSpecError extends CLIParseError {
message += `:\n${list}`
}

super({message, parse})
super({exit, message, parse})
this.args = args
}
}
Expand All @@ -51,6 +51,7 @@ export class RequiredArgsError extends CLIParseError {

constructor({
args,
exit,
flagsWithMultiple,
parse,
}: CLIParseErrorOptions & {args: Arg<any>[]; flagsWithMultiple?: string[]}) {
Expand All @@ -71,38 +72,38 @@ export class RequiredArgsError extends CLIParseError {
message += '\nAlternatively, you can use "--" to signify the end of the flags and the beginning of arguments.'
}

super({message, parse})
super({exit, message, parse})
this.args = args
}
}

export class RequiredFlagError extends CLIParseError {
public flag: Flag<any>

constructor({flag, parse}: CLIParseErrorOptions & {flag: Flag<any>}) {
constructor({exit, flag, parse}: CLIParseErrorOptions & {flag: Flag<any>}) {
const usage = renderList(flagUsages([flag], {displayRequired: false}))
const message = `Missing required flag:\n${usage}`
super({message, parse})
super({exit, message, parse})
this.flag = flag
}
}

export class UnexpectedArgsError extends CLIParseError {
public args: unknown[]

constructor({args, parse}: CLIParseErrorOptions & {args: unknown[]}) {
constructor({args, exit, parse}: CLIParseErrorOptions & {args: unknown[]}) {
const message = `Unexpected argument${args.length === 1 ? '' : 's'}: ${args.join(', ')}`
super({message, parse})
super({exit, message, parse})
this.args = args
}
}

export class NonExistentFlagsError extends CLIParseError {
public flags: string[]

constructor({flags, parse}: CLIParseErrorOptions & {flags: string[]}) {
constructor({exit, flags, parse}: CLIParseErrorOptions & {flags: string[]}) {
const message = `Nonexistent flag${flags.length === 1 ? '' : 's'}: ${flags.join(', ')}`
super({message, parse})
super({exit, message, parse})
this.flags = flags
}
}
Expand All @@ -122,11 +123,11 @@ export class ArgInvalidOptionError extends CLIParseError {
}

export class FailedFlagValidationError extends CLIParseError {
constructor({failed, parse}: CLIParseErrorOptions & {failed: Validation[]}) {
constructor({exit, failed, parse}: CLIParseErrorOptions & {failed: Validation[]}) {
const reasons = failed.map((r) => r.reason)
const deduped = uniq(reasons)
const errString = deduped.length === 1 ? 'error' : 'errors'
const message = `The following ${errString} occurred:\n ${chalk.dim(deduped.join('\n '))}`
super({message, parse})
super({exit, message, parse})
}
}
13 changes: 8 additions & 5 deletions src/parser/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}

function validateArgs() {
if (parse.output.nonExistentFlags?.length > 0) {
throw new NonExistentFlagsError({flags: parse.output.nonExistentFlags, parse})
// this is the first error that could be thrown, exit codes 1 and 2 have been used, so we'll start with 3
throw new NonExistentFlagsError({exit: 3, flags: parse.output.nonExistentFlags, parse})
}

const maxArgs = Object.keys(parse.input.args).length
if (parse.input.strict && parse.output.argv.length > maxArgs) {
const extras = parse.output.argv.slice(maxArgs)
throw new UnexpectedArgsError({args: extras, parse})
throw new UnexpectedArgsError({args: extras, exit: 4, parse})
}

const missingRequiredArgs: Arg<any>[] = []
Expand All @@ -32,7 +33,7 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
} else if (hasOptional) {
// (required arg) check whether an optional has occurred before
// optionals should follow required, not before
throw new InvalidArgsSpecError({args: parse.input.args, parse})
throw new InvalidArgsSpecError({args: parse.input.args, exit: 5, parse})
}

if (arg.required && !parse.output.args[name] && parse.output.args[name] !== 0) {
Expand All @@ -45,7 +46,8 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
.filter(([_, flagDef]) => flagDef.type === 'option' && Boolean(flagDef.multiple))
.map(([name]) => name)

throw new RequiredArgsError({args: missingRequiredArgs, flagsWithMultiple, parse})
// if a command is missing args -> exit 6
throw new RequiredArgsError({args: missingRequiredArgs, exit: 6, flagsWithMultiple, parse})
}
}

Expand Down Expand Up @@ -76,7 +78,8 @@ export async function validate(parse: {input: ParserInput; output: ParserOutput}
const results = await Promise.all(promises)

const failed = results.filter((r) => r.status === 'failed')
if (failed.length > 0) throw new FailedFlagValidationError({failed, parse})
// if a command is missing flags -> exit 7
if (failed.length > 0) throw new FailedFlagValidationError({exit: 7, failed, parse})
}

async function resolveFlags(flags: FlagRelationship[]): Promise<Record<string, unknown>> {
Expand Down
144 changes: 144 additions & 0 deletions test/parser/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,85 @@ describe('validate', () => {
'--': true,
}

it('will exit 3 when a nonExistentFlags flag is passed', async () => {
const output = {
args: {},
argv: [],
nonExistentFlags: ['foobar'],
}

try {
// @ts-expect-error
await validate({input, output})
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(3)
expect(err.message).to.include('Nonexistent flag: foobar')
}
})

it('will exit 4 when an unexpected argument is found', async () => {
const output = {
args: {},
argv: ['found', 'me'],
nonExistentFlags: [],
}

try {
// @ts-expect-error
await validate({input, output})
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(4)
expect(err.message).to.include('Unexpected arguments: found, me')
}
})

it('throws when required flag is mixed with args -> exit 5', async () => {
const input = {
argv: [],
flags: {
foo: {
description: 'foo flag',
required: true,
},
},
raw: [
{
type: 'flag',
flag: 'foo',
input: 'value',
},
],
args: {foo: {required: false}, bar: {required: true}},
strict: true,
context: {},
'--': true,
}

const output = {
args: {},
argv: [],
flags: {foobar: 'value'},
raw: [],
metadata: {
flags: {},
},
}

try {
// @ts-expect-error
await validate({input, output})
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.message).to.include('Invalid argument spec')
expect(err.oclif.exit).to.equal(5)
}
})

it('enforces exclusivity for flags', async () => {
const output = {
args: {},
Expand Down Expand Up @@ -58,6 +137,7 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)
expect(err.message).to.include('--dessert=cheesecake cannot also be provided when using --dinner')
}
})
Expand Down Expand Up @@ -153,6 +233,50 @@ describe('validate', () => {
} catch (error) {
const err = error as CLIError
expect(err.message).to.include('Missing required flag')
expect(err.oclif.exit).to.equal(7)
}
})

it('throws when required flag is missing value', async () => {
const input = {
argv: [],
flags: {
foobar: {
description: 'foobar flag',
required: true,
},
},
raw: [
{
type: 'flag',
flag: 'foobar',
input: 'value',
},
],
args: {foobar: {required: true}},
strict: true,
context: {},
'--': true,
}

const output = {
args: {},
argv: [],
flags: {foobar: 'value'},
raw: [],
metadata: {
flags: {},
},
}

try {
// @ts-expect-error
await validate({input, output})
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.message).to.include('Missing 1 required arg')
expect(err.oclif.exit).to.equal(6)
}
})

Expand Down Expand Up @@ -272,6 +396,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include(
'All of the following must be provided when using --dessert: --cookies, --sprinkles',
)
Expand Down Expand Up @@ -322,6 +448,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include(
'All of the following must be provided when using --dessert: --cookies, --sprinkles',
)
Expand Down Expand Up @@ -372,6 +500,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include('All of the following must be provided when using --dessert: --cookies')
}
})
Expand Down Expand Up @@ -453,6 +583,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include(
'One of the following must be provided when using --dessert: --cookies, --sprinkles',
)
Expand Down Expand Up @@ -503,6 +635,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include(
'One of the following must be provided when using --dessert: --cookies, --sprinkles',
)
Expand Down Expand Up @@ -553,6 +687,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include('One of the following must be provided when using --dessert: --cookies')
}
})
Expand Down Expand Up @@ -639,6 +775,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include('--sprinkles=true cannot also be provided when using --dessert')
}
})
Expand Down Expand Up @@ -696,6 +834,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include('--sprinkles=true cannot also be provided when using --dessert')
}
})
Expand Down Expand Up @@ -827,6 +967,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include('--cookies=false cannot also be provided when using --dessert')
}
})
Expand Down Expand Up @@ -925,6 +1067,8 @@ describe('validate', () => {
fail('should have thrown')
} catch (error) {
const err = error as CLIError
expect(err.oclif.exit).to.equal(7)

expect(err.message).to.include(
'All of the following must be provided when using --dessert: --cookies, --cake',
)
Expand Down
Loading