Skip to content

Commit

Permalink
fix(options): Map type options should not have their default value va…
Browse files Browse the repository at this point in the history
…lidated (#1250)

* No need to convert defaultValue for options of type map
* Added test for map declaration option with foreign default value
  • Loading branch information
krisztianb committed Mar 29, 2020
1 parent 2e63096 commit c296503
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/lib/utils/options/options.ts
@@ -1,7 +1,7 @@
import * as _ from 'lodash';
import * as ts from 'typescript';

import { DeclarationOption, ParameterScope, convert, TypeDocOptions, KeyToDeclaration, TypeDocAndTSOptions, TypeDocOptionMap } from './declaration';
import { DeclarationOption, ParameterScope, ParameterType, convert, TypeDocOptions, KeyToDeclaration, TypeDocAndTSOptions, TypeDocOptionMap } from './declaration';
import { Logger } from '../loggers';
import { Result, Ok, Err } from '../result';
import { insertPrioritySorted } from '../array';
Expand Down Expand Up @@ -109,10 +109,7 @@ export class Options {
*/
reset() {
for (const declaration of this._declarations.values()) {
if (declaration.scope !== ParameterScope.TypeScript) {
this._values[declaration.name] = convert(declaration.defaultValue, declaration)
.expect(`Failed to validate default value for ${declaration.name}`);
}
this.setOptionValueToDefault(declaration);
}
this._compilerOptions = {};
}
Expand Down Expand Up @@ -169,10 +166,7 @@ export class Options {
}
}

if (declaration.scope !== ParameterScope.TypeScript) {
this._values[declaration.name] = convert(declaration.defaultValue, declaration)
.expect(`Failed to validate default value for ${declaration.name}`);
}
this.setOptionValueToDefault(declaration);
}

/**
Expand Down Expand Up @@ -320,6 +314,22 @@ export class Options {
}
return errors.length ? Err(errors) : Ok(void 0);
}

/**
* Sets the value of a given option to its default value.
* @param declaration The option whoes value should be reset.
*/
private setOptionValueToDefault(declaration: Readonly<DeclarationOption>): void {
if (declaration.scope !== ParameterScope.TypeScript) {
// No nead to convert the defaultValue for a map type as it has to be of a specific type
if (declaration.type === ParameterType.Map) {
this._values[declaration.name] = declaration.defaultValue;
} else {
this._values[declaration.name] = convert(declaration.defaultValue, declaration)
.expect(`Failed to validate default value for ${declaration.name}`);
}
}
}
}

/**
Expand Down
15 changes: 15 additions & 0 deletions src/test/utils/options/options.test.ts
Expand Up @@ -25,6 +25,21 @@ describe('Options', () => {
options.removeDeclarationByName(declaration.name);
});

it('Does not error if a map declaration has a default value that is not part of the map of possible values', () => {
logger.resetErrors();
options.addDeclaration({
name: 'testMapDeclarationWithForeignDefaultValue',
help: '',
type: ParameterType.Map,
map: new Map([
['a', 1],
['b', 2]
]),
defaultValue: 0
});
equal(logger.hasErrors(), false);
});

it('Supports removing a declaration by name', () => {
options.addDeclaration({ name: 'not-an-option', help: '' });
options.removeDeclarationByName('not-an-option');
Expand Down

0 comments on commit c296503

Please sign in to comment.