Skip to content

Commit

Permalink
fix(@angular/cli): pass arguments to all targets
Browse files Browse the repository at this point in the history
When running a command with args against multiple targets, all targets
should be given the args. As parseArguments was mutating the passed args
array this wasn't the case. Fix by not mutating the array.

This was especially noticeable when using the `ng lint --fix` command
on a newly generated project, as files in the app target would be fixed,
but e2e target would be only be linted (with no fix)

Possibly closes #10657, #10656, #11005
  • Loading branch information
mnahkies authored and alexeagle committed Dec 6, 2018
1 parent f7f693c commit 7d88182
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 13 deletions.
34 changes: 22 additions & 12 deletions packages/angular/cli/models/parser.ts
Expand Up @@ -122,7 +122,7 @@ function _removeLeadingDashes(key: string): string {

function _assignOption(
arg: string,
args: string[],
nextArg: string | undefined,
{ options, parsedOptions, leftovers, ignored, errors, warnings }: {
options: Option[],
parsedOptions: Arguments,
Expand All @@ -134,9 +134,10 @@ function _assignOption(
},
) {
const from = arg.startsWith('--') ? 2 : 1;
let consumedNextArg = false;
let key = arg.substr(from);
let option: Option | null = null;
let value = '';
let value: string | undefined = '';
const i = arg.indexOf('=');

// If flag is --no-abc AND there's no equal sign.
Expand All @@ -155,7 +156,7 @@ function _assignOption(
// Set it to true if it's a boolean and the next argument doesn't match true/false.
const maybeOption = _getOptionFromName(key, options);
if (maybeOption) {
value = args[0];
value = nextArg;
let shouldShift = true;

if (value && value.startsWith('-')) {
Expand All @@ -167,7 +168,7 @@ function _assignOption(

// Only absorb it if it leads to a better value.
if (shouldShift && _coerce(value, maybeOption) !== undefined) {
args.shift();
consumedNextArg = true;
} else {
value = '';
}
Expand All @@ -183,9 +184,9 @@ function _assignOption(
}

if (option === null) {
if (args[0] && !args[0].startsWith('-')) {
leftovers.push(arg, args[0]);
args.shift();
if (nextArg && !nextArg.startsWith('-')) {
leftovers.push(arg, nextArg);
consumedNextArg = true;
} else {
leftovers.push(arg);
}
Expand Down Expand Up @@ -220,6 +221,8 @@ function _assignOption(
ignored.push(arg);
}
}

return consumedNextArg;
}


Expand Down Expand Up @@ -303,30 +306,33 @@ export function parseArguments(

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

for (let arg = args.shift(); arg !== undefined; arg = args.shift()) {
for (let argIndex = 0; argIndex < args.length; argIndex++) {
const arg = args[argIndex];
let consumedNextArg = false;

if (arg == '--') {
// If we find a --, we're done.
leftovers.push(...args);
leftovers.push(...args.slice(argIndex + 1));
break;
}

if (arg.startsWith('--')) {
_assignOption(arg, args, state);
consumedNextArg = _assignOption(arg, args[argIndex + 1], state);
} else if (arg.startsWith('-')) {
// Argument is of form -abcdef. Starts at 1 because we skip the `-`.
for (let i = 1; i < arg.length; i++) {
const flag = arg[i];
// If the next character is an '=', treat it as a long flag.
if (arg[i + 1] == '=') {
const f = '-' + flag + arg.slice(i + 1);
_assignOption(f, args, state);
consumedNextArg = _assignOption(f, args[argIndex + 1], state);
break;
}
// Treat the last flag as `--a` (as if full flag but just one letter). We do this in
// the loop because it saves us a check to see if the arg is just `-`.
if (i == arg.length - 1) {
const arg = '-' + flag;
_assignOption(arg, args, state);
consumedNextArg = _assignOption(arg, args[argIndex + 1], state);
} else {
const maybeOption = _getOptionFromName(flag, options);
if (maybeOption) {
Expand All @@ -340,6 +346,10 @@ export function parseArguments(
} else {
positionals.push(arg);
}

if (consumedNextArg) {
argIndex++;
}
}

// Deal with positionals.
Expand Down
6 changes: 5 additions & 1 deletion packages/angular/cli/models/parser_spec.ts
Expand Up @@ -150,10 +150,14 @@ describe('parseArguments', () => {
Object.entries(tests).forEach(([str, expected]) => {
it(`works for ${str}`, () => {
try {
const actual = parseArguments(str.split(' '), options);
const originalArgs = str.split(' ');
const args = originalArgs.slice();

const actual = parseArguments(args, options);

expect(Array.isArray(expected)).toBe(false);
expect(actual).toEqual(expected as Arguments);
expect(args).toEqual(originalArgs);
} catch (e) {
if (!(e instanceof ParseArgumentException)) {
throw e;
Expand Down

0 comments on commit 7d88182

Please sign in to comment.