diff --git a/lib/option.js b/lib/option.js index bded840ae..172448539 100644 --- a/lib/option.js +++ b/lib/option.js @@ -312,17 +312,32 @@ function camelcase(str) { function splitOptionFlags(flags) { let shortFlag; let longFlag; - // Use original very loose parsing to maintain backwards compatibility for now, - // which allowed for example unintended `-sw, --short-word` [sic]. - const flagParts = flags.split(/[ |,]+/); - if (flagParts.length > 1 && !/^[[<]/.test(flagParts[1])) - shortFlag = flagParts.shift(); - longFlag = flagParts.shift(); - // Add support for lone short flag without significantly changing parsing! - if (!shortFlag && /^-[^-]$/.test(longFlag)) { - shortFlag = longFlag; - longFlag = undefined; - } + // short flag, single dash and single character + const shortFlagExp = /^-[^-]$/; + // long flag, double dash and at least one character + const longFlagExp = /^--[^-]/; + + const flagParts = flags.split(/[ |,]+/).concat('guard'); + if (shortFlagExp.test(flagParts[0])) shortFlag = flagParts.shift(); + if (longFlagExp.test(flagParts[0])) longFlag = flagParts.shift(); + + // Check for some unsupported flags that people try. + if (/^-[^-][^-]/.test(flagParts[0])) + throw new Error( + `invalid Option flags, short option is dash and single character: '${flags}'`, + ); + if (shortFlag && shortFlagExp.test(flagParts[0])) + throw new Error( + `invalid Option flags, more than one short flag: '${flags}'`, + ); + if (longFlag && longFlagExp.test(flagParts[0])) + throw new Error( + `invalid Option flags, more than one long flag: '${flags}'`, + ); + // Generic error if failed to find a flag or an unexpected flag left over. + if (!(shortFlag || longFlag) || flagParts[0].startsWith('-')) + throw new Error(`invalid Option flags: '${flags}'`); + return { shortFlag, longFlag }; } diff --git a/tests/option.bad-flags.test.js b/tests/option.bad-flags.test.js new file mode 100644 index 000000000..814de52a4 --- /dev/null +++ b/tests/option.bad-flags.test.js @@ -0,0 +1,34 @@ +const { Option } = require('../'); + +// Check that unsupported flags throw. +test.each([ + { flags: '-a, -b' }, // too many short flags + { flags: '-a, -b ' }, + { flags: '-a, -b, --long' }, + { flags: '--one, --two' }, // too many long flags + { flags: '--one, --two [value]' }, + { flags: '-ws' }, // short flag with more than one character + { flags: 'sdkjhskjh' }, // oops, no flags + { flags: '-a,-b' }, // try all the separators + { flags: '-a|-b' }, + { flags: '-a -b' }, +])('when construct Option with flags %p then throw', ({ flags }) => { + expect(() => { + new Option(flags); + }).toThrow(/^invalid Option flags/); +}); + +// Check that supported flags do not throw. +test.each([ + { flags: '-s' }, // single short + { flags: '--long' }, // single long + { flags: '-b, --both' }, // short and long + { flags: '-b,--both ' }, + { flags: '-b|--both ' }, + { flags: '-b --both [space]' }, + { flags: '-v, --variadic ' }, +])('when construct Option with flags %p then do not throw', ({ flags }) => { + expect(() => { + new Option(flags); + }).not.toThrow(); +}); diff --git a/tests/options.registerClash.test.js b/tests/options.registerClash.test.js index 0e93ee895..42d4460e9 100644 --- a/tests/options.registerClash.test.js +++ b/tests/options.registerClash.test.js @@ -29,8 +29,8 @@ describe('.option()', () => { test('when reuse flags in subcommand then does not throw', () => { expect(() => { const program = new Command(); - program.option('e, --example'); - program.command('sub').option('e, --example'); + program.option('-e, --example'); + program.command('sub').option('-e, --example'); }).not.toThrow(); }); });