diff --git a/lib/argument.js b/lib/argument.js index a3c7773ed..c16430250 100644 --- a/lib/argument.js +++ b/lib/argument.js @@ -97,10 +97,10 @@ class Argument { */ choices(values) { - this.argChoices = values; + this.argChoices = values.slice(); this.parseArg = (arg, previous) => { - if (!values.includes(arg)) { - throw new InvalidArgumentError(`Allowed choices are ${values.join(', ')}.`); + if (!this.argChoices.includes(arg)) { + throw new InvalidArgumentError(`Allowed choices are ${this.argChoices.join(', ')}.`); } if (this.variadic) { return this._concatValue(arg, previous); diff --git a/lib/option.js b/lib/option.js index b1c1a57e8..c68110e4a 100644 --- a/lib/option.js +++ b/lib/option.js @@ -135,10 +135,10 @@ class Option { */ choices(values) { - this.argChoices = values; + this.argChoices = values.slice(); this.parseArg = (arg, previous) => { - if (!values.includes(arg)) { - throw new InvalidArgumentError(`Allowed choices are ${values.join(', ')}.`); + if (!this.argChoices.includes(arg)) { + throw new InvalidArgumentError(`Allowed choices are ${this.argChoices.join(', ')}.`); } if (this.variadic) { return this._concatValue(arg, previous); diff --git a/tests/argument.choices.test.js b/tests/argument.choices.test.js new file mode 100644 index 000000000..fe63ea73b --- /dev/null +++ b/tests/argument.choices.test.js @@ -0,0 +1,59 @@ +const commander = require('../'); + +test('when command argument in choices then argument set', () => { + const program = new commander.Command(); + let shade; + program + .exitOverride() + .addArgument(new commander.Argument('').choices(['red', 'blue'])) + .action((shadeParam) => { shade = shadeParam; }); + program.parse(['red'], { from: 'user' }); + expect(shade).toBe('red'); +}); + +test('when command argument is not in choices then error', () => { + // Lightweight check, more detailed testing of behaviour in command.exitOverride.test.js + const program = new commander.Command(); + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .addArgument(new commander.Argument('').choices(['red', 'blue'])); + expect(() => { + program.parse(['orange'], { from: 'user' }); + }).toThrow(); +}); + +describe('choices parameter is treated as readonly, per TypeScript declaration', () => { + test('when choices called then parameter does not change', () => { + // Unlikely this could break, but check the API we are declaring in TypeScript. + const original = ['red', 'blue', 'green']; + const param = original.slice(); + new commander.Argument('').choices(param); + expect(param).toEqual(original); + }); + + test('when choices called and argChoices later changed then parameter does not change', () => { + const original = ['red', 'blue', 'green']; + const param = original.slice(); + const argument = new commander.Argument('').choices(param); + argument.argChoices.push('purple'); + expect(param).toEqual(original); + }); + + test('when choices called and parameter changed the choices does not change', () => { + const program = new commander.Command(); + const param = ['red', 'blue']; + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .addArgument(new commander.Argument('').choices(param)); + param.push('orange'); + expect(() => { + program.parse(['orange'], { from: 'user' }); + }).toThrow(); + }); +}); diff --git a/tests/argument.custom-processing.test.js b/tests/argument.custom-processing.test.js index ffea759a8..ef3d16822 100644 --- a/tests/argument.custom-processing.test.js +++ b/tests/argument.custom-processing.test.js @@ -205,15 +205,3 @@ test('when custom processing for argument throws plain error then not CommanderE expect(caughtErr).toBeInstanceOf(Error); expect(caughtErr).not.toBeInstanceOf(commander.CommanderError); }); - -// this is the happy path, testing failure case in command.exitOverride.test.js -test('when argument argument in choices then argument set', () => { - const program = new commander.Command(); - let shade; - program - .exitOverride() - .addArgument(new commander.Argument('').choices(['red', 'blue'])) - .action((shadeParam) => { shade = shadeParam; }); - program.parse(['red'], { from: 'user' }); - expect(shade).toBe('red'); -}); diff --git a/tests/options.choices.test.js b/tests/options.choices.test.js new file mode 100644 index 000000000..83020449a --- /dev/null +++ b/tests/options.choices.test.js @@ -0,0 +1,57 @@ +const commander = require('../'); + +test('when option argument in choices then option set', () => { + const program = new commander.Command(); + program + .exitOverride() + .addOption(new commander.Option('--colour ').choices(['red', 'blue'])); + program.parse(['--colour', 'red'], { from: 'user' }); + expect(program.opts().colour).toBe('red'); +}); + +test('when option argument is not in choices then error', () => { + // Lightweight check, more detailed testing of behaviour in command.exitOverride.test.js + const program = new commander.Command(); + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .addOption(new commander.Option('--colour ').choices(['red', 'blue'])); + expect(() => { + program.parse(['--colour', 'orange'], { from: 'user' }); + }).toThrow(); +}); + +describe('choices parameter is treated as readonly, per TypeScript declaration', () => { + test('when choices called then parameter does not change', () => { + // Unlikely this could break, but check the API we are declaring in TypeScript. + const original = ['red', 'blue', 'green']; + const param = original.slice(); + new commander.Option('--colour ').choices(param); + expect(param).toEqual(original); + }); + + test('when choices called and argChoices later changed then parameter does not change', () => { + const original = ['red', 'blue', 'green']; + const param = original.slice(); + const option = new commander.Option('--colour ').choices(param); + option.argChoices.push('purple'); + expect(param).toEqual(original); + }); + + test('when choices called and parameter changed the choices does not change', () => { + const program = new commander.Command(); + const param = ['red', 'blue']; + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .addOption(new commander.Option('--colour ').choices(param)); + param.push('orange'); + expect(() => { + program.parse(['--colour', 'orange'], { from: 'user' }); + }).toThrow(); + }); +}); diff --git a/tests/options.custom-processing.test.js b/tests/options.custom-processing.test.js index fb48c1661..aa1022fbb 100644 --- a/tests/options.custom-processing.test.js +++ b/tests/options.custom-processing.test.js @@ -98,16 +98,6 @@ test('when option specified multiple times then callback called with value and p expect(mockCoercion).toHaveBeenNthCalledWith(2, '2', 'callback'); }); -// this is the happy path, testing failure case in command.exitOverride.test.js -test('when option argument in choices then option set', () => { - const program = new commander.Command(); - program - .exitOverride() - .addOption(new commander.Option('--colour ').choices(['red', 'blue'])); - program.parse(['--colour', 'red'], { from: 'user' }); - expect(program.opts().colour).toBe('red'); -}); - // Now some functional tests like the examples in the README! test('when parseFloat "1e2" then value is 100', () => { diff --git a/typings/index.d.ts b/typings/index.d.ts index d9fbfa2ea..b2a700502 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -61,7 +61,7 @@ export class Argument { /** * Only allow argument value to be one of choices. */ - choices(values: string[]): this; + choices(values: readonly string[]): this; /** * Make argument required. @@ -140,7 +140,7 @@ export class Option { /** * Only allow option value to be one of choices. */ - choices(values: string[]): this; + choices(values: readonly string[]): this; /** * Return option name. diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 3fcc62f23..09d1c085c 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -394,6 +394,7 @@ expectType(baseOption.hideHelp(false)); // choices expectType(baseOption.choices(['a', 'b'])); +expectType(baseOption.choices(['a', 'b'] as const)); // name expectType(baseOption.name()); @@ -425,6 +426,7 @@ expectType(baseArgument.argParser((value: string, previous: // choices expectType(baseArgument.choices(['a', 'b'])); +expectType(baseArgument.choices(['a', 'b'] as const)); // argRequired expectType(baseArgument.argRequired());