Skip to content

Commit 2ffde25

Browse files
committed
fix(@angular/cli): numerical flags should not give 0 if empty
And numerical positional flags will be ignored. If the value is an empty string, a number conversion would give 0. It is unexpected from the user standpoint ("--num=" has the user expect a string value).
1 parent c3e2ab7 commit 2ffde25

File tree

2 files changed

+17
-5
lines changed

2 files changed

+17
-5
lines changed

packages/angular/cli/models/parser.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu
5454
case OptionType.Number:
5555
if (str === undefined) {
5656
return 0;
57+
} else if (str === '') {
58+
return undefined;
5759
} else if (Number.isFinite(+str)) {
5860
return +str;
5961
} else {
@@ -308,6 +310,8 @@ export function parseArguments(args: string[], options: Option[] | null): Argume
308310
}
309311

310312
// Deal with positionals.
313+
// TODO(hansl): this is by far the most complex piece of code in this file. Try to refactor it
314+
// simpler.
311315
if (positionals.length > 0) {
312316
let pos = 0;
313317
for (let i = 0; i < positionals.length;) {
@@ -317,10 +321,11 @@ export function parseArguments(args: string[], options: Option[] | null): Argume
317321

318322
// We do this with a found flag because more than 1 option could have the same positional.
319323
for (const option of options) {
320-
// If any option has this positional and no value, we need to remove it.
324+
// If any option has this positional and no value, AND fit the type, we need to remove it.
321325
if (option.positional === pos) {
322-
if (parsedOptions[option.name] === undefined) {
323-
parsedOptions[option.name] = positionals[i];
326+
const coercedValue = _coerce(positionals[i], option, parsedOptions[option.name]);
327+
if (parsedOptions[option.name] === undefined && coercedValue !== undefined) {
328+
parsedOptions[option.name] = coercedValue;
324329
found = true;
325330
} else {
326331
incrementI = false;

packages/angular/cli/models/parser_spec.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ describe('parseArguments', () => {
1919
{ name: 'arr', aliases: [ 'a' ], type: OptionType.Array, description: '' },
2020
{ name: 'p1', positional: 0, aliases: [], type: OptionType.String, description: '' },
2121
{ name: 'p2', positional: 1, aliases: [], type: OptionType.String, description: '' },
22+
{ name: 'p3', positional: 2, aliases: [], type: OptionType.Number, description: '' },
2223
{ name: 't1', aliases: [], type: OptionType.Boolean,
2324
types: [OptionType.Boolean, OptionType.String], description: '' },
2425
{ name: 't2', aliases: [], type: OptionType.Boolean,
@@ -63,8 +64,13 @@ describe('parseArguments', () => {
6364
'val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' },
6465
'--p1=val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' },
6566
'--p1=val1 --num=1 --p2=val2 val3': { num: 1, p1: 'val1', p2: 'val2', '--': ['val3'] },
66-
'--bool val1 --etc --num val2 --v': { bool: true, num: 0, p1: 'val1', p2: 'val2',
67-
'--': ['--etc', '--v'] },
67+
'--bool val1 --etc --num val2 --v': [
68+
'!!!',
69+
{ bool: true, p1: 'val1', p2: 'val2', '--': ['--etc', '--v'] },
70+
['--num' ],
71+
],
72+
'--bool val1 --etc --num=1 val2 --v': { bool: true, num: 1, p1: 'val1', p2: 'val2',
73+
'--': ['--etc', '--v'] },
6874
'--arr=a --arr=b --arr c d': { arr: ['a', 'b', 'c'], p1: 'd' },
6975
'--arr=1 --arr --arr c d': { arr: ['1', '', 'c'], p1: 'd' },
7076
'--arr=1 --arr --arr c d e': { arr: ['1', '', 'c'], p1: 'd', p2: 'e' },
@@ -121,6 +127,7 @@ describe('parseArguments', () => {
121127
'--e3': { e3: true },
122128
'--e3 true': { e3: true },
123129
'--e3=true': { e3: true },
130+
'a b c 1': { p1: 'a', p2: 'b', '--': ['c', '1'] },
124131
};
125132

126133
Object.entries(tests).forEach(([str, expected]) => {

0 commit comments

Comments
 (0)