From 5cc6bd135bbfde52ce551466ae3f6daf1a632fca Mon Sep 17 00:00:00 2001 From: Barrett Schonefeld Date: Tue, 4 Feb 2020 15:46:36 -0600 Subject: [PATCH] feat: added configurable warning limits through .thresholdrc file - added code to process a .thresholdrc file to get warnings limit - used warning limit to set exit code to 1 and print a message if warning limit is exceeded - added validation on the user input from .thresholdrc to check if valid keys and values provided in .thresholdrc - set default limit to Number.MAX_VALUE if no .thresholdrc provided - added tests to test error handling for .thresholdrc file and expected output when warning limits exceeded - added test to check if error thrown when invalid JSON provided in .thresholdrc - refactored error printing in processConfiguration file into a printConfigErrors function - ran npm run fix to fix style - documented how to add a warnings limit in the README --- README.md | 18 ++- package-lock.json | 16 ++- src/cli-validator/runValidator.js | 32 ++++- .../utils/processConfiguration.js | 117 +++++++++++++++--- .../mockFiles/thresholds/.fiveWarnings | 3 + .../mockFiles/thresholds/.invalidJSON | 2 + .../mockFiles/thresholds/.invalidValues | 5 + .../mockFiles/thresholds/.zeroWarnings | 3 + .../cli-validator/tests/thresholdValidator.js | 89 +++++++++++++ 9 files changed, 263 insertions(+), 22 deletions(-) create mode 100644 test/cli-validator/mockFiles/thresholds/.fiveWarnings create mode 100644 test/cli-validator/mockFiles/thresholds/.invalidJSON create mode 100644 test/cli-validator/mockFiles/thresholds/.invalidValues create mode 100644 test/cli-validator/mockFiles/thresholds/.zeroWarnings create mode 100644 test/cli-validator/tests/thresholdValidator.js diff --git a/README.md b/README.md index 8b51105b7..07d4835bc 100644 --- a/README.md +++ b/README.md @@ -144,11 +144,11 @@ The object will always have `errors` and `warnings` keys that map to arrays. If ## Configuration The command line validator is built so that each IBM validation can be configured. To get started configuring the validator, [set up](#setup) a [configuration file](#configuration-file) and continue reading this section. Specific validation "rules" can be turned off, or configured to trigger either errors or warnings in the validator. Some validations can be configured even further, such as switching the case convention to validate against for parameter names. -Additionally, certain files files can be ignored by the validator. Any glob placed in a file called `.validateignore` will always be ignored by the validator at runtime. This is set up like a `.gitignore` or a `.eslintignore` file. +Additionally, certain files can be ignored by the validator. Any glob placed in a file called `.validateignore` will always be ignored by the validator at runtime. This is set up like a `.gitignore` or a `.eslintignore` file. ### Setup To set up the configuration capability, simply run the command `lint-openapi init`. -This will create (or over-write) a `.validaterc` file with all rules set to their [default value](#default-values). This command does not create a `.validateignore`. That file must be created manually. These rules can then be changed to configure the validator. Continue reading for more details. +This will create (or overwrite) a `.validaterc` file with all rules set to their [default value](#default-values). This command does not create a `.validateignore`. That file must be created manually. These rules can then be changed to configure the validator. Continue reading for more details. _WARNING: If a `.validaterc` file already exists and has been customized, this command will reset all rules to their default values._ @@ -429,6 +429,20 @@ The default values for each rule are described below. | duplicate_sibling_description | warning | | incorrect_ref_pattern | warning | +## Warnings Limit + +To impose a warnings limit on a project, add a `.thresholdrc` to your project. It is recommended to add this file to the root of the project. The validator recursively searches up the filesystem from whichever directory the validator is invoked, and the nearest `.thresholdrc` will be used. + +The format for the `.thresholdrc` file is a top-level JSON object with a `"warnings"` field (shown below). + + { + "warnings": 0 + } + +###### limits +| Limit | Default | +| ----------------------- | --------- | +| warnings | MAX_VALUE | ## Turning off `update-notifier` This package uses [`update-notifier`](https://github.com/yeoman/update-notifier) to alert users when new versions of the tool are available. To turn this feature _off_, follow [these instructions](https://github.com/yeoman/update-notifier/tree/8df01b35fbb8093e91d79fdf9900c344c2236f08#user-settings) from the package authors. It is recommended to keep this feature _on_ to help stay up to date with the latest changes. diff --git a/package-lock.json b/package-lock.json index f057946c6..8e8a1d92a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3760,6 +3760,7 @@ "version": "4.5.3", "resolved": "https://registry.npmjs.org/handlebars/-/handlebars-4.5.3.tgz", "integrity": "sha512-3yPecJoJHK/4c6aZhSvxOyG4vJKDshV36VHp0iVCDVh7o9w2vwi3NSnL2MMPj3YdduqaBcu7cGbggJQM0br9xA==", + "dev": true, "requires": { "neo-async": "^2.6.0", "optimist": "^0.6.1", @@ -3770,7 +3771,8 @@ "source-map": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", - "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==" + "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", + "dev": true } } }, @@ -5274,7 +5276,8 @@ "minimist": { "version": "0.0.8", "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", - "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" + "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", + "dev": true }, "minimist-options": { "version": "3.0.2", @@ -5421,7 +5424,8 @@ "neo-async": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/neo-async/-/neo-async-2.6.1.tgz", - "integrity": "sha512-iyam8fBuCUpWeKPGpaNMetEocMt364qkCsfL9JuhjXX6dRnguRVOfk2GZaDpPjcOKiiXCPINZC1GczQ7iTq3Zw==" + "integrity": "sha512-iyam8fBuCUpWeKPGpaNMetEocMt364qkCsfL9JuhjXX6dRnguRVOfk2GZaDpPjcOKiiXCPINZC1GczQ7iTq3Zw==", + "dev": true }, "nerf-dart": { "version": "1.0.0", @@ -9317,6 +9321,7 @@ "version": "0.6.1", "resolved": "https://registry.npmjs.org/optimist/-/optimist-0.6.1.tgz", "integrity": "sha1-2j6nRob6IaGaERwybpDrFaAZZoY=", + "dev": true, "requires": { "minimist": "~0.0.1", "wordwrap": "~0.0.2" @@ -9325,7 +9330,8 @@ "wordwrap": { "version": "0.0.3", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-0.0.3.tgz", - "integrity": "sha1-o9XabNXAvAAI03I0u68b7WMFkQc=" + "integrity": "sha1-o9XabNXAvAAI03I0u68b7WMFkQc=", + "dev": true } } }, @@ -11540,6 +11546,7 @@ "version": "3.4.9", "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-3.4.9.tgz", "integrity": "sha512-8CJsbKOtEbnJsTyv6LE6m6ZKniqMiFWmm9sRbopbkGs3gMPPfd3Fh8iIA4Ykv5MgaTbqHr4BaoGLJLZNhsrW1Q==", + "dev": true, "optional": true, "requires": { "commander": "~2.17.1", @@ -11550,6 +11557,7 @@ "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", + "dev": true, "optional": true } } diff --git a/src/cli-validator/runValidator.js b/src/cli-validator/runValidator.js index d96e3f8c3..1cb5bbc56 100644 --- a/src/cli-validator/runValidator.js +++ b/src/cli-validator/runValidator.js @@ -41,6 +41,8 @@ const processInput = async function(program) { const configFileOverride = program.config; + const limitsFileOverride = program.limits; + // turn on coloring by default const colors = turnOffColoring ? false : true; @@ -140,6 +142,14 @@ const processInput = async function(program) { return Promise.reject(err); } + // get limits from .thresholdrc file + let limitsObject; + try { + limitsObject = await config.limits(chalk, limitsFileOverride); + } catch (err) { + return Promise.reject(err); + } + // define an exit code to return. this will tell the parent program whether // the validator passed or not let exitCode = 0; @@ -250,8 +260,26 @@ const processInput = async function(program) { originalFile, errorsOnly ); - // fail on errors, but not if there are only warnings - if (results.error) exitCode = 1; + // fail on errors or if number of warnings exceeds warnings limit + if (results.error) { + exitCode = 1; + } else { + // Calculate number of warnings and set exit code to 1 if warning limit exceeded + let numWarnings = 0; + for (const key of Object.keys(results.warnings)) { + numWarnings += results.warnings[key].length; + } + if (numWarnings > limitsObject.warnings) { + exitCode = 1; + console.log( + chalk.red( + `Number of warnings (${numWarnings}) exceeds warnings limit (${ + limitsObject.warnings + }).` + ) + ); + } + } } else { console.log(chalk.green(`\n${validFile} passed the validator`)); if (validFile === last(filesToValidate)) console.log(); diff --git a/src/cli-validator/utils/processConfiguration.js b/src/cli-validator/utils/processConfiguration.js index d2379c0d1..59b1797d5 100644 --- a/src/cli-validator/utils/processConfiguration.js +++ b/src/cli-validator/utils/processConfiguration.js @@ -13,6 +13,26 @@ const defaultObject = defaultConfig.defaults; const deprecatedRuleObject = defaultConfig.deprecated; const configOptions = defaultConfig.options; +const printConfigErrors = function(problems, chalk, fileName) { + const description = `Invalid configuration in ${chalk.underline( + fileName + )} file. See below for details.`; + + const message = []; + + // add all errors for printError + problems.forEach(function(problem) { + message.push( + `\n - ${chalk.red(problem.message)}\n ${chalk.magenta( + problem.correction + )}` + ); + }); + if (message.length) { + printError(chalk, description, message.join('\n')); + } +}; + const validateConfigObject = function(configObject, chalk) { const configErrors = []; let validObject = true; @@ -147,21 +167,8 @@ const validateConfigObject = function(configObject, chalk) { configObject.invalid = false; } else { // if the object is not valid, exit and tell the user why - const description = `Invalid configuration in ${chalk.underline( - '.validaterc' - )} file. See below for details.`; - const message = []; - - // concatenate all the error messages for the printError module - configErrors.forEach(function(problem) { - message.push( - `\n - ${chalk.red(problem.message)}\n ${chalk.magenta( - problem.correction - )}` - ); - }); + printConfigErrors(configErrors, chalk, '.validaterc'); - printError(chalk, description, message.join('\n')); configObject.invalid = true; } @@ -263,6 +270,86 @@ const getFilesToIgnore = async function() { return filesToIgnore; }; +const validateLimits = function(limitsObject, chalk) { + const allowedLimits = ['warnings']; + const limitErrors = []; + + Object.keys(limitsObject).forEach(function(key) { + if (!allowedLimits.includes(key)) { + // remove the entry and notify the user + delete limitsObject[key]; + limitErrors.push({ + message: `"${key}" limit not supported. This value will be ignored.`, + correction: `Valid limits for .thresholdrc are: ${allowedLimits.join( + ', ' + )}.` + }); + } else { + // valid limit option, ensure the limit given is a number + if (typeof limitsObject[key] !== 'number') { + // remove the entry and notify the user + delete limitsObject[key]; + limitErrors.push({ + message: `Value provided for ${key} limit is invalid.`, + correction: `${key} limit should be a number.` + }); + } + } + }); + + // give the user corrections for .thresholdrc file + if (limitErrors.length) { + printConfigErrors(limitErrors, chalk, '.thresholdrc'); + } + + // sets all limits options not defined by user to default + for (const limitOption of allowedLimits) { + if (!(limitOption in limitsObject)) { + limitsObject[limitOption] = Number.MAX_VALUE; + } + } + + return limitsObject; +}; + +const getLimits = async function(chalk, limitsFileOverride) { + let limitsObject = {}; + + const findUpOpts = {}; + let limitsFileName; + + if (limitsFileOverride) { + limitsFileName = path.basename(limitsFileOverride); + findUpOpts.cwd = path.dirname(limitsFileOverride); + } else { + limitsFileName = '.thresholdrc'; + } + + // search up the file system for the first instance + // of the threshold file + const limitsFile = await findUp(limitsFileName, findUpOpts); + + if (limitsFile !== null) { + try { + // the threshold file must be in the root folder of the project + const fileAsString = await readFile(limitsFile, 'utf8'); + limitsObject = JSON.parse(fileAsString); + } catch (err) { + // this most likely means there is a problem in the json syntax itself + const description = + 'There is a problem with the .thresholdrc file. See below for details.'; + printError(chalk, description, err); + return Promise.reject(2); + } + } + + // returns complete limits object with all valid user settings + // and default values for undefined limits + limitsObject = validateLimits(limitsObject, chalk); + + return limitsObject; +}; + const validateConfigOption = function(userOption, defaultOption) { const result = { valid: true }; // determine what type of option it is @@ -286,3 +373,5 @@ module.exports.get = getConfigObject; module.exports.validate = validateConfigObject; module.exports.ignore = getFilesToIgnore; module.exports.validateOption = validateConfigOption; +module.exports.validateLimits = validateLimits; +module.exports.limits = getLimits; diff --git a/test/cli-validator/mockFiles/thresholds/.fiveWarnings b/test/cli-validator/mockFiles/thresholds/.fiveWarnings new file mode 100644 index 000000000..fcebb9e7b --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/.fiveWarnings @@ -0,0 +1,3 @@ +{ + "warnings": 5 +} diff --git a/test/cli-validator/mockFiles/thresholds/.invalidJSON b/test/cli-validator/mockFiles/thresholds/.invalidJSON new file mode 100644 index 000000000..dd56dd85c --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/.invalidJSON @@ -0,0 +1,2 @@ +[ + } diff --git a/test/cli-validator/mockFiles/thresholds/.invalidValues b/test/cli-validator/mockFiles/thresholds/.invalidValues new file mode 100644 index 000000000..5b5049bf5 --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/.invalidValues @@ -0,0 +1,5 @@ +{ + "errors": 0, + "warnings": "text", + "population": 10 +} diff --git a/test/cli-validator/mockFiles/thresholds/.zeroWarnings b/test/cli-validator/mockFiles/thresholds/.zeroWarnings new file mode 100644 index 000000000..1f14b6ccc --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/.zeroWarnings @@ -0,0 +1,3 @@ +{ + "warnings": 0 +} diff --git a/test/cli-validator/tests/thresholdValidator.js b/test/cli-validator/tests/thresholdValidator.js new file mode 100644 index 000000000..c5dea39c8 --- /dev/null +++ b/test/cli-validator/tests/thresholdValidator.js @@ -0,0 +1,89 @@ +// the rule names are all snake case and need to stay that way. don't lint them +/* eslint-disable camelcase */ + +const intercept = require('intercept-stdout'); +const expect = require('expect'); +const stripAnsiFrom = require('strip-ansi'); + +const commandLineValidator = require('../../../src/cli-validator/runValidator'); + +describe('test the .thresholdrc limits', function() { + it('should show error and set exit code to 1 when warning limit exceeded', async function() { + const capturedText = []; + + const program = {}; + program.args = ['./test/cli-validator/mockFiles/circularRefs.yml']; + program.limits = './test/cli-validator/mockFiles/thresholds/.fiveWarnings'; + program.default_mode = true; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + const exitCode = await commandLineValidator(program); + + unhookIntercept(); + + expect(exitCode).toEqual(1); + + expect(capturedText[capturedText.length - 1].slice(0, 18)).toEqual( + `Number of warnings` + ); + }); + + it('should print errors for unsupported limit options and invalid limit values', async function() { + const capturedText = []; + + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = './test/cli-validator/mockFiles/thresholds/.invalidValues'; + program.default_mode = true; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + const exitCode = await commandLineValidator(program); + + unhookIntercept(); + + // limit values invalid, so default limit, Number.MAX_VALUE, used + expect(exitCode).toEqual(0); + + const allOutput = capturedText.join(''); + + expect( + allOutput.includes('"population" limit not supported.') && + allOutput.includes('Value provided for warnings') + ).toEqual(true); + }); + + it('should give exit code 0 when warnings limit not exceeded', async function() { + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = './test/cli-validator/mockFiles/thresholds/.zeroWarnings'; + program.default_mode = true; + + const exitCode = await commandLineValidator(program); + + expect(exitCode).toEqual(0); + }); + + it('should give an error for invalid JSON', async function() { + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = './test/cli-validator/mockFiles/thresholds/.invalidJSON'; + program.default_mode = true; + + try { + await commandLineValidator(program); + // error should be thrown for invalid JSON + expect(true).toEqual(false); + } catch (err) { + // invalid JSON in .thresholdrc, reject with exit code 2 + expect(true).toEqual(true); + } + }); +});