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

feat: support complex flag relationships #468

Merged
merged 12 commits into from Aug 24, 2022
1 change: 1 addition & 0 deletions src/config/config.ts
Expand Up @@ -768,6 +768,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
multiple: flag.multiple,
options: flag.options,
dependsOn: flag.dependsOn,
relationships: flag.relationships,
exclusive: flag.exclusive,
default: await defaultToCached(flag),
}
Expand Down
10 changes: 10 additions & 0 deletions src/interfaces/parser.ts
Expand Up @@ -86,6 +86,12 @@ export type DefaultContext<T, P> = {
export type Default<T, P = Record<string, unknown>> = T | ((context: DefaultContext<T, P>) => Promise<T>)
export type DefaultHelp<T, P = Record<string, unknown>> = T | ((context: DefaultContext<T, P>) => Promise<string | undefined>)

export type FlagRelationship = string | {name: string; when: (flags: Record<string, unknown>) => Promise<boolean>};
export type Relationship = {
type: 'all' | 'some' | 'none';
Copy link
Member

Choose a reason for hiding this comment

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

consider one or 1 since we've already got an exactlyOne since it happened often enough.

flags: FlagRelationship[];
}

export type FlagProps = {
name: string;
char?: AlphabetLowercase | AlphabetUppercase;
Expand Down Expand Up @@ -133,6 +139,10 @@ export type FlagProps = {
* Exactly one of these flags must be provided.
*/
exactlyOne?: string[];
/**
* Define complex relationships between flags.
*/
relationships?: Relationship[];
}

export type BooleanFlagProps = FlagProps & {
Expand Down
20 changes: 20 additions & 0 deletions src/parser/errors.ts
Expand Up @@ -3,7 +3,9 @@ import {CLIError} from '../errors'
import Deps from './deps'
import * as Help from './help'
import * as List from './list'
import * as chalk from 'chalk'
import {ParserArg, CLIParseErrorOptions, OptionFlag, Flag} from '../interfaces'
import {uniq} from '../config/util'

export {CLIError} from '../errors'

Expand All @@ -13,6 +15,14 @@ const m = Deps()
.add('help', () => require('./help') as typeof Help)
// eslint-disable-next-line node/no-missing-require
.add('list', () => require('./list') as typeof List)
.add('chalk', () => require('chalk') as typeof chalk)

export type Validation = {
name: string;
status: 'success' | 'failed';
validationFn: string;
reason?: string;
}

export class CLIParseError extends CLIError {
public parse: CLIParseErrorOptions['parse']
Expand Down Expand Up @@ -90,3 +100,13 @@ export class ArgInvalidOptionError extends CLIParseError {
super({parse: {}, message})
}
}

export class FailedFlagValidationError extends CLIParseError {
constructor({parse, failed}: 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 ${m.chalk.dim(deduped.join('\n '))}`
super({parse, message})
}
}
2 changes: 1 addition & 1 deletion src/parser/index.ts
Expand Up @@ -27,7 +27,7 @@ export async function parse<TFlags, GFlags, TArgs extends { [name: string]: stri
}
const parser = new Parser(input)
const output = await parser.parse()
m.validate({input, output})
await m.validate({input, output})
return output as ParserOutput<TFlags, GFlags, TArgs>
}

Expand Down
186 changes: 135 additions & 51 deletions src/parser/validate.ts
@@ -1,14 +1,15 @@
import {CLIError} from '../errors'

import {
InvalidArgsSpecError,
RequiredArgsError,
RequiredFlagError,
Validation,
UnexpectedArgsError,
FailedFlagValidationError,
} from './errors'
import {ParserArg, ParserInput, ParserOutput, Flag} from '../interfaces'
import {ParserArg, ParserInput, ParserOutput, Flag, CompletableFlag} from '../interfaces'
import {FlagRelationship} from '../interfaces/parser'
import {uniq} from '../config/util'

export function validate(parse: {
export async function validate(parse: {
input: ParserInput;
output: ParserOutput;
}) {
Expand Down Expand Up @@ -41,64 +42,147 @@ export function validate(parse: {
}
}

function validateAcrossFlags(flag: Flag<any>) {
async function validateFlags() {
const promises = Object.entries(parse.input.flags).map(async ([name, flag]) => {
const results: Validation[] = []
if (parse.output.flags[name] !== undefined) {
results.push(
...await validateRelationships(name, flag),
await validateDependsOn(name, flag.dependsOn ?? []),
await validateExclusive(name, flag.exclusive ?? []),
await validateExactlyOne(name, flag.exactlyOne ?? []),
)
} else if (flag.required) {
results.push({status: 'failed', name, validationFn: 'required', reason: `Missing required flag ${name}`})
} else if (flag.exactlyOne && flag.exactlyOne.length > 0) {
results.push(validateAcrossFlags(flag))
}

return results
})

const results = (await Promise.all(promises)).flat()

const failed = results.filter(r => r.status === 'failed')
if (failed.length > 0) throw new FailedFlagValidationError({parse, failed})
}

async function resolveFlags(flags: FlagRelationship[]): Promise<Record<string, unknown>> {
const promises = flags.map(async flag => {
if (typeof flag === 'string') {
return [flag, parse.output.flags[flag]]
}

const result = await flag.when(parse.output.flags)
return result ? [flag.name, parse.output.flags[flag.name]] : null
})
const resolved = await Promise.all(promises)
return Object.fromEntries(resolved.filter(r => r !== null) as [string, unknown][])
}

function getPresentFlags(flags: Record<string, unknown>): string[] {
return Object.keys(flags).reduce((acc, key) => {
if (flags[key]) acc.push(key)
return acc
}, [] as string[])
}

function validateAcrossFlags(flag: Flag<any>): Validation {
const base = {name: flag.name, validationFn: 'validateAcrossFlags'}
const intersection = Object.entries(parse.input.flags)
.map(entry => entry[0]) // array of flag names
.filter(flagName => parse.output.flags[flagName] !== undefined) // with values
.filter(flagName => flag.exactlyOne && flag.exactlyOne.includes(flagName)) // and in the exactlyOne list
if (intersection.length === 0) {
// the command's exactlyOne may or may not include itself, so we'll use Set to add + de-dupe
throw new CLIError(`Exactly one of the following must be provided: ${[
...new Set(flag.exactlyOne?.map(flag => `--${flag}`)),
].join(', ')}`)
const deduped = uniq(flag.exactlyOne?.map(flag => `--${flag}`) ?? []).join(', ')
const reason = `Exactly one of the following must be provided: ${deduped}`
return {...base, status: 'failed', reason}
}

return {...base, status: 'success'}
}

function validateFlags() {
for (const [name, flag] of Object.entries(parse.input.flags)) {
if (parse.output.flags[name] !== undefined) {
for (const also of flag.dependsOn || []) {
if (!parse.output.flags[also]) {
throw new CLIError(
`--${also}= must also be provided when using --${name}=`,
)
}
}

for (const also of flag.exclusive || []) {
// do not enforce exclusivity for flags that were defaulted
if (
parse.output.metadata.flags[also] &&
parse.output.metadata.flags[also].setFromDefault
)
continue
if (
parse.output.metadata.flags[name] &&
parse.output.metadata.flags[name].setFromDefault
)
continue
if (parse.output.flags[also]) {
throw new CLIError(
`--${also}= cannot also be provided when using --${name}=`,
)
}
}

for (const also of flag.exactlyOne || []) {
if (also !== name && parse.output.flags[also]) {
throw new CLIError(
`--${also}= cannot also be provided when using --${name}=`,
)
}
}
} else if (flag.required) {
throw new RequiredFlagError({parse, flag})
} else if (flag.exactlyOne && flag.exactlyOne.length > 0) {
validateAcrossFlags(flag)
async function validateExclusive(name: string, flags: FlagRelationship[]): Promise<Validation> {
const base = {name, validationFn: 'validateExclusive'}
const resolved = await resolveFlags(flags)
const keys = getPresentFlags(resolved)
for (const flag of keys) {
// do not enforce exclusivity for flags that were defaulted
Copy link
Member

Choose a reason for hiding this comment

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

consider making this configurable. Flag B exists, but can't be used unless you make Flag A something other than its default

if (parse.output.metadata.flags && parse.output.metadata.flags[flag]?.setFromDefault)
continue
if (parse.output.metadata.flags && parse.output.metadata.flags[name]?.setFromDefault)
continue
if (parse.output.flags[flag]) {
return {...base, status: 'failed', reason: `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`}
}
}

return {...base, status: 'success'}
}

async function validateExactlyOne(name: string, flags: FlagRelationship[]): Promise<Validation> {
const base = {name, validationFn: 'validateExactlyOne'}
const resolved = await resolveFlags(flags)
const keys = getPresentFlags(resolved)
for (const flag of keys) {
if (flag !== name && parse.output.flags[flag]) {
return {...base, status: 'failed', reason: `--${flag} cannot also be provided when using --${name}`}
}
}

return {...base, status: 'success'}
}

async function validateDependsOn(name: string, flags: FlagRelationship[]): Promise<Validation> {
const base = {name, validationFn: 'validateDependsOn'}
const resolved = await resolveFlags(flags)
const foundAll = Object.values(resolved).every(Boolean)
if (!foundAll) {
const formattedFlags = Object.keys(resolved).map(f => `--${f}`).join(', ')
return {...base, status: 'failed', reason: `All of the following must be provided when using --${name}: ${formattedFlags}`}
}

return {...base, status: 'success'}
}

async function validateSome(name: string, flags: FlagRelationship[]): Promise<Validation> {
const base = {name, validationFn: 'validateSome'}
const resolved = await resolveFlags(flags)
const foundAtLeastOne = Object.values(resolved).some(Boolean)
if (!foundAtLeastOne) {
const formattedFlags = Object.keys(resolved).map(f => `--${f}`).join(', ')
return {...base, status: 'failed', reason: `One of the following must be provided when using --${name}: ${formattedFlags}`}
}

return {...base, status: 'success'}
}

async function validateRelationships(name: string, flag: CompletableFlag<any>): Promise<Validation[]> {
if (!flag.relationships) return []
const results = await Promise.all(flag.relationships.map(async relationship => {
const flags = relationship.flags ?? []
const results = []
switch (relationship.type) {
case 'all':
results.push(await validateDependsOn(name, flags))
break
case 'some':
results.push(await validateSome(name, flags))
break
case 'none':
results.push(await validateExclusive(name, flags))
break
default:
break
}

return results
}))

return results.flat()
}

validateArgs()
validateFlags()
await validateFlags()
}
16 changes: 8 additions & 8 deletions test/parser/parse.test.ts
Expand Up @@ -135,7 +135,7 @@ describe('parse', () => {
}

expect(message).to.equal(
'Missing required flag:\n --myflag MYFLAG flag description\nSee more help with --help',
'The following error occurred:\n Missing required flag myflag\nSee more help with --help',
)
})

Expand Down Expand Up @@ -925,7 +925,7 @@ See more help with --help`)
message = error.message
}

expect(message).to.equal('--bar= must also be provided when using --foo=')
expect(message).to.equal('The following error occurred:\n All of the following must be provided when using --foo: --bar\nSee more help with --help')
})
})

