Skip to content

Commit

Permalink
fix: add exit codes to different flag validation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@WillieRuemmele
WillieRuemmele committed Nov 9, 2023
1 parent 672abf0 commit 6cc03e7
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/interfaces/parser.ts
View file
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
27 changes: 14 additions & 13 deletions src/parser/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ export type Validation = {
export class CLIParseError extends CLIError {
public parse: CLIParseErrorOptions['parse']

constructor(options: CLIParseErrorOptions & {message: string}) {
constructor(options: CLIParseErrorOptions & {exit?: number; 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
View file
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
View file
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

0 comments on commit 6cc03e7

Please sign in to comment.