From 7d782a3f5eb2332ba58ad0f1087c576dab451730 Mon Sep 17 00:00:00 2001 From: Hans Date: Tue, 18 Sep 2018 13:43:00 -0700 Subject: [PATCH] feat(@angular/cli): add support for parsing enums Options can now contain enumerations of values. --- packages/angular/cli/models/interface.ts | 17 +++++-- packages/angular/cli/models/parser.ts | 44 ++++++++++++----- packages/angular/cli/models/parser_spec.ts | 57 ++++++++++++++++++++-- 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/packages/angular/cli/models/interface.ts b/packages/angular/cli/models/interface.ts index 261540412d0c..940ae85c1fd1 100644 --- a/packages/angular/cli/models/interface.ts +++ b/packages/angular/cli/models/interface.ts @@ -63,11 +63,11 @@ export interface CommandContext { * Value types of an Option. */ export enum OptionType { - String = 'string', - Number = 'number', - Boolean = 'boolean', - Array = 'array', Any = 'any', + Array = 'array', + Boolean = 'boolean', + Number = 'number', + String = 'string', } /** @@ -95,6 +95,15 @@ export interface Option { */ types?: OptionType[]; + /** + * If this field is set, only values contained in this field are valid. This array can be mixed + * types (strings, numbers, boolean). For example, if this field is "enum: ['hello', true]", + * then "type" will be either string or boolean, types will be at least both, and the values + * accepted will only be either 'hello' or true (not false or any other string). + * This mean that prefixing with `no-` will not work on this field. + */ + enum?: Value[]; + /** * If this option maps to a subcommand in the parent command, will contain all the subcommands * supported. There is a maximum of 1 subcommand Option per command, and the type of this diff --git a/packages/angular/cli/models/parser.ts b/packages/angular/cli/models/parser.ts index 94f190e75080..a4a3b6b1abb0 100644 --- a/packages/angular/cli/models/parser.ts +++ b/packages/angular/cli/models/parser.ts @@ -12,7 +12,7 @@ import { Arguments, Option, OptionType, Value } from './interface'; function _coerceType(str: string | undefined, type: OptionType, v?: Value): Value | undefined { switch (type) { - case 'any': + case OptionType.Any: if (Array.isArray(v)) { return v.concat(str || ''); } @@ -23,10 +23,10 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu ? _coerceType(str, OptionType.Number, v) : _coerceType(str, OptionType.String, v); - case 'string': + case OptionType.String: return str || ''; - case 'boolean': + case OptionType.Boolean: switch (str) { case 'false': return false; @@ -40,7 +40,7 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu return undefined; } - case 'number': + case OptionType.Number: if (str === undefined) { return 0; } else if (Number.isFinite(+str)) { @@ -49,7 +49,7 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu return undefined; } - case 'array': + case OptionType.Array: return Array.isArray(v) ? v.concat(str || '') : [str || '']; default: @@ -61,7 +61,20 @@ function _coerce(str: string | undefined, o: Option | null, v?: Value): Value | if (!o) { return _coerceType(str, OptionType.Any, v); } else { - return _coerceType(str, o.type, v); + const types = o.types || [o.type]; + + // Try all the types one by one and pick the first one that returns a value contained in the + // enum. If there's no enum, just return the first one that matches. + for (const type of types) { + const maybeResult = _coerceType(str, type, v); + if (maybeResult !== undefined) { + if (!o.enum || o.enum.includes(maybeResult)) { + return maybeResult; + } + } + } + + return undefined; } } @@ -118,10 +131,18 @@ 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) { - // Not of type boolean, consume the next value. value = args[0]; - // Only absorb it if it leads to a value. - if (_coerce(value, maybeOption) !== undefined) { + let shouldShift = true; + + if (value && value.startsWith('-')) { + // Verify if not having a value results in a correct parse, if so don't shift. + if (_coerce(undefined, maybeOption) !== undefined) { + shouldShift = false; + } + } + + // Only absorb it if it leads to a better value. + if (shouldShift && _coerce(value, maybeOption) !== undefined) { args.shift(); } else { value = ''; @@ -134,9 +155,6 @@ function _assignOption( option = _getOptionFromName(key, options) || null; if (option) { value = arg.substring(i + 1); - if (option.type === 'boolean' && _coerce(value, option) === undefined) { - value = 'true'; - } } } if (option === null) { @@ -150,6 +168,8 @@ function _assignOption( const v = _coerce(value, option, parsedOptions[option.name]); if (v !== undefined) { parsedOptions[option.name] = v; + } else { + leftovers.push(arg); } } } diff --git a/packages/angular/cli/models/parser_spec.ts b/packages/angular/cli/models/parser_spec.ts index 961f37952aae..783f1fd797a4 100644 --- a/packages/angular/cli/models/parser_spec.ts +++ b/packages/angular/cli/models/parser_spec.ts @@ -19,11 +19,23 @@ describe('parseArguments', () => { { name: 'arr', aliases: [ 'a' ], type: OptionType.Array, description: '' }, { name: 'p1', positional: 0, aliases: [], type: OptionType.String, description: '' }, { name: 'p2', positional: 1, aliases: [], type: OptionType.String, description: '' }, + { name: 't1', aliases: [], type: OptionType.Boolean, + types: [OptionType.Boolean, OptionType.String], description: '' }, + { name: 't2', aliases: [], type: OptionType.Boolean, + types: [OptionType.Boolean, OptionType.Number], description: '' }, + { name: 't3', aliases: [], type: OptionType.Number, + types: [OptionType.Number, OptionType.Any], description: '' }, + { name: 'e1', aliases: [], type: OptionType.String, enum: ['hello', 'world'], description: '' }, + { name: 'e2', aliases: [], type: OptionType.String, enum: ['hello', ''], description: '' }, + { name: 'e3', aliases: [], type: OptionType.Boolean, + types: [OptionType.String, OptionType.Boolean], enum: ['json', true, false], + description: '' }, ]; const tests: { [test: string]: Partial } = { '--bool': { bool: true }, - '--bool=1': { bool: true }, + '--bool=1': { '--': ['--bool=1'] }, + '--bool=yellow': { '--': ['--bool=yellow'] }, '--bool=true': { bool: true }, '--bool=false': { bool: false }, '--no-bool': { bool: false }, @@ -33,7 +45,7 @@ describe('parseArguments', () => { '--b true': { bool: true }, '--b false': { bool: false }, '--bool --num': { bool: true, num: 0 }, - '--bool --num=true': { bool: true }, + '--bool --num=true': { bool: true, '--': ['--num=true'] }, '--bool=true --num': { bool: true, num: 0 }, '--bool true --num': { bool: true, num: 0 }, '--bool=false --num': { bool: false, num: 0 }, @@ -51,16 +63,55 @@ describe('parseArguments', () => { '--bool val1 --etc --num val2 --v': { bool: true, num: 0, p1: 'val1', p2: 'val2', '--': ['--etc', '--v'] }, '--arr=a --arr=b --arr c d': { arr: ['a', 'b', 'c'], p1: 'd' }, - '--arr=1 --arr --arr c d': { arr: ['1', '--arr'], p1: 'c', p2: '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' }, '--str=1': { str: '1' }, '--hello-world=1': { helloWorld: '1' }, '--hello-bool': { helloBool: true }, '--helloBool': { helloBool: true }, '--no-helloBool': { helloBool: false }, '--noHelloBool': { helloBool: false }, + '--noBool': { bool: false }, '-b': { bool: true }, '-sb': { bool: true, str: '' }, '-bs': { bool: true, str: '' }, + '--t1=true': { t1: true }, + '--t1': { t1: true }, + '--t1 --num': { t1: true, num: 0 }, + '--no-t1': { t1: false }, + '--t1=yellow': { t1: 'yellow' }, + '--no-t1=true': { '--': ['--no-t1=true'] }, + '--t1=123': { t1: '123' }, + '--t2=true': { t2: true }, + '--t2': { t2: true }, + '--no-t2': { t2: false }, + '--t2=yellow': { '--': ['--t2=yellow'] }, + '--no-t2=true': { '--': ['--no-t2=true'] }, + '--t2=123': { t2: 123 }, + '--t3=a': { t3: 'a' }, + '--t3': { t3: 0 }, + '--t3 true': { t3: true }, + '--e1 hello': { e1: 'hello' }, + '--e1=hello': { e1: 'hello' }, + '--e1 yellow': { p1: 'yellow', '--': ['--e1'] }, + '--e1=yellow': { '--': ['--e1=yellow'] }, + '--e1': { '--': ['--e1'] }, + '--e1 true': { p1: 'true', '--': ['--e1'] }, + '--e1=true': { '--': ['--e1=true'] }, + '--e2 hello': { e2: 'hello' }, + '--e2=hello': { e2: 'hello' }, + '--e2 yellow': { p1: 'yellow', e2: '' }, + '--e2=yellow': { '--': ['--e2=yellow'] }, + '--e2': { e2: '' }, + '--e2 true': { p1: 'true', e2: '' }, + '--e2=true': { '--': ['--e2=true'] }, + '--e3 json': { e3: 'json' }, + '--e3=json': { e3: 'json' }, + '--e3 yellow': { p1: 'yellow', e3: true }, + '--e3=yellow': { '--': ['--e3=yellow'] }, + '--e3': { e3: true }, + '--e3 true': { e3: true }, + '--e3=true': { e3: true }, }; Object.entries(tests).forEach(([str, expected]) => {