Skip to content

Commit

Permalink
Fix: validate the type of severity level (fixes #5499)
Browse files Browse the repository at this point in the history
Currently, for example, [ "error" ] is also regarded as a valid severity. This is a totally unexpected behavior.
  • Loading branch information
shinnn committed Mar 11, 2016
1 parent d727f17 commit 641b3f7
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
13 changes: 10 additions & 3 deletions lib/config/config-validator.js
Expand Up @@ -13,7 +13,8 @@

var rules = require("../rules"),
Environments = require("./environments"),
schemaValidator = require("is-my-json-valid");
schemaValidator = require("is-my-json-valid"),
util = require("util");

var validators = {
rules: Object.create(null)
Expand Down Expand Up @@ -83,7 +84,10 @@ function validateRuleOptions(id, options, source) {
localOptions = [];
}

validSeverity = (severity === 0 || severity === 1 || severity === 2 || /^(?:off|warn|error)$/i.test(severity));
validSeverity = (
severity === 0 || severity === 1 || severity === 2 ||
(typeof severity === "string" && /^(?:off|warn|error)$/i.test(severity))
);

if (validateRule) {
validateRule(localOptions);
Expand All @@ -97,7 +101,10 @@ function validateRuleOptions(id, options, source) {

if (!validSeverity) {
message.push(
"\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed \"", severity, "\").\n");
"\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed '",
util.inspect(severity).replace(/'/g, "\"").replace(/\n/g, ""),
"').\n"
);
}

if (validateRule && validateRule.errors) {
Expand Down
20 changes: 13 additions & 7 deletions tests/lib/config/config-validator.js
Expand Up @@ -138,7 +138,7 @@ describe("Validator", function() {
it("should catch invalid rule options", function() {
var 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 = warning, 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 = warning, 2 = error (you passed '3').\n\tValue \"third\" must be an enum value.\n");
});

it("should allow for rules with no options", function() {
Expand Down Expand Up @@ -201,28 +201,34 @@ describe("Validator", function() {

describe("validateRuleOptions", function() {

it("should throw for incorrect warning level", function() {
it("should throw for incorrect warning level number", function() {
var fn = validator.validateRuleOptions.bind(null, "mock-rule", 3, "tests");

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

it("should throw for incorrect warning level", function() {
it("should throw for incorrect warning level string", function() {
var fn = validator.validateRuleOptions.bind(null, "mock-rule", "booya", "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed \"booya\").\n");
assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed '\"booya\"').\n");
});

it("should throw for invalid-type warning level", function() {
var fn = validator.validateRuleOptions.bind(null, "mock-rule", [["error"]], "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed '[ \"error\" ]').\n");
});

it("should only check warning level for nonexistent rules", function() {
var fn = validator.validateRuleOptions.bind(null, "non-existent-rule", [3, "foobar"], "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"non-existent-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed \"3\").\n");
assert.throws(fn, "tests:\n\tConfiguration for rule \"non-existent-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed '3').\n");
});

it("should only check warning level for plugin rules", function() {
var fn = validator.validateRuleOptions.bind(null, "plugin/rule", 3, "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"plugin/rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed \"3\").\n");
assert.throws(fn, "tests:\n\tConfiguration for rule \"plugin/rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed '3').\n");
});

it("should throw for incorrect configuration values", function() {
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/eslint.js
Expand Up @@ -1701,7 +1701,7 @@ describe("eslint", function() {
var config = { rules: {} };

var fn = eslint.verify.bind(eslint, code, config, filename);
assert.throws(fn, "filename.js line 1:\n\tConfiguration for rule \"no-alert\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed \"true\").\n");
assert.throws(fn, "filename.js line 1:\n\tConfiguration for rule \"no-alert\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warning, 2 = error (you passed 'true').\n");
});
});

Expand Down

0 comments on commit 641b3f7

Please sign in to comment.