Skip to content

Commit

Permalink
Config: prevent meaningless empty arrays override options in file
Browse files Browse the repository at this point in the history
It fixes the impossibility that the 'rules' and 'skip' options specified
in speccy.yaml config file are used when these options are not
override from command line.

The Commander.js configuration has an empty array as a default/initial
value for 'rules' and 'skip'. If you execute the lint command from the
command line without specifying those arguments, when the config is
loaded those 'supplied' literal config values (empty arrays) overrides
the ones defined in a speccy.yaml file (because they are not 'undefined').

There is a test validating the config hierarchy but, as the configuration
passed to init() is handcrafted, it wasn't simulating the output of
Commander.js read options.
Now, a test simulating this case was added.
  • Loading branch information
Martin D'Aloia committed May 19, 2019
1 parent f7ba6d5 commit cc775db
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
15 changes: 13 additions & 2 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class Config {
verbose: args.verbose,
// Command specific options
lint: {
rules: args.rules,
skip: args.skip,
rules: this.notEmptyArray(args.rules),
skip: this.notEmptyArray(args.skip),
},
resolve: {
output: args.output,
Expand Down Expand Up @@ -71,6 +71,17 @@ class Config {
});
return cleaned;
}

/**
* Check and returns the given array if it is not empty. Otherwise it returns 'undefined'.
* Useful for configuration options where an empty array has no meaning.
*
* @param array The array instance to check.
* @returns {array|undefined} The given array; or 'undefined' if array is 'undefined' or empty.
*/
notEmptyArray(array) {
return array && array.length > 0 ? array : undefined;
}
}

module.exports = new Config;
4 changes: 2 additions & 2 deletions lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ const command = async (specFile, cmd) => {
config.init(cmd);
const jsonSchema = config.get('jsonSchema');
const verbose = config.get('quiet') ? 0 : config.get('verbose', 1);
const rulesets = config.get('lint:rules');
const skip = config.get('lint:skip');
const rulesets = config.get('lint:rules', []);
const skip = config.get('lint:skip', []);

rules.init({
skip
Expand Down
14 changes: 14 additions & 0 deletions test/lib/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ describe('Config', () => {
'https://example.org/my-rules.json',
]);
});

test('meaningless empty arrays in default config must not override options of config file', () => {
config.init({ config: configFile, rules: [], skip: []});

expect(config.get('lint:rules')).toEqual([
'strict',
'./some/local/rules.json',
'https://example.org/my-rules.json',
]);

expect(config.get('lint:skip')).toEqual([
'info-contact',
]);
});
});
});

Expand Down

0 comments on commit cc775db

Please sign in to comment.