Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add .allowExcessArguments() and error message #1407

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 52 additions & 18 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,24 +404,24 @@ class Option {
/**
* Whether the option is mandatory and must have a value after parsing.
*
* @param {boolean} [mandatory]
* @param {boolean} [mandatory=true]
* @return {Option}
*/

makeOptionMandatory(mandatory) {
this.mandatory = (mandatory === undefined) || !!mandatory;
makeOptionMandatory(mandatory = true) {
this.mandatory = !!mandatory;
return this;
};

/**
* Hide option in help.
*
* @param {boolean} [hide]
* @param {boolean} [hide=true]
* @return {Option}
*/

hideHelp(hide) {
this.hidden = (hide === undefined) || !!hide;
hideHelp(hide = true) {
this.hidden = !!hide;
return this;
};

Expand Down Expand Up @@ -535,6 +535,7 @@ class Command extends EventEmitter {
this.options = [];
this.parent = null;
this._allowUnknownOption = false;
this._allowExcessArguments = true;
this._args = [];
this.rawArgs = null;
this._scriptPath = null;
Expand Down Expand Up @@ -1152,35 +1153,46 @@ Read more on https://git.io/JJc0W`);
* .combineFlagAndOptionalValue(true) // `-f80` is treated like `--flag=80`, this is the default behaviour
* .combineFlagAndOptionalValue(false) // `-fb` is treated like `-f -b`
*
* @param {Boolean} [combine] - if `true` or omitted, an optional value can be specified directly after the flag.
* @param {Boolean} [combine=true] - if `true` or omitted, an optional value can be specified directly after the flag.
*/
combineFlagAndOptionalValue(combine) {
this._combineFlagAndOptionalValue = (combine === undefined) || !!combine;
combineFlagAndOptionalValue(combine = true) {
this._combineFlagAndOptionalValue = !!combine;
return this;
};

/**
* Allow unknown options on the command line.
*
* @param {Boolean} [allowUnknown] - if `true` or omitted, no error will be thrown
* @param {Boolean} [allowUnknown=true] - if `true` or omitted, no error will be thrown
* for unknown options.
*/
allowUnknownOption(allowUnknown) {
this._allowUnknownOption = (allowUnknown === undefined) || !!allowUnknown;
allowUnknownOption(allowUnknown = true) {
this._allowUnknownOption = !!allowUnknown;
return this;
};

/**
* Allow excess arguments on the command line.
*
* @param {Boolean} [allowExcess=true] - if `true` or omitted, no error will be thrown
* for excess arguments.
*/
allowExcessArguments(allowExcess = true) {
this._allowExcessArguments = !!allowExcess;
return this;
};

/**
* Whether to store option values as properties on command object,
* or store separately (specify false). In both cases the option values can be accessed using .opts().
*
* @param {boolean} storeAsProperties
* @param {boolean} [storeAsProperties=true]
* @return {Command} `this` command for chaining
*/

storeOptionsAsProperties(storeAsProperties) {
storeOptionsAsProperties(storeAsProperties = true) {
this._storeOptionsAsPropertiesCalled = true;
this._storeOptionsAsProperties = (storeAsProperties === undefined) || !!storeAsProperties;
this._storeOptionsAsProperties = !!storeAsProperties;
if (this.options.length) {
throw new Error('call .storeOptionsAsProperties() before adding options');
}
Expand All @@ -1191,12 +1203,12 @@ Read more on https://git.io/JJc0W`);
* Whether to pass command to action handler,
* or just the options (specify false).
*
* @param {boolean} passCommand
* @param {boolean} [passCommand=true]
* @return {Command} `this` command for chaining
*/

passCommandToAction(passCommand) {
this._passCommandToAction = (passCommand === undefined) || !!passCommand;
passCommandToAction(passCommand = true) {
this._passCommandToAction = !!passCommand;
return this;
};

Expand Down Expand Up @@ -1492,14 +1504,19 @@ Read more on https://git.io/JJc0W`);

const commandEvent = `command:${this.name()}`;
if (this._actionHandler) {
// Check expected arguments and collect variadic together.
const args = this.args.slice();
this._args.forEach((arg, i) => {
if (arg.required && args[i] == null) {
this.missingArgument(arg.name);
} else if (arg.variadic) {
args[i] = args.splice(i);
args.length = Math.min(i + 1, args.length);
}
});
if (args.length > this._args.length) {
this._excessArguments(args);
}

this._actionHandler(args);
if (this.parent) this.parent.emit(commandEvent, operands, unknown); // legacy
Expand Down Expand Up @@ -1753,6 +1770,23 @@ Read more on https://git.io/JJc0W`);
this._displayError(1, 'commander.unknownOption', message);
};

/**
* Excess arguments, more than expected.
*
* @param {string[]} receivedArgs
* @api private
*/

_excessArguments(receivedArgs) {
if (this._allowExcessArguments) return;

const expected = this._args.length;
const s = (expected === 1) ? '' : 's';
const forSubcommand = this.parent ? ` for '${this.name()}'` : '';
const message = `error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.`;
this._displayError(1, 'commander.excessArguments', message);
};

/**
* Unknown command.
*
Expand Down
145 changes: 145 additions & 0 deletions tests/command.allowExcessArguments.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
const commander = require('../');

// Not testing output, just testing whether an error is detected.

describe('allowUnknownOption', () => {
// Optional. Use internal knowledge to suppress output to keep test output clean.
let writeErrorSpy;

beforeAll(() => {
writeErrorSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => { });
});

afterEach(() => {
writeErrorSpy.mockClear();
});

afterAll(() => {
writeErrorSpy.mockRestore();
});

test('when specify excess program argument then ignored by default', () => {
const program = new commander.Command();
program
.exitOverride()
.action(() => {});

expect(() => {
program.parse(['excess'], { from: 'user' });
}).not.toThrow();
});

test('when specify excess program argument and allowExcessArguments(false) then error', () => {
const program = new commander.Command();
program
.exitOverride()
.allowExcessArguments(false)
.action(() => {});

expect(() => {
program.parse(['excess'], { from: 'user' });
}).toThrow();
});

test('when specify excess program argument and allowExcessArguments() then no error', () => {
const program = new commander.Command();
program
.exitOverride()
.allowExcessArguments()
.action(() => {});

expect(() => {
program.parse(['excess'], { from: 'user' });
}).not.toThrow();
});

test('when specify excess program argument and allowExcessArguments(true) then no error', () => {
const program = new commander.Command();
program
.exitOverride()
.allowExcessArguments(true)
.action(() => {});

expect(() => {
program.parse(['excess'], { from: 'user' });
}).not.toThrow();
});

test('when specify excess command argument then no error (by default)', () => {
const program = new commander.Command();
program
.exitOverride()
.command('sub')
.action(() => { });

expect(() => {
program.parse(['sub', 'excess'], { from: 'user' });
}).not.toThrow();
});

test('when specify excess command argument and allowExcessArguments(false) then error', () => {
const program = new commander.Command();
program
.exitOverride()
.command('sub')
.allowUnknownOption()
.allowExcessArguments(false)
.action(() => { });

expect(() => {
program.parse(['sub', 'excess'], { from: 'user' });
}).toThrow();
});

test('when specify expected arg and allowExcessArguments(false) then no error', () => {
const program = new commander.Command();
program
.arguments('<file>')
.exitOverride()
.allowExcessArguments(false)
.action(() => {});

expect(() => {
program.parse(['file'], { from: 'user' });
}).not.toThrow();
});

test('when specify excess after <arg> and allowExcessArguments(false) then error', () => {
const program = new commander.Command();
program
.arguments('<file>')
.exitOverride()
.allowExcessArguments(false)
.action(() => {});

expect(() => {
program.parse(['file', 'excess'], { from: 'user' });
}).toThrow();
});

test('when specify excess after [arg] and allowExcessArguments(false) then error', () => {
const program = new commander.Command();
program
.arguments('[file]')
.exitOverride()
.allowExcessArguments(false)
.action(() => {});

expect(() => {
program.parse(['file', 'excess'], { from: 'user' });
}).toThrow();
});

test('when specify args for [args...] and allowExcessArguments(false) then no error', () => {
const program = new commander.Command();
program
.arguments('[files...]')
.exitOverride()
.allowExcessArguments(false)
.action(() => {});

expect(() => {
program.parse(['file1', 'file2', 'file3'], { from: 'user' });
}).not.toThrow();
});
});
6 changes: 6 additions & 0 deletions tests/command.chain.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ describe('Command methods that should return this for chaining', () => {
expect(result).toBe(program);
});

test('when call .allowExcessArguments() then returns this', () => {
const program = new Command();
const result = program.allowExcessArguments();
expect(result).toBe(program);
});

test('when call .storeOptionsAsProperties() then returns this', () => {
const program = new Command();
const result = program.storeOptionsAsProperties();
Expand Down
37 changes: 37 additions & 0 deletions tests/command.exitOverride.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,43 @@ describe('.exitOverride and error details', () => {
expectCommanderError(caughtErr, 1, 'commander.missingArgument', "error: missing required argument 'arg-name'");
});

test('when specify excess argument then throw CommanderError', () => {
const program = new commander.Command();
program
.exitOverride()
.allowExcessArguments(false)
.action(() => { });

let caughtErr;
try {
program.parse(['node', 'test', 'excess']);
} catch (err) {
caughtErr = err;
}

expect(stderrSpy).toHaveBeenCalled();
expectCommanderError(caughtErr, 1, 'commander.excessArguments', 'error: too many arguments. Expected 0 arguments but got 1.');
});

test('when specify command with excess argument then throw CommanderError', () => {
const program = new commander.Command();
program
.exitOverride()
.command('speak')
.allowExcessArguments(false)
.action(() => { });

let caughtErr;
try {
program.parse(['node', 'test', 'speak', 'excess']);
} catch (err) {
caughtErr = err;
}

expect(stderrSpy).toHaveBeenCalled();
expectCommanderError(caughtErr, 1, 'commander.excessArguments', "error: too many arguments for 'speak'. Expected 0 arguments but got 1.");
});

test('when specify --help then throw CommanderError', () => {
const writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => { });
const program = new commander.Command();
Expand Down
4 changes: 4 additions & 0 deletions typings/commander-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ const combineFlagAndOptionalValueThis2: commander.Command = program.combineFlagA
const allowUnknownOptionThis1: commander.Command = program.allowUnknownOption();
const allowUnknownOptionThis2: commander.Command = program.allowUnknownOption(false);

// allowExcessArguments
const allowExcessArgumentsThis1: commander.Command = program.allowExcessArguments();
const allowExcessArgumentsThis2: commander.Command = program.allowExcessArguments(false);

// parse
const parseThis1: commander.Command = program.parse();
const parseThis2: commander.Command = program.parse(process.argv);
Expand Down
7 changes: 7 additions & 0 deletions typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,13 @@ declare namespace commander {
*/
allowUnknownOption(allowUnknown?: boolean): this;

/**
* Allow excess arguments on the command line.
*
* @returns `this` command for chaining
*/
allowExcessArguments(allowExcess?: boolean): this;

/**
* Parse `argv`, setting options and invoking commands when defined.
*
Expand Down