Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix: don't validate schemas for disabled rules (fixes #7690) (#7692)
  • Loading branch information
not-an-aardvark authored and kaicataldo committed Dec 6, 2016
1 parent 2ac07d8 commit abfd24f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 49 deletions.
90 changes: 42 additions & 48 deletions lib/config/config-validator.js
Expand Up @@ -54,65 +54,59 @@ function getRuleOptionsSchema(id) {
}

/**
* Validates a rule's options against its schema.
* @param {string} id The rule's unique name.
* @param {array|number} options The given options for the rule.
* @param {string} source The name of the configuration source.
* @returns {void}
*/
function validateRuleOptions(id, options, source) {
const schema = getRuleOptionsSchema(id);
let validateRule = validators.rules[id],
severity,
localOptions,
validSeverity = true;

if (!validateRule && schema) {
validateRule = schemaValidator(schema, { verbose: true });
validators.rules[id] = validateRule;
* Validates a rule's severity and returns the severity value. Throws an error if the severity is invalid.
* @param {options} options The given options for the rule.
* @returns {number|string} The rule's severity value
*/
function validateRuleSeverity(options) {
const severity = Array.isArray(options) ? options[0] : options;

if (severity !== 0 && severity !== 1 && severity !== 2 && !(typeof severity === "string" && /^(?:off|warn|error)$/i.test(severity))) {
throw new Error(`\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '${util.inspect(severity).replace(/'/g, "\"").replace(/\n/g, "")}').\n`);
}

// if it's not an array, it should be just a severity
if (Array.isArray(options)) {
localOptions = options.concat(); // clone
severity = localOptions.shift();
} else {
severity = options;
localOptions = [];
return severity;
}

/**
* Validates the non-severity options passed to a rule, based on its schema.
* @param {string} id The rule's unique name
* @param {array} localOptions The options for the rule, excluding severity
* @returns {void}
*/
function validateRuleSchema(id, localOptions) {
const schema = getRuleOptionsSchema(id);

if (!validators.rules[id] && schema) {
validators.rules[id] = schemaValidator(schema, { verbose: true });
}

validSeverity = (
severity === 0 || severity === 1 || severity === 2 ||
(typeof severity === "string" && /^(?:off|warn|error)$/i.test(severity))
);
const validateRule = validators.rules[id];

if (validateRule) {
validateRule(localOptions);
if (validateRule.errors) {
throw new Error(validateRule.errors.map(error => `\tValue "${error.value}" ${error.message}.\n`).join(""));
}
}
}

if ((validateRule && validateRule.errors) || !validSeverity) {
const message = [
source, ":\n",
"\tConfiguration for rule \"", id, "\" is invalid:\n"
];

if (!validSeverity) {
message.push(
"\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '",
util.inspect(severity).replace(/'/g, "\"").replace(/\n/g, ""),
"').\n"
);
}
/**
* Validates a rule's options against its schema.
* @param {string} id The rule's unique name.
* @param {array|number} options The given options for the rule.
* @param {string} source The name of the configuration source.
* @returns {void}
*/
function validateRuleOptions(id, options, source) {
try {
const severity = validateRuleSeverity(options);

if (validateRule && validateRule.errors) {
validateRule.errors.forEach(error => {
message.push(
"\tValue \"", error.value, "\" ", error.message, ".\n"
);
});
if (severity !== 0 && !(typeof severity === "string" && severity.toLowerCase() === "off")) {
validateRuleSchema(id, Array.isArray(options) ? options.slice(1) : []);
}

throw new Error(message.join(""));
} catch (err) {
throw new Error(`${source}:\n\tConfiguration for rule "${id}" is invalid:\n${err.message}`);
}
}

Expand Down
31 changes: 30 additions & 1 deletion tests/lib/config/config-validator.js
Expand Up @@ -71,10 +71,27 @@ function mockNoOptionsRule(context) {

mockNoOptionsRule.schema = [];

const mockRequiredOptionsRule = {
meta: {
schema: {
type: "array",
minItems: 1
}
},
create(context) {
return {
Program(node) {
context.report(node, "Expected a validation error.");
}
};
}
};

describe("Validator", () => {

beforeEach(() => {
eslint.defineRule("mock-rule", mockRule);
eslint.defineRule("mock-required-options-rule", mockRequiredOptionsRule);
});

describe("validate", () => {
Expand Down Expand Up @@ -103,6 +120,18 @@ describe("Validator", () => {
assert.doesNotThrow(fn);
});

it("should do nothing with an invalid config when severity is off", () => {
const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": "off" } }, "tests");

assert.doesNotThrow(fn);
});

it("should do nothing with an invalid config when severity is an array with 'off'", () => {
const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": ["off"] } }, "tests");

assert.doesNotThrow(fn);
});

it("should do nothing with a valid config when severity is warn", () => {
const fn = validator.validate.bind(null, { rules: { "mock-rule": ["warn", "second"] } }, "tests");

Expand Down Expand Up @@ -136,7 +165,7 @@ describe("Validator", () => {
it("should catch invalid rule options", () => {
const fn = validator.validate.bind(null, { rules: { "mock-rule": [3, "third"] } }, "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n\tValue \"third\" must be an enum value.\n");
assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n");
});

it("should allow for rules with no options", () => {
Expand Down

0 comments on commit abfd24f

Please sign in to comment.