Expand Down Expand Up @@ -962,7 +962,7 @@ See more help with --help`)
message = error.message
}

expect(message).to.equal('--bar= cannot also be provided when using --foo=')
expect(message).to.equal('The following error occurred:\n --bar=b cannot also be provided when using --foo\nSee more help with --help')
})
})

Expand All @@ -980,7 +980,7 @@ See more help with --help`)
message = error.message
}

expect(message).to.equal('Exactly one of the following must be provided: --bar, --foo')
expect(message).to.equal('The following error occurred:\n Exactly one of the following must be provided: --bar, --foo\nSee more help with --help')
})

it('throws if multiple are set', async () => {
Expand All @@ -996,7 +996,7 @@ See more help with --help`)
message = error.message
}

expect(message).to.equal('--bar= cannot also be provided when using --foo=')
expect(message).to.equal('The following errors occurred:\n --bar cannot also be provided when using --foo\n --foo cannot also be provided when using --bar\nSee more help with --help')
})

it('succeeds if exactly one', async () => {
Expand Down Expand Up @@ -1057,7 +1057,7 @@ See more help with --help`)
message = error.message
}

expect(message).to.equal('--else= cannot also be provided when using --foo=')
expect(message).to.equal('The following errors occurred:\n --else cannot also be provided when using --foo\n --foo cannot also be provided when using --else\nSee more help with --help')
})

it('handles cross-references/pairings that don\'t make sense', async () => {
Expand All @@ -1075,7 +1075,7 @@ See more help with --help`)
message1 = error.message
}

expect(message1).to.equal('--bar= cannot also be provided when using --foo=')
expect(message1).to.equal('The following error occurred:\n --bar cannot also be provided when using --foo\nSee more help with --help')

let message2 = ''
try {
Expand All @@ -1086,7 +1086,7 @@ See more help with --help`)
message2 = error.message
}

expect(message2).to.equal('--else= cannot also be provided when using --bar=')
expect(message2).to.equal('The following error occurred:\n --else cannot also be provided when using --bar\nSee more help with --help')

const out = await parse(['--foo', 'a', '--else', '4'], {
flags: crazyFlags,
Expand Down