Skip to content

Commit

Permalink
feat: support complex flag relationships (#468)
Browse files Browse the repository at this point in the history
* feat: support complex flag relationships

* feat: change type

* feat: add when property

* fix: more improvements

* feat: update when signature

* chore: tests

* chore: more tests

* chore: rename never to none

* chore: code review
  • Loading branch information
mdonnalley committed Aug 24, 2022
1 parent c59dc10 commit 222d1f6
Show file tree
Hide file tree
Showing 7 changed files with 904 additions and 72 deletions.
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';
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
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

0 comments on commit 222d1f6

Please sign in to comment.