Skip to content

Commit

Permalink
feat(@angular/cli): add warning for overriding flags in arguments
Browse files Browse the repository at this point in the history
Fixes #12948
  • Loading branch information
hansl committed Nov 20, 2018
1 parent f7ac352 commit 3e9705f
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 10 deletions.
34 changes: 24 additions & 10 deletions packages/angular/cli/models/parser.ts
Expand Up @@ -63,7 +63,11 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu
}

case OptionType.Array:
return Array.isArray(v) ? v.concat(str || '') : [str || ''];
return Array.isArray(v)
? v.concat(str || '')
: v === undefined
? [str || '']
: [v + '', str || ''];

default:
return undefined;
Expand Down Expand Up @@ -119,14 +123,14 @@ function _removeLeadingDashes(key: string): string {
function _assignOption(
arg: string,
args: string[],
{ options, parsedOptions, leftovers, ignored, errors, deprecations }: {
{ options, parsedOptions, leftovers, ignored, errors, warnings }: {
options: Option[],
parsedOptions: Arguments,
positionals: string[],
leftovers: string[],
ignored: string[],
errors: string[],
deprecations: string[],
warnings: string[],
},
) {
const from = arg.startsWith('--') ? 2 : 1;
Expand Down Expand Up @@ -188,11 +192,21 @@ function _assignOption(
} else {
const v = _coerce(value, option, parsedOptions[option.name]);
if (v !== undefined) {
parsedOptions[option.name] = v;
if (parsedOptions[option.name] !== v) {
if (parsedOptions[option.name] !== undefined) {
warnings.push(
`Option ${JSON.stringify(option.name)} was already specified with value `
+ `${JSON.stringify(parsedOptions[option.name])}. The new value ${JSON.stringify(v)} `
+ `will override it.`,
);
}

parsedOptions[option.name] = v;

if (option.deprecated !== undefined && option.deprecated !== false) {
deprecations.push(`Option ${JSON.stringify(option.name)} is deprecated${
if (option.deprecated !== undefined && option.deprecated !== false) {
warnings.push(`Option ${JSON.stringify(option.name)} is deprecated${
typeof option.deprecated == 'string' ? ': ' + option.deprecated : ''}.`);
}
}
} else {
let error = `Argument ${key} could not be parsed using value ${JSON.stringify(value)}.`;
Expand Down Expand Up @@ -285,9 +299,9 @@ export function parseArguments(

const ignored: string[] = [];
const errors: string[] = [];
const deprecations: string[] = [];
const warnings: string[] = [];

const state = { options, parsedOptions, positionals, leftovers, ignored, errors, deprecations };
const state = { options, parsedOptions, positionals, leftovers, ignored, errors, warnings };

for (let arg = args.shift(); arg !== undefined; arg = args.shift()) {
if (arg == '--') {
Expand Down Expand Up @@ -369,8 +383,8 @@ export function parseArguments(
parsedOptions['--'] = [...positionals, ...leftovers];
}

if (deprecations.length > 0 && logger) {
deprecations.forEach(message => logger.warn(message));
if (warnings.length > 0 && logger) {
warnings.forEach(message => logger.warn(message));
}

if (errors.length > 0) {
Expand Down
37 changes: 37 additions & 0 deletions packages/angular/cli/models/parser_spec.ts
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*
*/
// tslint:disable:no-global-tslint-disable no-big-function
import { logging } from '@angular-devkit/core';
import { Arguments, Option, OptionType } from './interface';
import { ParseArgumentException, parseArguments } from './parser';
Expand Down Expand Up @@ -64,6 +65,10 @@ describe('parseArguments', () => {
'--str=false val1 val2': { str: 'false', p1: 'val1', p2: 'val2' },
'--str=false val1 val2 --num1': { str: 'false', p1: 'val1', p2: 'val2', '--': ['--num1'] },
'--str=false val1 --num1 val2': { str: 'false', p1: 'val1', '--': ['--num1', 'val2'] },
'--bool --bool=false': { bool: false },
'--bool --bool=false --bool': { bool: true },
'--num=1 --num=2 --num=3': { num: 3 },
'--str=1 --str=2 --str=3': { str: '3' },
'val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' },
'--p1=val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' },
'--p1=val1 --num=1 --p2=val2 val3': { num: 1, p1: 'val1', p2: 'val2', '--': ['val3'] },
Expand All @@ -74,6 +79,7 @@ describe('parseArguments', () => {
],
'--bool val1 --etc --num=1 val2 --v': { bool: true, num: 1, p1: 'val1', p2: 'val2',
'--': ['--etc', '--v'] },
'--arr=a d': { arr: ['a'], p1: 'd' },
'--arr=a --arr=b --arr c d': { arr: ['a', 'b', 'c'], p1: 'd' },
'--arr=1 --arr --arr c d': { arr: ['1', '', 'c'], p1: 'd' },
'--arr=1 --arr --arr c d e': { arr: ['1', '', 'c'], p1: 'd', p2: 'e' },
Expand Down Expand Up @@ -197,4 +203,35 @@ describe('parseArguments', () => {
expect(messages[1]).toMatch(/\bABCD\b/);
messages.shift();
});

it('handles a flag being added multiple times', () => {
const options = [
{ name: 'bool', aliases: [], type: OptionType.Boolean, description: '' },
];

const logger = new logging.Logger('');
const messages: string[] = [];

logger.subscribe(entry => messages.push(entry.message));

let result = parseArguments(['--bool'], options, logger);
expect(result).toEqual({ bool: true });
expect(messages).toEqual([]);

result = parseArguments(['--bool', '--bool'], options, logger);
expect(result).toEqual({ bool: true });
expect(messages).toEqual([]);

result = parseArguments(['--bool', '--bool=false'], options, logger);
expect(result).toEqual({ bool: false });
expect(messages.length).toEqual(1);
expect(messages[0]).toMatch(/\bbool\b.*\btrue\b.*\bfalse\b/);
messages.shift();

result = parseArguments(['--bool', '--bool=false', '--bool=false'], options, logger);
expect(result).toEqual({ bool: false });
expect(messages.length).toEqual(1);
expect(messages[0]).toMatch(/\bbool\b.*\btrue\b.*\bfalse\b/);
messages.shift();
});
});

0 comments on commit 3e9705f

Please sign in to comment.