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

fix: strict should fail unknown arguments #1977

Merged
merged 3 commits into from Jul 11, 2021
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
28 changes: 24 additions & 4 deletions lib/validation.ts
Expand Up @@ -154,7 +154,7 @@ export function validation(

Object.keys(argv).forEach(key => {
if (
specialKeys.indexOf(key) === -1 &&
!specialKeys.includes(key) &&
!Object.prototype.hasOwnProperty.call(positionalMap, key) &&
!Object.prototype.hasOwnProperty.call(
yargs.getInternalMethods().getParseContext(),
Expand All @@ -173,13 +173,33 @@ export function validation(
isDefaultCommand)
) {
argv._.slice(currentContext.commands.length).forEach(key => {
if (commandKeys.indexOf('' + key) === -1) {
if (!commandKeys.includes('' + key)) {
unknown.push('' + key);
}
});
}

if (unknown.length > 0) {
// https://github.com/yargs/yargs/issues/1861
if (checkPositionals) {
// Check for non-option args that are not in currentContext.commands
// Take into account expected args from commands and yargs.demand(number)
const demandedCommands = yargs.getDemandedCommands();
const maxNonOptDemanded = demandedCommands._?.max || 0;
const expected = currentContext.commands.length + maxNonOptDemanded;
if (expected < argv._.length) {
argv._.slice(expected).forEach(key => {
key = String(key);
if (
!currentContext.commands.includes(key) &&
!unknown.includes(key)
) {
unknown.push(key);
}
});
}
}

if (unknown.length) {
usage.fail(
__n(
'Unknown argument: %s',
Expand All @@ -201,7 +221,7 @@ export function validation(

if (currentContext.commands.length > 0 || commandKeys.length > 0) {
argv._.slice(currentContext.commands.length).forEach(key => {
if (commandKeys.indexOf('' + key) === -1) {
if (!commandKeys.includes('' + key)) {
unknown.push('' + key);
}
});
Expand Down
37 changes: 35 additions & 2 deletions test/validation.cjs
Expand Up @@ -330,6 +330,29 @@ describe('validation tests', () => {
expect.fail('no parsing failure');
});

// addresses: https://github.com/yargs/yargs/issues/1861
it('fails in strict mode when no commands defined but command is passed', done => {
yargs
.strict()
.fail(msg => {
msg.should.equal('Unknown argument: foo');
done();
})
.parse('foo');
expect.fail('no parsing failure');
});

it('fails because of undefined command and not because of argument after --', done => {
yargs
.strict()
.fail(msg => {
msg.should.equal('Unknown argument: foo');
done();
})
.parse('foo -- hello');
expect.fail('no parsing failure');
});

it('fails in strict mode with invalid command', done => {
yargs(['koala'])
.command('wombat', 'wombat burrows')
Expand Down Expand Up @@ -916,7 +939,7 @@ describe('validation tests', () => {
});

it('does not fail for hidden options', () => {
const args = yargs('--foo hey')
const args = yargs('--foo')
.strict()
.option('foo', {boolean: true, describe: false})
.fail(msg => {
Expand All @@ -926,8 +949,18 @@ describe('validation tests', () => {
args.foo.should.equal(true);
});

it('does not fail for hidden options but does for unknown arguments', () => {
const args = yargs('--foo hey')
.strict()
.option('foo', {boolean: true, describe: false})
.fail(msg => {
msg.should.equal('Unknown argument: hey');
})
.parse();
});

it('does not fail if an alias is provided, rather than option itself', () => {
const args = yargs('--cat hey')
const args = yargs('--cat')
.strict()
.option('foo', {boolean: true, describe: false})
.alias('foo', 'bar')
Expand Down