feat(@angular/cli): add warning for overriding flags in arguments

Fixes #12948

Author: hansl

packages/angular/cli/models/parser.ts

@@ -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;
@@ -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;
@@ -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)}.`;
@@ -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 == '--') {
@@ -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) {

packages/angular/cli/models/parser_spec.ts

@@ -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';
@@ -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'] },
@@ -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' },
@@ -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();
+  });
 });