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
4 changes: 3 additions & 1 deletion src/config/cache.ts → src/cache.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {Plugin} from '../interfaces'
import {PJSON, Plugin} from './interfaces'

type CacheContents = {
rootPlugin: Plugin
exitCodes: PJSON.Plugin['oclif']['exitCodes']
}

type ValueOf<T> = T[keyof T]
Expand All @@ -20,6 +21,7 @@ export default class Cache extends Map<keyof CacheContents, ValueOf<CacheContent
}

public get(key: 'rootPlugin'): Plugin | undefined
public get(key: 'exitCodes'): PJSON.Plugin['oclif']['exitCodes'] | undefined
public get(key: keyof CacheContents): ValueOf<CacheContents> | undefined {
return super.get(key)
}
Expand Down
6 changes: 4 additions & 2 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {arch, userInfo as osUserInfo, release, tmpdir, type} from 'node:os'
import {join, resolve, sep} from 'node:path'
import {URL, fileURLToPath} from 'node:url'

import Cache from '../cache'
import {ux} from '../cli-ux'
import {parseTheme} from '../cli-ux/theme'
import {Command} from '../command'
Expand All @@ -19,7 +20,6 @@ import {settings} from '../settings'
import {requireJson, safeReadJson} from '../util/fs'
import {getHomeDir, getPlatform} from '../util/os'
import {compact, isProd} from '../util/util'
import Cache from './cache'
import PluginLoader from './plugin-loader'
import {tsPath} from './ts-node'
import {Debug, collectUsableIds, getCommandIdPermutations} from './util'
Expand Down Expand Up @@ -290,7 +290,9 @@ export class Config implements IConfig {

// Cache the root plugin so that we can reference it later when determining if
// we should skip ts-node registration for an ESM plugin.
Cache.getInstance().set('rootPlugin', this.rootPlugin)
const cache = Cache.getInstance()
cache.set('rootPlugin', this.rootPlugin)
cache.set('exitCodes', this.rootPlugin.pjson.oclif.exitCodes ?? {})

this.root = this.rootPlugin.root
this.pjson = this.rootPlugin.pjson
Expand Down
2 changes: 1 addition & 1 deletion src/config/ts-node.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {join, relative as pathRelative, sep} from 'node:path'
import * as TSNode from 'ts-node'

import Cache from '../cache'
import {memoizedWarn} from '../errors'
import {Plugin, TSConfig} from '../interfaces'
import {settings} from '../settings'
import {existsSync, readTSConfig} from '../util/fs'
import {isProd} from '../util/util'
import Cache from './cache'
import {Debug} from './util'
// eslint-disable-next-line new-cap
const debug = Debug('ts-node')
Expand Down
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
8 changes: 8 additions & 0 deletions src/interfaces/pjson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ export namespace PJSON {
default?: string
description?: string
devPlugins?: string[]
exitCodes?: {
failedFlagParsing?: number
failedFlagValidation?: number
invalidArgsSpec?: number
nonExistentFlag?: number
requiredArgs?: number
unexpectedArgs?: number
}
flexibleTaxonomy?: boolean
helpClass?: string
helpOptions?: HelpOptions
Expand Down
47 changes: 23 additions & 24 deletions src/parser/errors.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import chalk from 'chalk'

import Cache from '../cache'
import {renderList} from '../cli-ux/list'
import {CLIError} from '../errors'
import {Flag, OptionFlag} from '../interfaces'
import {Arg, ArgInput, CLIParseErrorOptions} from '../interfaces/parser'
import {Arg, ArgInput, CLIParseErrorOptions, OptionFlag} from '../interfaces/parser'
import {uniq} from '../util/util'
import {flagUsages} from './help'

export {CLIError} from '../errors'

Expand All @@ -21,15 +20,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 +40,7 @@ export class InvalidArgsSpecError extends CLIParseError {
message += `:\n${list}`
}

super({message, parse})
super({exit: Cache.getInstance().get('exitCodes')?.invalidArgsSpec ?? exit, message, parse})
this.args = args
}
}
Expand All @@ -51,6 +50,7 @@ export class RequiredArgsError extends CLIParseError {

constructor({
args,
exit,
flagsWithMultiple,
parse,
}: CLIParseErrorOptions & {args: Arg<any>[]; flagsWithMultiple?: string[]}) {
Expand All @@ -71,38 +71,27 @@ 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: Cache.getInstance().get('exitCodes')?.requiredArgs ?? exit, message, parse})
this.args = args
}
}

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

constructor({flag, parse}: CLIParseErrorOptions & {flag: Flag<any>}) {
const usage = renderList(flagUsages([flag], {displayRequired: false}))
const message = `Missing required flag:\n${usage}`
super({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: Cache.getInstance().get('exitCodes')?.unexpectedArgs ?? 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: Cache.getInstance().get('exitCodes')?.nonExistentFlag ?? exit, message, parse})
this.flags = flags
}
}
Expand All @@ -122,11 +111,21 @@ 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: Cache.getInstance().get('exitCodes')?.failedFlagValidation ?? exit, message, parse})
}
}

export class FailedFlagParsingError extends CLIParseError {
constructor({flag, message}: {flag: string; message: string}) {
super({
exit: Cache.getInstance().get('exitCodes')?.failedFlagParsing,
message: `Parsing --${flag} \n\t${message}`,
parse: {},
})
}
}
8 changes: 5 additions & 3 deletions src/parser/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
ParsingToken,
} from '../interfaces/parser'
import {isTruthy, last, pickBy} from '../util/util'
import {ArgInvalidOptionError, CLIError, FlagInvalidOptionError} from './errors'
import {ArgInvalidOptionError, CLIError, FailedFlagParsingError, FlagInvalidOptionError} from './errors'

let debug: any
try {
Expand Down Expand Up @@ -341,8 +341,10 @@ export class Parser<

return await flag.parse(input, ctx, flag)
} catch (error: any) {
error.message = `Parsing --${flag.name} \n\t${error.message}\nSee more help with --help`
throw error
// console.log(error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't leave the comments in

// error.message = `Parsing --${flag.name} \n\t${error.message}\nSee more help with --help`
// throw error
throw new FailedFlagParsingError({flag: flag.name, message: error.message})
}
}

Expand Down
27 changes: 22 additions & 5 deletions src/parser/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ 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})
throw new NonExistentFlagsError({
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,
parse,
})
}

const missingRequiredArgs: Arg<any>[] = []
Expand All @@ -32,7 +38,10 @@ 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,
parse,
})
}

if (arg.required && !parse.output.args[name] && parse.output.args[name] !== 0) {
Expand All @@ -45,7 +54,11 @@ 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})
throw new RequiredArgsError({
args: missingRequiredArgs,
flagsWithMultiple,
parse,
})
}
}

Expand Down Expand Up @@ -76,7 +89,11 @@ 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 (failed.length > 0)
throw new FailedFlagValidationError({
failed,
parse,
})
}

async function resolveFlags(flags: FlagRelationship[]): Promise<Record<string, unknown>> {
Expand Down
Loading
Loading