From f8d003448e2286982b6f285c03def3eae6199e76 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 7 Feb 2023 10:57:03 -0600 Subject: [PATCH 1/5] fix: handling quoted strings and options validation for comma-delimited multiple-flag --- src/parser/parse.ts | 10 ++- test/parser/parse.test.ts | 162 +++++++++++++++++++++++++++++--------- 2 files changed, 134 insertions(+), 38 deletions(-) diff --git a/src/parser/parse.ts b/src/parser/parse.ts index 492d27d60..fcd769a18 100644 --- a/src/parser/parse.ts +++ b/src/parser/parse.ts @@ -227,15 +227,21 @@ export class Parser this._parseFlag(v.trim(), flag, token)), + input.split(flag.delimiter).map(async v => this._parseFlag(v.trim().replace(/^"(.*)"$/, '$1'), flag, token)), ) + // the parse that each element aligns with the `options` property + for (const v of values) { + this._validateOptions(flag, v) + } + flags[token.flag] = flags[token.flag] || [] flags[token.flag].push(...values) } else { + this._validateOptions(flag, input) const value = await this._parseFlag(input, flag, token) if (flag.multiple) { flags[token.flag] = flags[token.flag] || [] diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index ac5b9a598..2b3ada119 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -1,5 +1,5 @@ /* eslint-disable max-nested-callbacks */ -import {assert, expect} from 'chai' +import {assert, expect, config} from 'chai' import * as fs from 'fs' import {parse} from '../../src/parser' @@ -9,6 +9,7 @@ import {URL} from 'url' import * as sinon from 'sinon' import {CLIError} from '../../src/errors' +config.truncateThreshold = 0 const stripAnsi = require('strip-ansi') describe('parse', () => { @@ -145,8 +146,8 @@ describe('parse', () => { message = stripAnsi(error.message) } - expect(message).to.equal( - 'The following error occurred:\n Missing required flag myflag\nSee more help with --help', + expect(message).to.include( + 'Missing required flag myflag', ) }) @@ -174,7 +175,7 @@ describe('parse', () => { message = error.message } - expect(message).to.equal(`Missing 2 required args: + expect(message).to.include(`Missing 2 required args: arg2 arg2 desc arg3 arg3 desc See more help with --help`) @@ -192,7 +193,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Unexpected argument: arg2\nSee more help with --help') + expect(message).to.include('Unexpected argument: arg2') }) it('parses args', async () => { @@ -258,7 +259,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal(`Missing 1 required arg: + expect(message).to.include(`Missing 1 required arg: arg1 See more help with --help`) }) @@ -300,7 +301,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal(`Invalid argument spec: + expect(message).to.include(`Invalid argument spec: arg1 (optional) arg2 (required) See more help with --help`) @@ -321,7 +322,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal(`Invalid argument spec: + expect(message).to.include(`Invalid argument spec: arg1 (optional) arg2 (optional) arg3 (optional) @@ -351,6 +352,91 @@ See more help with --help`) }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) }) + it('allowed options on multiple', async () => { + const out = await parse(['--foo', 'a', '--foo=b'], { + flags: { + foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a', 'b']}) + }) + + it('one of allowed options on multiple', async () => { + const out = await parse(['--foo', 'a'], { + flags: { + foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a']}) + }) + it('throws if non-allowed options on multiple', async () => { + try { + await parse(['--foo', 'a', '--foo=c'], { + flags: { + foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + }, + }) + } catch (error:any) { + expect(error.message).to.include('Expected --foo=c to be one of: a, b') + } + }) + describe('comma delimiter', () => { + it('basic', async () => { + const out = await parse(['--foo', 'a,b'], { + flags: { + foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a', 'b']}) + }) + it('with spaces inside quotes', async () => { + const out = await parse(['--foo', '"a a","b b"'], { + flags: { + foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) + }) + it('with options', async () => { + const out = await parse(['--foo', 'a,b'], { + flags: { + foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a', 'b']})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a', 'b']}) + }) + it('throws if non-allowed options on multiple', async () => { + try { + await parse(['--foo', 'a,c'], { + flags: { + foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + }, + }) + } catch (error:any) { + expect(error.message).to.include('Expected --foo=a,c to be one of: a, b') + } + }) + + it('with options and quotes with spaces', async () => { + const out = await parse(['--foo', '"a a","b b"'], { + flags: { + foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a a', 'b b']})(), + }, + }) + expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) + }) + it('throws if non-allowed comma as delimiter with options and quotes with spaces', async () => { + try { + await parse(['--foo', '"a a","b c"'], { + flags: { + foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a a', 'b b']})(), + }, + }) + } catch (error:any) { + expect(error.message).to.include('Expected --foo=b c to be one of: a a, b b') + } + }) + }) }) describe('strict: false', () => { @@ -422,7 +508,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --int \n\tExpected an integer but received: 3.14\nSee more help with --help') + expect(message).to.include('Parsing --int \n\tExpected an integer but received: 3.14') }) it('does not parse fractions', async () => { @@ -435,7 +521,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --int \n\tExpected an integer but received: 3/4\nSee more help with --help') + expect(message).to.include('Parsing --int \n\tExpected an integer but received: 3/4') }) it('does not parse strings', async () => { @@ -448,7 +534,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --int \n\tExpected an integer but received: s10\nSee more help with --help') + expect(message).to.include('Parsing --int \n\tExpected an integer but received: s10') }) describe('min/max', () => { @@ -487,7 +573,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --int \n\tExpected an integer greater than or equal to 10 but received: 9\nSee more help with --help') + expect(message).to.include('Parsing --int \n\tExpected an integer greater than or equal to 10 but received: 9') }) it('max fail gt', async () => { let message = '' @@ -499,7 +585,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --int \n\tExpected an integer less than or equal to 20 but received: 21\nSee more help with --help') + expect(message).to.include('Parsing --int \n\tExpected an integer less than or equal to 20 but received: 21') }) }) }) @@ -524,8 +610,8 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal( - `Parsing --int \n\t${customParseException}\nSee more help with --help`) + expect(error.message).to.include( + `Parsing --int \n\t${customParseException}`) } }) }) @@ -821,7 +907,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Expected --foo=invalidopt to be one of: myopt, myotheropt\nSee more help with --help') + expect(message).to.include('Expected --foo=invalidopt to be one of: myopt, myotheropt') }) it('fails when invalid env var', async () => { let message = '' @@ -834,7 +920,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Expected --foo=invalidopt to be one of: myopt, myotheropt\nSee more help with --help') + expect(message).to.include('Expected --foo=invalidopt to be one of: myopt, myotheropt') }) it('accepts valid option env var', async () => { @@ -866,7 +952,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Parsing --foo \n\tExpected a valid url but received: example\nSee more help with --help') + expect(message).to.include('Parsing --foo \n\tExpected a valid url but received: example') }) }) @@ -888,7 +974,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('Expected invalidopt to be one of: myopt, myotheropt\nSee more help with --help') + expect(message).to.include('Expected invalidopt to be one of: myopt, myotheropt') }) }) @@ -1011,7 +1097,7 @@ See more help with --help`) message = error.message } - 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') + expect(message).to.include('All of the following must be provided when using --foo: --bar') }) }) @@ -1048,7 +1134,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('The following error occurred:\n --bar=b cannot also be provided when using --foo\nSee more help with --help') + expect(message).to.include('--bar=b cannot also be provided when using --foo') }) }) @@ -1066,7 +1152,7 @@ See more help with --help`) message = error.message } - expect(message).to.equal('The following error occurred:\n Exactly one of the following must be provided: --bar, --foo\nSee more help with --help') + expect(message).to.include('Exactly one of the following must be provided: --bar, --foo') }) it('throws if multiple are set', async () => { @@ -1082,7 +1168,9 @@ See more help with --help`) message = error.message } - 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') + expect(message).to.include('The following errors occurred:') + expect(message).to.include('--bar cannot also be provided when using --foo') + expect(message).to.include('--foo cannot also be provided when using --bar') }) it('succeeds if exactly one', async () => { @@ -1143,7 +1231,9 @@ See more help with --help`) message = error.message } - 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') + expect(message).to.include('The following errors occurred:') + expect(message).to.include('--else cannot also be provided when using --foo') + expect(message).to.include('--foo cannot also be provided when using --else') }) it('handles cross-references/pairings that don\'t make sense', async () => { @@ -1161,7 +1251,7 @@ See more help with --help`) message1 = error.message } - expect(message1).to.equal('The following error occurred:\n --bar cannot also be provided when using --foo\nSee more help with --help') + expect(message1).to.include('--bar cannot also be provided when using --foo') let message2 = '' try { @@ -1172,7 +1262,7 @@ See more help with --help`) message2 = error.message } - expect(message2).to.equal('The following error occurred:\n --else cannot also be provided when using --bar\nSee more help with --help') + expect(message2).to.include('--else cannot also be provided when using --bar') const out = await parse(['--foo', 'a', '--else', '4'], { flags: crazyFlags, @@ -1257,8 +1347,8 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal( - `Parsing --dir \n\tNo directory found at ${testDir}\nSee more help with --help`, + expect(error.message).to.include( + `Parsing --dir \n\tNo directory found at ${testDir}`, ) } }) @@ -1272,8 +1362,8 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal( - `Parsing --dir \n\t${testDir} exists but is not a directory\nSee more help with --help`) + expect(error.message).to.include( + `Parsing --dir \n\t${testDir} exists but is not a directory`) } }) describe('custom parse functions', () => { @@ -1297,8 +1387,8 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal( - `Parsing --dir \n\t${customParseException}\nSee more help with --help`) + expect(error.message).to.include( + `Parsing --dir \n\t${customParseException}`) } }) }) @@ -1337,7 +1427,7 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal(`Parsing --file \n\tNo file found at ${testFile}\nSee more help with --help`) + expect(error.message).to.include(`Parsing --file \n\tNo file found at ${testFile}`) } }) it('fails when file exists but is not a file', async () => { @@ -1350,7 +1440,7 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal(`Parsing --file \n\t${testFile} exists but is not a file\nSee more help with --help`) + expect(error.message).to.include(`Parsing --file \n\t${testFile} exists but is not a file`) } }) describe('custom parse functions', () => { @@ -1374,8 +1464,8 @@ See more help with --help`) throw new Error(`Should have thrown an error ${JSON.stringify(out)}`) } catch (error_) { const error = error_ as Error - expect(error.message).to.equal( - `Parsing --dir \n\t${customParseException}\nSee more help with --help`) + expect(error.message).to.include( + `Parsing --dir \n\t${customParseException}`) } }) }) From dcf796b273d61e5c636e181d32cce4ac4cf01210 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 7 Feb 2023 11:02:29 -0600 Subject: [PATCH 2/5] chore: comment typo --- src/parser/parse.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/parse.ts b/src/parser/parse.ts index fcd769a18..578924e8b 100644 --- a/src/parser/parse.ts +++ b/src/parser/parse.ts @@ -233,7 +233,7 @@ export class Parser this._parseFlag(v.trim().replace(/^"(.*)"$/, '$1'), flag, token)), ) - // the parse that each element aligns with the `options` property + // then parse that each element aligns with the `options` property for (const v of values) { this._validateOptions(flag, v) } From 3864220cd47786f8cdc0670615a4d4950e0b1802 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 7 Feb 2023 11:12:27 -0600 Subject: [PATCH 3/5] refactor: string flags --- test/parser/parse.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index 2b3ada119..58531c7d1 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -355,7 +355,7 @@ See more help with --help`) it('allowed options on multiple', async () => { const out = await parse(['--foo', 'a', '--foo=b'], { flags: { - foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + foo: Flags.string({multiple: true, parse: async i => i, options: ['a', 'b']}), }, }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) @@ -364,7 +364,7 @@ See more help with --help`) it('one of allowed options on multiple', async () => { const out = await parse(['--foo', 'a'], { flags: { - foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + foo: Flags.string({multiple: true, options: ['a', 'b']}), }, }) expect(out.flags).to.deep.include({foo: ['a']}) @@ -373,7 +373,7 @@ See more help with --help`) try { await parse(['--foo', 'a', '--foo=c'], { flags: { - foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + foo: Flags.string({multiple: true, options: ['a', 'b']}), }, }) } catch (error:any) { @@ -384,7 +384,7 @@ See more help with --help`) it('basic', async () => { const out = await parse(['--foo', 'a,b'], { flags: { - foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i})(), + foo: Flags.string({multiple: true, delimiter: ','}), }, }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) @@ -392,7 +392,7 @@ See more help with --help`) it('with spaces inside quotes', async () => { const out = await parse(['--foo', '"a a","b b"'], { flags: { - foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i})(), + foo: Flags.string({multiple: true, delimiter: ','}), }, }) expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) @@ -400,7 +400,7 @@ See more help with --help`) it('with options', async () => { const out = await parse(['--foo', 'a,b'], { flags: { - foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a', 'b']})(), + foo: Flags.string({multiple: true, delimiter: ',', options: ['a', 'b']}), }, }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) @@ -409,7 +409,7 @@ See more help with --help`) try { await parse(['--foo', 'a,c'], { flags: { - foo: Flags.custom({multiple: true, parse: async i => i, options: ['a', 'b']})(), + foo: Flags.string({multiple: true, options: ['a', 'b']}), }, }) } catch (error:any) { @@ -420,7 +420,7 @@ See more help with --help`) it('with options and quotes with spaces', async () => { const out = await parse(['--foo', '"a a","b b"'], { flags: { - foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a a', 'b b']})(), + foo: Flags.string({multiple: true, delimiter: ',', options: ['a a', 'b b']}), }, }) expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) @@ -429,7 +429,7 @@ See more help with --help`) try { await parse(['--foo', '"a a","b c"'], { flags: { - foo: Flags.custom({multiple: true, delimiter: ',', parse: async i => i, options: ['a a', 'b b']})(), + foo: Flags.string({multiple: true, delimiter: ',', options: ['a a', 'b b']}), }, }) } catch (error:any) { From bd22be1802da6bef61a770996cd6cd28f00c51d3 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 7 Feb 2023 11:20:56 -0600 Subject: [PATCH 4/5] refactor: single-quotes work, too. --- src/parser/parse.ts | 2 +- test/parser/parse.test.ts | 33 ++++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/parser/parse.ts b/src/parser/parse.ts index 578924e8b..72fbd4f1d 100644 --- a/src/parser/parse.ts +++ b/src/parser/parse.ts @@ -231,7 +231,7 @@ export class Parser this._parseFlag(v.trim().replace(/^"(.*)"$/, '$1'), flag, token)), + input.split(flag.delimiter).map(async v => this._parseFlag(v.trim().replace(/^"(.*)"$/, '$1').replace(/^'(.*)'$/, '$1'), flag, token)), ) // then parse that each element aligns with the `options` property for (const v of values) { diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index 58531c7d1..958df099d 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -389,7 +389,7 @@ See more help with --help`) }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) }) - it('with spaces inside quotes', async () => { + it('with spaces inside double quotes', async () => { const out = await parse(['--foo', '"a a","b b"'], { flags: { foo: Flags.string({multiple: true, delimiter: ','}), @@ -397,6 +397,14 @@ See more help with --help`) }) expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) }) + it('with spaces inside single quotes', async () => { + const out = await parse(['--foo', "'a a','b b'"], { + flags: { + foo: Flags.string({multiple: true, delimiter: ','}), + }, + }) + expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) + }) it('with options', async () => { const out = await parse(['--foo', 'a,b'], { flags: { @@ -417,7 +425,7 @@ See more help with --help`) } }) - it('with options and quotes with spaces', async () => { + it('with options and double quotes with spaces', async () => { const out = await parse(['--foo', '"a a","b b"'], { flags: { foo: Flags.string({multiple: true, delimiter: ',', options: ['a a', 'b b']}), @@ -425,7 +433,15 @@ See more help with --help`) }) expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) }) - it('throws if non-allowed comma as delimiter with options and quotes with spaces', async () => { + it('with options and single quotes with spaces', async () => { + const out = await parse(['--foo', "'a a','b b'"], { + flags: { + foo: Flags.string({multiple: true, delimiter: ',', options: ['a a', 'b b']}), + }, + }) + expect(out.flags).to.deep.include({foo: ['a a', 'b b']}) + }) + it('throws if non-allowed with options and double quotes with spaces', async () => { try { await parse(['--foo', '"a a","b c"'], { flags: { @@ -436,6 +452,17 @@ See more help with --help`) expect(error.message).to.include('Expected --foo=b c to be one of: a a, b b') } }) + it('throws if non-allowed with options and single quotes with spaces', async () => { + try { + await parse(['--foo', "'a a','b c'"], { + flags: { + foo: Flags.string({multiple: true, delimiter: ',', options: ['a a', 'b b']}), + }, + }) + } catch (error:any) { + expect(error.message).to.include('Expected --foo=b c to be one of: a a, b b') + } + }) }) }) From 5cf029c8f4bc6a5f899bebc35c5c4c676c92832c Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 7 Feb 2023 11:24:53 -0600 Subject: [PATCH 5/5] test: interior quote preservation --- test/parser/parse.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index 958df099d..c5d34f9f2 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -389,6 +389,22 @@ See more help with --help`) }) expect(out.flags).to.deep.include({foo: ['a', 'b']}) }) + it('preserves non-exterior double quotes (single and pairs)', async () => { + const out = await parse(['--foo', 'a,",b,hi"yo"'], { + flags: { + foo: Flags.string({multiple: true, delimiter: ','}), + }, + }) + expect(out.flags).to.deep.include({foo: ['a', '"', 'b', 'hi"yo"']}) + }) + it('preserves non-exterior single quotes (single and pairs)', async () => { + const out = await parse(['--foo', "a,',b,hi'yo'"], { + flags: { + foo: Flags.string({multiple: true, delimiter: ','}), + }, + }) + expect(out.flags).to.deep.include({foo: ['a', "'", 'b', "hi'yo'"]}) + }) it('with spaces inside double quotes', async () => { const out = await parse(['--foo', '"a a","b b"'], { flags: {