From ace31f39008b5ccb7a03db3a3e0be139ff1a26be Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 10 Aug 2022 16:15:35 -0600 Subject: [PATCH 1/9] feat: support complex flag relationships --- src/config/config.ts | 2 + src/interfaces/parser.ts | 10 ++++ src/parser/validate.ts | 124 +++++++++++++++++++++++++++++---------- 3 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/config/config.ts b/src/config/config.ts index b886da2f..8062ff82 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -751,6 +751,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise 0) { + validateAcrossFlags(flag) + } + } + } + + function validateExclusive(name: string, exclusive: string[]) { + for (const also of 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}=`, + ) + } + } + } + + function validateExactlyOne(name: string, exactlyOne: string[]) { + for (const also of exactlyOne || []) { + if (also !== name && parse.output.flags[also]) { + throw new CLIError( + `--${also}= cannot also be provided when using --${name}=`, + ) + } + } + } + + function validateDependsOn(name: string, dependsOn: string[]) { + for (const also of dependsOn || []) { + if (!parse.output.flags[also]) { + throw new CLIError( + `--${also}= must also be provided when using --${name}=`, + ) + } + } + } + + function validateRelationships(name: string, flag: CompletableFlag) { + if (!flag.relationships) return + + if (flag.relationships?.dependsOn) { + const dependsOnFlags = flag.relationships.dependsOn.flags ?? [] + if (flag.relationships?.dependsOn.type === 'all') { + validateDependsOn(name, dependsOnFlags) + } + + if (flag.relationships.dependsOn.type === 'atLeastOne') { + let foundAtLeastOne = false + for (const flag of dependsOnFlags) { + if (parse.output.flags[flag]) { + foundAtLeastOne = true + break } } - 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}=`, - ) - } + if (!foundAtLeastOne) { + const flags = (dependsOnFlags ?? []).map(f => `--${f}`).join(', ') + throw new CLIError(`One of the following must be provided when using --${name}: ${flags}`) } + } + } + + if (flag.relationships?.exclusive) { + const exclusiveFlags = flag.relationships.exclusive.flags ?? [] - for (const also of flag.exactlyOne || []) { - if (also !== name && parse.output.flags[also]) { - throw new CLIError( - `--${also}= cannot also be provided when using --${name}=`, - ) + if (flag.relationships.exclusive.type === 'all') { + validateExclusive(name, exclusiveFlags) + } + + if (flag.relationships.exclusive.type === 'atLeastOne') { + let foundAtLeastOne = false + for (const flag of exclusiveFlags) { + if (parse.output.flags[flag]) { + foundAtLeastOne = true + break } } - } else if (flag.required) { - throw new RequiredFlagError({parse, flag}) - } else if (flag.exactlyOne && flag.exactlyOne.length > 0) { - validateAcrossFlags(flag) + + if (!foundAtLeastOne) { + const flags = (exclusiveFlags ?? []).map(f => `--${f}`).join(', ') + throw new CLIError(`The following cannot be provided when using --${name}: ${flags}`) + } } } } From 0fe00cb46e95ccd46fd81ab774fd4f6bb74f6da8 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 10 Aug 2022 16:46:25 -0600 Subject: [PATCH 2/9] feat: change type --- src/interfaces/parser.ts | 30 ++++++++++++------- src/parser/index.ts | 2 +- src/parser/validate.ts | 64 ++++++++++++++++++---------------------- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts index 3280dff0..6f000417 100644 --- a/src/interfaces/parser.ts +++ b/src/interfaces/parser.ts @@ -86,6 +86,13 @@ export type DefaultContext = { export type Default = T | ((context: DefaultContext) => Promise) export type DefaultHelp = T | ((context: DefaultContext) => Promise) +type Relationship = { + type: 'dependency' | 'exclusive'; + method: 'all' | 'some'; + flags: string[]; + // | Array<{name: string; value: () => Promise}>; +} + export type FlagProps = { name: string; char?: AlphabetLowercase | AlphabetUppercase; @@ -112,16 +119,19 @@ export type FlagProps = { hidden?: boolean; required?: boolean; dependsOn?: string[]; - relationships?: { - dependsOn?: { - type: 'all' | 'atLeastOne'; - flags: string[]; - }; - exclusive?: { - type: 'all' | 'atLeastOne'; - flags: string[]; - } - }; + // relationships?: Array<{ + // dependsOn?: { + // type: 'all' | 'atLeastOne'; + // flags: string[] + // // | Array<{name: string; value: () => Promise}>; + // }; + // exclusive?: { + // type: 'all' | 'atLeastOne'; + // flags: string[] + // // | Array<{name: string; value: () => Promise}>; + // } + // }>; + relationships?: Relationship[]; exclusive?: string[]; } diff --git a/src/parser/index.ts b/src/parser/index.ts index dfbf21ed..75c2ee9a 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -27,7 +27,7 @@ export async function parse } diff --git a/src/parser/validate.ts b/src/parser/validate.ts index 26445812..a66a4559 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -8,7 +8,7 @@ import { } from './errors' import {ParserArg, ParserInput, ParserOutput, Flag, CompletableFlag} from '../interfaces' -export function validate(parse: { +export async function validate(parse: { input: ParserInput; output: ParserOutput; }) { @@ -110,50 +110,42 @@ export function validate(parse: { } } - function validateRelationships(name: string, flag: CompletableFlag) { - if (!flag.relationships) return - - if (flag.relationships?.dependsOn) { - const dependsOnFlags = flag.relationships.dependsOn.flags ?? [] - if (flag.relationships?.dependsOn.type === 'all') { - validateDependsOn(name, dependsOnFlags) + function validateSome(flags: string[], errorMessage: string) { + let foundAtLeastOne = false + for (const flag of flags) { + if (parse.output.flags[flag]) { + foundAtLeastOne = true + break } + } - if (flag.relationships.dependsOn.type === 'atLeastOne') { - let foundAtLeastOne = false - for (const flag of dependsOnFlags) { - if (parse.output.flags[flag]) { - foundAtLeastOne = true - break - } - } - - if (!foundAtLeastOne) { - const flags = (dependsOnFlags ?? []).map(f => `--${f}`).join(', ') - throw new CLIError(`One of the following must be provided when using --${name}: ${flags}`) - } - } + if (!foundAtLeastOne) { + throw new CLIError(errorMessage) } + } - if (flag.relationships?.exclusive) { - const exclusiveFlags = flag.relationships.exclusive.flags ?? [] + function validateRelationships(name: string, flag: CompletableFlag) { + if (!flag.relationships) return + for (const relationship of flag.relationships) { + const flags = relationship.flags ?? [] + const formattedFlags = (flags ?? []).map(f => `--${f}`).join(', ') + if (relationship.type === 'dependency') { + if (relationship.method === 'all') { + validateDependsOn(name, flags) + } - if (flag.relationships.exclusive.type === 'all') { - validateExclusive(name, exclusiveFlags) + if (relationship.method === 'some') { + validateSome(flags, `One of the following must be provided when using --${name}: ${formattedFlags}`) + } } - if (flag.relationships.exclusive.type === 'atLeastOne') { - let foundAtLeastOne = false - for (const flag of exclusiveFlags) { - if (parse.output.flags[flag]) { - foundAtLeastOne = true - break - } + if (relationship.type === 'exclusive') { + if (relationship.method === 'all') { + validateExclusive(name, flags) } - if (!foundAtLeastOne) { - const flags = (exclusiveFlags ?? []).map(f => `--${f}`).join(', ') - throw new CLIError(`The following cannot be provided when using --${name}: ${flags}`) + if (relationship.method === 'some') { + validateSome(flags, `The following cannot be provided when using --${name}: ${formattedFlags}`) } } } From f96df37b53ac12b764e2a291338b85cfb8413c30 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 10 Aug 2022 20:13:35 -0600 Subject: [PATCH 3/9] feat: add when property --- src/config/config.ts | 1 - src/interfaces/parser.ts | 21 ++----- src/parser/validate.ts | 115 +++++++++++++++++++++-------------- test/parser/parse.test.ts | 12 ++-- test/parser/validate.test.ts | 35 +++++++---- 5 files changed, 103 insertions(+), 81 deletions(-) diff --git a/src/config/config.ts b/src/config/config.ts index 8062ff82..3d032b87 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -751,7 +751,6 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise = { export type Default = T | ((context: DefaultContext) => Promise) export type DefaultHelp = T | ((context: DefaultContext) => Promise) -type Relationship = { - type: 'dependency' | 'exclusive'; - method: 'all' | 'some'; - flags: string[]; - // | Array<{name: string; value: () => Promise}>; +export type FlagRelationship = string | {name: string; when: (value: string) => Promise}; +export type Relationship = { + type: 'all' | 'some' | 'never'; + flags: FlagRelationship[]; } export type FlagProps = { @@ -119,18 +118,6 @@ export type FlagProps = { hidden?: boolean; required?: boolean; dependsOn?: string[]; - // relationships?: Array<{ - // dependsOn?: { - // type: 'all' | 'atLeastOne'; - // flags: string[] - // // | Array<{name: string; value: () => Promise}>; - // }; - // exclusive?: { - // type: 'all' | 'atLeastOne'; - // flags: string[] - // // | Array<{name: string; value: () => Promise}>; - // } - // }>; relationships?: Relationship[]; exclusive?: string[]; } diff --git a/src/parser/validate.ts b/src/parser/validate.ts index a66a4559..a4e12b39 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-await-in-loop */ import {CLIError} from '../errors' import { @@ -7,6 +8,7 @@ import { UnexpectedArgsError, } from './errors' import {ParserArg, ParserInput, ParserOutput, Flag, CompletableFlag} from '../interfaces' +import {FlagRelationship} from '../interfaces/parser' export async function validate(parse: { input: ParserInput; @@ -54,12 +56,12 @@ export async function validate(parse: { } } - function validateFlags() { + async function validateFlags() { for (const [name, flag] of Object.entries(parse.input.flags)) { if (parse.output.flags[name] !== undefined) { - validateRelationships(name, flag) - validateDependsOn(name, flag.dependsOn ?? []) - validateExclusive(name, flag.exclusive ?? []) + await validateRelationships(name, flag) + await validateDependsOn(name, flag.dependsOn ?? []) + await validateExclusive(name, flag.exclusive ?? []) validateExactlyOne(name, flag.exactlyOne ?? []) } else if (flag.required) { throw new RequiredFlagError({parse, flag}) @@ -69,12 +71,26 @@ export async function validate(parse: { } } - function validateExclusive(name: string, exclusive: string[]) { - for (const also of exclusive) { + async function validateExclusive(name: string, flags: FlagRelationship[]) { + const promises = flags.map(async flag => { + if (typeof flag === 'string') { + return parse.output.flags[flag] ? flag : false + } + + if (await flag.when(parse.output.flags[flag.name])) { + return parse.output.flags[flag.name] ? flag.name : false + } + + return flag.name + }) + + const results = (await Promise.all(promises)).filter(Boolean) as string[] + + for (const flag of results) { // do not enforce exclusivity for flags that were defaulted if ( - parse.output.metadata.flags[also] && - parse.output.metadata.flags[also].setFromDefault + parse.output.metadata.flags[flag] && + parse.output.metadata.flags[flag].setFromDefault ) continue if ( @@ -82,75 +98,84 @@ export async function validate(parse: { parse.output.metadata.flags[name].setFromDefault ) continue - if (parse.output.flags[also]) { + if (parse.output.flags[flag]) { throw new CLIError( - `--${also}= cannot also be provided when using --${name}=`, + `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`, ) } } } - function validateExactlyOne(name: string, exactlyOne: string[]) { - for (const also of exactlyOne || []) { - if (also !== name && parse.output.flags[also]) { + function validateExactlyOne(name: string, exactlyOne: FlagRelationship[]) { + for (const flag of exactlyOne || []) { + const flagName = typeof flag === 'string' ? flag : flag.name + if (flagName !== name && parse.output.flags[flagName]) { throw new CLIError( - `--${also}= cannot also be provided when using --${name}=`, + `--${flagName} cannot also be provided when using --${name}`, ) } } } - function validateDependsOn(name: string, dependsOn: string[]) { - for (const also of dependsOn || []) { - if (!parse.output.flags[also]) { - throw new CLIError( - `--${also}= must also be provided when using --${name}=`, - ) + async function validateDependsOn(name: string, flags: FlagRelationship[]) { + const promises = flags.map(async flag => { + if (typeof flag === 'string') { + return parse.output.flags[flag] } + + if (await flag.when(parse.output.flags[flag.name])) { + return parse.output.flags[flag.name] + } + + return true + }) + const foundAll = (await Promise.all(promises)).every(Boolean) + if (!foundAll) { + const formattedFlags = (flags ?? []).map(f => typeof f === 'string' ? `--${f}` : `--${f.name}`).join(', ') + throw new CLIError( + `All of the following must be provided when using --${name}: ${formattedFlags}`, + ) } } - function validateSome(flags: string[], errorMessage: string) { - let foundAtLeastOne = false - for (const flag of flags) { - if (parse.output.flags[flag]) { - foundAtLeastOne = true - break + async function validateSome(flags: FlagRelationship[], errorMessage: string) { + const promises = flags.map(async flag => { + if (typeof flag === 'string') { + return parse.output.flags[flag] } - } + if (await flag.when(parse.output.flags[flag.name])) { + return parse.output.flags[flag.name] + } + + return true + }) + + const foundAtLeastOne = (await Promise.all(promises)).some(Boolean) if (!foundAtLeastOne) { throw new CLIError(errorMessage) } } - function validateRelationships(name: string, flag: CompletableFlag) { + async function validateRelationships(name: string, flag: CompletableFlag) { if (!flag.relationships) return for (const relationship of flag.relationships) { const flags = relationship.flags ?? [] - const formattedFlags = (flags ?? []).map(f => `--${f}`).join(', ') - if (relationship.type === 'dependency') { - if (relationship.method === 'all') { - validateDependsOn(name, flags) - } - - if (relationship.method === 'some') { - validateSome(flags, `One of the following must be provided when using --${name}: ${formattedFlags}`) - } + const formattedFlags = (flags ?? []).map(f => typeof f === 'string' ? `--${f}` : `--${f.name}`).join(', ') + if (relationship.type === 'all') { + await validateDependsOn(name, flags) } - if (relationship.type === 'exclusive') { - if (relationship.method === 'all') { - validateExclusive(name, flags) - } + if (relationship.type === 'some') { + await validateSome(flags, `One of the following must be provided when using --${name}: ${formattedFlags}`) + } - if (relationship.method === 'some') { - validateSome(flags, `The following cannot be provided when using --${name}: ${formattedFlags}`) - } + if (relationship.type === 'never') { + await validateExclusive(name, flags) } } } validateArgs() - validateFlags() + await validateFlags() } diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index 35d4900e..de99e21a 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -877,7 +877,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('All of the following must be provided when using --foo: --bar') }) }) @@ -914,7 +914,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('--bar=b cannot also be provided when using --foo') }) }) @@ -948,7 +948,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('--bar cannot also be provided when using --foo') }) it('succeeds if exactly one', async () => { @@ -1009,7 +1009,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('--else cannot also be provided when using --foo') }) it('handles cross-references/pairings that don\'t make sense', async () => { @@ -1027,7 +1027,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('--bar cannot also be provided when using --foo') let message2 = '' try { @@ -1038,7 +1038,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('--else cannot also be provided when using --bar') const out = await parse(['--foo', 'a', '--else', '4'], { flags: crazyFlags, diff --git a/test/parser/validate.test.ts b/test/parser/validate.test.ts index 8676a05b..b62216f4 100644 --- a/test/parser/validate.test.ts +++ b/test/parser/validate.test.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import {expect} from 'chai' import {validate} from '../../src/parser/validate' @@ -26,7 +27,7 @@ describe('validate', () => { '--': true, } - it('enforces exclusivity for flags', () => { + it('enforces exclusivity for flags', async () => { const output = { args: {}, argv: [], @@ -48,11 +49,16 @@ describe('validate', () => { }, } - // @ts-ignore - expect(validate.bind({input, output})).to.throw() + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch { + expect(true).to.be.true + } }) - it('ignores exclusivity for defaulted flags', () => { + it('ignores exclusivity for defaulted flags', async () => { const output = { args: {}, argv: [], @@ -74,11 +80,11 @@ describe('validate', () => { }, } - // @ts-ignore - validate({input, output}) + // @ts-expect-error + await validate({input, output}) }) - it('allows zero for integer', () => { + it('allows zero for integer', async () => { const input = { argv: [], flags: { @@ -108,11 +114,11 @@ describe('validate', () => { }, } - // @ts-ignore - validate({input, output}) + // @ts-expect-error + await validate({input, output}) }) - it('throws when required flag is undefined', () => { + it('throws when required flag is undefined', async () => { const input = { argv: [], flags: { @@ -137,7 +143,12 @@ describe('validate', () => { }, } - // @ts-ignore - expect(validate.bind({input, output})).to.throw() + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch { + expect(true).to.be.true + } }) }) From bbf8c9d7e823a640a88d209b8db8cfa0f30abb37 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 10 Aug 2022 23:30:18 -0600 Subject: [PATCH 4/9] fix: more improvements --- src/interfaces/parser.ts | 2 +- src/parser/validate.ts | 54 ++++++++++++++++------------------------ 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts index c0bedd97..c6e4a420 100644 --- a/src/interfaces/parser.ts +++ b/src/interfaces/parser.ts @@ -86,7 +86,7 @@ export type DefaultContext = { export type Default = T | ((context: DefaultContext) => Promise) export type DefaultHelp = T | ((context: DefaultContext) => Promise) -export type FlagRelationship = string | {name: string; when: (value: string) => Promise}; +export type FlagRelationship = string | {name: string; when: (value: unknown) => Promise}; export type Relationship = { type: 'all' | 'some' | 'never'; flags: FlagRelationship[]; diff --git a/src/parser/validate.ts b/src/parser/validate.ts index a4e12b39..afd2231f 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -71,22 +71,29 @@ export async function validate(parse: { } } - async function validateExclusive(name: string, flags: FlagRelationship[]) { + async function resolveFlags(flags: FlagRelationship[], defaultValue = true): Promise> { const promises = flags.map(async flag => { if (typeof flag === 'string') { - return parse.output.flags[flag] ? flag : false + return [flag, parse.output.flags[flag]] } if (await flag.when(parse.output.flags[flag.name])) { - return parse.output.flags[flag.name] ? flag.name : false + return [flag.name, parse.output.flags[flag.name]] } - return flag.name + return [flag.name, defaultValue] }) + return Object.fromEntries(await Promise.all(promises)) + } - const results = (await Promise.all(promises)).filter(Boolean) as string[] + async function validateExclusive(name: string, flags: FlagRelationship[]) { + const resolved = await resolveFlags(flags, false) + const keys = Object.keys(resolved).reduce((acc, key) => { + if (resolved[key]) acc.push(key) + return acc + }, [] as string[]) - for (const flag of results) { + for (const flag of keys) { // do not enforce exclusivity for flags that were defaulted if ( parse.output.metadata.flags[flag] && @@ -118,20 +125,14 @@ export async function validate(parse: { } async function validateDependsOn(name: string, flags: FlagRelationship[]) { - const promises = flags.map(async flag => { - if (typeof flag === 'string') { - return parse.output.flags[flag] - } - - if (await flag.when(parse.output.flags[flag.name])) { - return parse.output.flags[flag.name] - } - - return true - }) - const foundAll = (await Promise.all(promises)).every(Boolean) + const resolved = await resolveFlags(flags) + const foundAll = Object.values(resolved).every(Boolean) if (!foundAll) { - const formattedFlags = (flags ?? []).map(f => typeof f === 'string' ? `--${f}` : `--${f.name}`).join(', ') + const required = Object.keys(resolved).reduce((acc, key) => { + if (!resolved[key]) acc.push(key) + return acc + }, [] as string[]) + const formattedFlags = required.map(f => `--${f}`).join(', ') throw new CLIError( `All of the following must be provided when using --${name}: ${formattedFlags}`, ) @@ -139,19 +140,8 @@ export async function validate(parse: { } async function validateSome(flags: FlagRelationship[], errorMessage: string) { - const promises = flags.map(async flag => { - if (typeof flag === 'string') { - return parse.output.flags[flag] - } - - if (await flag.when(parse.output.flags[flag.name])) { - return parse.output.flags[flag.name] - } - - return true - }) - - const foundAtLeastOne = (await Promise.all(promises)).some(Boolean) + const resolved = await resolveFlags(flags) + const foundAtLeastOne = Object.values(resolved).some(Boolean) if (!foundAtLeastOne) { throw new CLIError(errorMessage) } From 3c58582968e2cbf3afb67ceee38f910e90c8c197 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 11 Aug 2022 09:08:47 -0600 Subject: [PATCH 5/9] feat: update when signature --- src/interfaces/parser.ts | 2 +- src/parser/validate.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts index c6e4a420..52be1f5e 100644 --- a/src/interfaces/parser.ts +++ b/src/interfaces/parser.ts @@ -86,7 +86,7 @@ export type DefaultContext = { export type Default = T | ((context: DefaultContext) => Promise) export type DefaultHelp = T | ((context: DefaultContext) => Promise) -export type FlagRelationship = string | {name: string; when: (value: unknown) => Promise}; +export type FlagRelationship = string | {name: string; when: (flags: Record) => Promise}; export type Relationship = { type: 'all' | 'some' | 'never'; flags: FlagRelationship[]; diff --git a/src/parser/validate.ts b/src/parser/validate.ts index afd2231f..1bcff7d2 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -77,7 +77,7 @@ export async function validate(parse: { return [flag, parse.output.flags[flag]] } - if (await flag.when(parse.output.flags[flag.name])) { + if (await flag.when(parse.output.flags)) { return [flag.name, parse.output.flags[flag.name]] } From c67882a95b54ce537786168d9cc721a3f05fb8ac Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 11 Aug 2022 10:25:01 -0600 Subject: [PATCH 6/9] chore: tests --- src/parser/validate.ts | 29 +- test/parser/validate.test.ts | 566 ++++++++++++++++++++++++++++++++++- 2 files changed, 573 insertions(+), 22 deletions(-) diff --git a/src/parser/validate.ts b/src/parser/validate.ts index 1bcff7d2..7fac0c4f 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -71,23 +71,21 @@ export async function validate(parse: { } } - async function resolveFlags(flags: FlagRelationship[], defaultValue = true): Promise> { + async function resolveFlags(flags: FlagRelationship[]): Promise> { const promises = flags.map(async flag => { if (typeof flag === 'string') { return [flag, parse.output.flags[flag]] } - if (await flag.when(parse.output.flags)) { - return [flag.name, parse.output.flags[flag.name]] - } - - return [flag.name, defaultValue] + const result = await flag.when(parse.output.flags) + return result ? [flag.name, parse.output.flags[flag.name]] : null }) - return Object.fromEntries(await Promise.all(promises)) + const resolved = await Promise.all(promises) + return Object.fromEntries(resolved.filter(r => r !== null) as [string, unknown][]) } async function validateExclusive(name: string, flags: FlagRelationship[]) { - const resolved = await resolveFlags(flags, false) + const resolved = await resolveFlags(flags) const keys = Object.keys(resolved).reduce((acc, key) => { if (resolved[key]) acc.push(key) return acc @@ -128,22 +126,19 @@ export async function validate(parse: { const resolved = await resolveFlags(flags) const foundAll = Object.values(resolved).every(Boolean) if (!foundAll) { - const required = Object.keys(resolved).reduce((acc, key) => { - if (!resolved[key]) acc.push(key) - return acc - }, [] as string[]) - const formattedFlags = required.map(f => `--${f}`).join(', ') + const formattedFlags = Object.keys(resolved).map(f => `--${f}`).join(', ') throw new CLIError( `All of the following must be provided when using --${name}: ${formattedFlags}`, ) } } - async function validateSome(flags: FlagRelationship[], errorMessage: string) { + async function validateSome(name: string, flags: FlagRelationship[]) { const resolved = await resolveFlags(flags) const foundAtLeastOne = Object.values(resolved).some(Boolean) if (!foundAtLeastOne) { - throw new CLIError(errorMessage) + const formattedFlags = Object.keys(resolved).map(f => `--${f}`).join(', ') + throw new CLIError(`One of the following must be provided when using --${name}: ${formattedFlags}`) } } @@ -151,13 +146,13 @@ export async function validate(parse: { if (!flag.relationships) return for (const relationship of flag.relationships) { const flags = relationship.flags ?? [] - const formattedFlags = (flags ?? []).map(f => typeof f === 'string' ? `--${f}` : `--${f.name}`).join(', ') + if (relationship.type === 'all') { await validateDependsOn(name, flags) } if (relationship.type === 'some') { - await validateSome(flags, `One of the following must be provided when using --${name}: ${formattedFlags}`) + await validateSome(name, flags) } if (relationship.type === 'never') { diff --git a/test/parser/validate.test.ts b/test/parser/validate.test.ts index b62216f4..7a99694d 100644 --- a/test/parser/validate.test.ts +++ b/test/parser/validate.test.ts @@ -1,5 +1,6 @@ -import assert from 'assert' +import * as assert from 'assert' import {expect} from 'chai' +import {CLIError} from '../../src/errors' import {validate} from '../../src/parser/validate' @@ -53,8 +54,9 @@ describe('validate', () => { // @ts-expect-error await validate({input, output}) assert.fail('should have thrown') - } catch { - expect(true).to.be.true + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('--dessert=cheesecake cannot also be provided when using --dinner') } }) @@ -147,8 +149,562 @@ describe('validate', () => { // @ts-expect-error await validate({input, output}) assert.fail('should have thrown') - } catch { - expect(true).to.be.true + } catch (error) { + const err = error as CLIError + expect(err.message).to.include('Missing required flag') } }) + + describe('relationships', () => { + describe('type: all', () => { + it('should pass if all required flags are provided', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', sprinkles: true, cookies: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + {type: 'flag', flag: 'cookies', input: true}, + ], + metadata: {}, + } + + // @ts-expect-error + await validate({input, output}) + }) + + it('should require all specified flags', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream'}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies, --sprinkles') + } + }) + + it('should require all specified flags with when property that resolves to true', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: [ + 'cookies', + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: true}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies, --sprinkles') + } + }) + + it('should require all specified flags with when property that resolves to false', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: [ + 'cookies', + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: false}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies') + } + }) + }) + + describe('type: some', () => { + it('should pass if some of the specified flags are provided', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'some', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', sprinkles: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + ], + metadata: {}, + } + + // @ts-expect-error + await validate({input, output}) + }) + + it('should require some of the specified flags', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'some', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream'}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies, --sprinkles') + } + }) + + it('should require some of the specified flags with when property that resolves to true', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'some', + flags: [ + 'cookies', + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: true}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies, --sprinkles') + } + }) + + it('should require some of the specified flags with when property that resolves to false', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'some', + flags: [ + 'cookies', + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: false}, + raw: [{type: 'flag', flag: 'dessert', input: 'ice-cream'}], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies') + } + }) + }) + + describe('type: never', () => { + it('should pass if none of the specified flags are provided', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'never', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream'}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + ], + metadata: {}, + } + + // @ts-expect-error + await validate({input, output}) + }) + + it('should fail if the specified flags are provided', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'never', + flags: ['cookies', 'sprinkles'], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', sprinkles: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + ], + metadata: { + flags: { + dessert: {setFromDefault: false}, + sprinkles: {setFromDefault: false}, + }, + }, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('--sprinkles=true cannot also be provided when using --dessert') + } + }) + + it('should fail if the specified flags are provided with when property that resolves to true', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'never', + flags: [ + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: true, sprinkles: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + {type: 'flag', flag: 'birthday', input: true}, + ], + metadata: { + flags: { + dessert: {setFromDefault: false}, + sprinkles: {setFromDefault: false}, + birthday: {setFromDefault: false}, + }, + }, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.equal('--sprinkles=true cannot also be provided when using --dessert') + } + }) + + it('should pass if the specified flags are provided with when property that resolves to false', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + birthday: {input: [], name: 'birthday'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'never', + flags: [ + { + name: 'sprinkles', + when: async (flags: {birthday: boolean}) => Promise.resolve(flags.birthday), + }, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', birthday: false, sprinkles: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + {type: 'flag', flag: 'birthday', input: false}, + ], + metadata: { + flags: { + dessert: {setFromDefault: false}, + sprinkles: {setFromDefault: false}, + birthday: {setFromDefault: false}, + }, + }, + } + + // @ts-expect-error + await validate({input, output}) + }) + }) + }) }) From 4deb315170613f32ec3a10a756384860d19ca686 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 11 Aug 2022 10:37:00 -0600 Subject: [PATCH 7/9] chore: more tests --- test/parser/validate.test.ts | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/parser/validate.test.ts b/test/parser/validate.test.ts index 7a99694d..a1cf1210 100644 --- a/test/parser/validate.test.ts +++ b/test/parser/validate.test.ts @@ -196,6 +196,47 @@ describe('validate', () => { await validate({input, output}) }) + it('should exclude any flags whose when property resolves to false', async () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: [ + 'cookies', + {name: 'sprinkles', when: async () => Promise.resolve(false)}, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + const output = { + args: {}, + argv: [], + flags: {dessert: 'ice-cream', cookies: true}, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'cookies', input: true}, + ], + metadata: {}, + } + + // @ts-expect-error + await validate({input, output}) + }) + it('should require all specified flags', async () => { const input = { argv: [], From 57c1c9782f4da1e39d8da3d2bc7dd0c581fa0772 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 17 Aug 2022 13:19:38 -0600 Subject: [PATCH 8/9] chore: rename never to none --- src/interfaces/parser.ts | 2 +- src/parser/validate.ts | 2 +- test/parser/validate.test.ts | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts index f76c15c0..59902d1e 100644 --- a/src/interfaces/parser.ts +++ b/src/interfaces/parser.ts @@ -88,7 +88,7 @@ export type DefaultHelp = T | ((context: DefaultContext) => Promise) => Promise}; export type Relationship = { - type: 'all' | 'some' | 'never'; + type: 'all' | 'some' | 'none'; flags: FlagRelationship[]; } diff --git a/src/parser/validate.ts b/src/parser/validate.ts index 7fac0c4f..b6fcd379 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -155,7 +155,7 @@ export async function validate(parse: { await validateSome(name, flags) } - if (relationship.type === 'never') { + if (relationship.type === 'none') { await validateExclusive(name, flags) } } diff --git a/test/parser/validate.test.ts b/test/parser/validate.test.ts index a1cf1210..c16d34f6 100644 --- a/test/parser/validate.test.ts +++ b/test/parser/validate.test.ts @@ -552,7 +552,7 @@ describe('validate', () => { }) }) - describe('type: never', () => { + describe('type: none', () => { it('should pass if none of the specified flags are provided', async () => { const input = { argv: [], @@ -564,7 +564,7 @@ describe('validate', () => { name: 'dessert', relationships: [ { - type: 'never', + type: 'none', flags: ['cookies', 'sprinkles'], }, ], @@ -601,7 +601,7 @@ describe('validate', () => { name: 'dessert', relationships: [ { - type: 'never', + type: 'none', flags: ['cookies', 'sprinkles'], }, ], @@ -651,7 +651,7 @@ describe('validate', () => { name: 'dessert', relationships: [ { - type: 'never', + type: 'none', flags: [ { name: 'sprinkles', @@ -708,7 +708,7 @@ describe('validate', () => { name: 'dessert', relationships: [ { - type: 'never', + type: 'none', flags: [ { name: 'sprinkles', From c455eda93cefd1291060306bb8739d966b40c65b Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 24 Aug 2022 09:13:36 -0600 Subject: [PATCH 9/9] chore: code review --- src/parser/errors.ts | 20 +++++ src/parser/validate.ts | 154 ++++++++++++++++++++--------------- test/parser/parse.test.ts | 16 ++-- test/parser/validate.test.ts | 127 +++++++++++++++++++++++++++-- 4 files changed, 234 insertions(+), 83 deletions(-) diff --git a/src/parser/errors.ts b/src/parser/errors.ts index 22c97c19..b0c952f3 100644 --- a/src/parser/errors.ts +++ b/src/parser/errors.ts @@ -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' @@ -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'] @@ -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}) + } +} diff --git a/src/parser/validate.ts b/src/parser/validate.ts index b6fcd379..be3ec157 100644 --- a/src/parser/validate.ts +++ b/src/parser/validate.ts @@ -1,14 +1,13 @@ -/* eslint-disable no-await-in-loop */ -import {CLIError} from '../errors' - import { InvalidArgsSpecError, RequiredArgsError, - RequiredFlagError, + Validation, UnexpectedArgsError, + FailedFlagValidationError, } from './errors' import {ParserArg, ParserInput, ParserOutput, Flag, CompletableFlag} from '../interfaces' import {FlagRelationship} from '../interfaces/parser' +import {uniq} from '../config/util' export async function validate(parse: { input: ParserInput; @@ -43,32 +42,29 @@ export async function validate(parse: { } } - function validateAcrossFlags(flag: Flag) { - 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(', ')}`) - } - } - async function validateFlags() { - for (const [name, flag] of Object.entries(parse.input.flags)) { + const promises = Object.entries(parse.input.flags).map(async ([name, flag]) => { + const results: Validation[] = [] if (parse.output.flags[name] !== undefined) { - await validateRelationships(name, flag) - await validateDependsOn(name, flag.dependsOn ?? []) - await validateExclusive(name, flag.exclusive ?? []) - validateExactlyOne(name, flag.exactlyOne ?? []) + 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) { - throw new RequiredFlagError({parse, flag}) + results.push({status: 'failed', name, validationFn: 'required', reason: `Missing required flag ${name}`}) } else if (flag.exactlyOne && flag.exactlyOne.length > 0) { - validateAcrossFlags(flag) + 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> { @@ -84,81 +80,107 @@ export async function validate(parse: { return Object.fromEntries(resolved.filter(r => r !== null) as [string, unknown][]) } - async function validateExclusive(name: string, flags: FlagRelationship[]) { - const resolved = await resolveFlags(flags) - const keys = Object.keys(resolved).reduce((acc, key) => { - if (resolved[key]) acc.push(key) + function getPresentFlags(flags: Record): string[] { + return Object.keys(flags).reduce((acc, key) => { + if (flags[key]) acc.push(key) return acc }, [] as string[]) + } + function validateAcrossFlags(flag: Flag): 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 + 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'} + } + + async function validateExclusive(name: string, flags: FlagRelationship[]): Promise { + 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[flag] && - parse.output.metadata.flags[flag].setFromDefault - ) + if (parse.output.metadata.flags && parse.output.metadata.flags[flag]?.setFromDefault) continue - if ( - parse.output.metadata.flags[name] && - parse.output.metadata.flags[name].setFromDefault - ) + if (parse.output.metadata.flags && parse.output.metadata.flags[name]?.setFromDefault) continue if (parse.output.flags[flag]) { - throw new CLIError( - `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`, - ) + return {...base, status: 'failed', reason: `--${flag}=${parse.output.flags[flag]} cannot also be provided when using --${name}`} } } + + return {...base, status: 'success'} } - function validateExactlyOne(name: string, exactlyOne: FlagRelationship[]) { - for (const flag of exactlyOne || []) { - const flagName = typeof flag === 'string' ? flag : flag.name - if (flagName !== name && parse.output.flags[flagName]) { - throw new CLIError( - `--${flagName} cannot also be provided when using --${name}`, - ) + async function validateExactlyOne(name: string, flags: FlagRelationship[]): Promise { + 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[]) { + async function validateDependsOn(name: string, flags: FlagRelationship[]): Promise { + 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(', ') - throw new CLIError( - `All of the following must be provided when using --${name}: ${formattedFlags}`, - ) + 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[]) { + async function validateSome(name: string, flags: FlagRelationship[]): Promise { + 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(', ') - throw new CLIError(`One of the following must be provided when using --${name}: ${formattedFlags}`) + 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) { - if (!flag.relationships) return - for (const relationship of flag.relationships) { + async function validateRelationships(name: string, flag: CompletableFlag): Promise { + if (!flag.relationships) return [] + const results = await Promise.all(flag.relationships.map(async relationship => { const flags = relationship.flags ?? [] - - if (relationship.type === 'all') { - await validateDependsOn(name, 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 } - if (relationship.type === 'some') { - await validateSome(name, flags) - } + return results + })) - if (relationship.type === 'none') { - await validateExclusive(name, flags) - } - } + return results.flat() } validateArgs() diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index 7acaaaf1..49afcc9b 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -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', ) }) @@ -925,7 +925,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('All of the following must be provided when using --foo: --bar') + 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') }) }) @@ -962,7 +962,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('--bar=b 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') }) }) @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 { @@ -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, diff --git a/test/parser/validate.test.ts b/test/parser/validate.test.ts index c16d34f6..a64123bc 100644 --- a/test/parser/validate.test.ts +++ b/test/parser/validate.test.ts @@ -56,7 +56,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('--dessert=cheesecake cannot also be provided when using --dinner') + expect(err.message).to.include('--dessert=cheesecake cannot also be provided when using --dinner') } }) @@ -274,7 +274,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies, --sprinkles') + expect(err.message).to.include('All of the following must be provided when using --dessert: --cookies, --sprinkles') } }) @@ -322,7 +322,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies, --sprinkles') + expect(err.message).to.include('All of the following must be provided when using --dessert: --cookies, --sprinkles') } }) @@ -370,7 +370,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('All of the following must be provided when using --dessert: --cookies') + expect(err.message).to.include('All of the following must be provided when using --dessert: --cookies') } }) }) @@ -451,7 +451,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies, --sprinkles') + expect(err.message).to.include('One of the following must be provided when using --dessert: --cookies, --sprinkles') } }) @@ -499,7 +499,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies, --sprinkles') + expect(err.message).to.include('One of the following must be provided when using --dessert: --cookies, --sprinkles') } }) @@ -547,7 +547,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('One of the following must be provided when using --dessert: --cookies') + expect(err.message).to.include('One of the following must be provided when using --dessert: --cookies') } }) }) @@ -635,7 +635,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('--sprinkles=true cannot also be provided when using --dessert') + expect(err.message).to.include('--sprinkles=true cannot also be provided when using --dessert') } }) @@ -692,7 +692,7 @@ describe('validate', () => { assert.fail('should have thrown') } catch (error) { const err = error as CLIError - expect(err.message).to.equal('--sprinkles=true cannot also be provided when using --dessert') + expect(err.message).to.include('--sprinkles=true cannot also be provided when using --dessert') } }) @@ -747,5 +747,114 @@ describe('validate', () => { await validate({input, output}) }) }) + + describe('mixed', () => { + const input = { + argv: [], + flags: { + cookies: {input: [], name: 'cookies'}, + sprinkles: {input: [], name: 'sprinkles'}, + cake: {input: [], name: 'cake'}, + brownies: {input: [], name: 'brownies'}, + pie: {input: [], name: 'pie'}, + fudge: {input: [], name: 'fudge'}, + cupcake: {input: [], name: 'cupcake'}, + muffin: {input: [], name: 'muffin'}, + scone: {input: [], name: 'scone'}, + dessert: { + input: [], + name: 'dessert', + relationships: [ + { + type: 'all', + flags: [ + 'cookies', + {name: 'sprinkles', when: async () => Promise.resolve(false)}, + {name: 'cake', when: async () => Promise.resolve(true)}, + ], + }, + { + type: 'some', + flags: [ + 'brownies', + {name: 'pie', when: async () => Promise.resolve(false)}, + {name: 'fudge', when: async () => Promise.resolve(true)}, + ], + }, + { + type: 'none', + flags: [ + 'cupcake', + {name: 'muffin', when: async () => Promise.resolve(false)}, + {name: 'scone', when: async () => Promise.resolve(true)}, + ], + }, + ], + }, + }, + args: [], + strict: true, + context: {}, + '--': true, + } + + it('should succeed', async () => { + const output = { + args: {}, + argv: [], + flags: { + dessert: 'ice-cream', + cookies: true, + brownies: true, + cake: true, + muffin: true, + }, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'cookies', input: true}, + {type: 'flag', flag: 'brownies', input: true}, + {type: 'flag', flag: 'cake', input: true}, + {type: 'flag', flag: 'muffin', input: true}, + ], + metadata: {}, + } + + // @ts-expect-error + await validate({input, output}) + }) + + it('should fail', async () => { + const output = { + args: {}, + argv: [], + flags: { + dessert: 'ice-cream', + sprinkles: true, + cake: true, + scone: true, + pie: true, + }, + raw: [ + {type: 'flag', flag: 'dessert', input: 'ice-cream'}, + {type: 'flag', flag: 'sprinkles', input: true}, + {type: 'flag', flag: 'cake', input: true}, + {type: 'flag', flag: 'scone', input: true}, + {type: 'flag', flag: 'pie', input: true}, + ], + metadata: {}, + } + + try { + // @ts-expect-error + await validate({input, output}) + assert.fail('should have thrown') + } catch (error) { + const err = error as CLIError + expect(err.message).to.include('All of the following must be provided when using --dessert: --cookies, --cake') + expect(err.message).to.include('--scone=true cannot also be provided when using --dessert') + expect(err.message).to.include('One of the following must be provided when using --dessert: --brownies, --fudge') + } + }) + }) }) })