Skip to content

Commit

Permalink
Breaking: disallow invalid rule defaults in RuleTester (fixes #11473) (…
Browse files Browse the repository at this point in the history 
…#11599)

* Breaking: disallow invalid rule defaults in RuleTester (fixes #11473)

* Update: Remove remaining invalid defaults from core rules
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Apr 9, 2019
1 parent c021117 commit ef7801e
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 58 deletions.
2 changes: 1 addition & 1 deletion lib/config/config-validator.js
View file
Expand Up @@ -10,12 +10,12 @@
//------------------------------------------------------------------------------

const path = require("path"),
ajv = require("../util/ajv"),
lodash = require("lodash"),
configSchema = require("../../conf/config-schema.js"),
util = require("util"),
ConfigOps = require("./config-ops");

const ajv = require("../util/ajv")();
const ruleValidators = new WeakMap();

//------------------------------------------------------------------------------
Expand Down
3 changes: 1 addition & 2 deletions lib/rules/line-comment-position.js
View file
Expand Up @@ -31,8 +31,7 @@ module.exports = {
type: "object",
properties: {
position: {
enum: ["above", "beside"],
default: "above"
enum: ["above", "beside"]
},
ignorePattern: {
type: "string"
Expand Down
40 changes: 16 additions & 24 deletions lib/rules/max-len.js
View file
Expand Up @@ -14,44 +14,36 @@ const OPTIONS_SCHEMA = {
properties: {
code: {
type: "integer",
minimum: 0,
default: 80
minimum: 0
},
comments: {
type: "integer",
minimum: 0
},
tabWidth: {
type: "integer",
minimum: 0,
default: 4
minimum: 0
},
ignorePattern: {
type: "string"
},
ignoreComments: {
type: "boolean",
default: false
type: "boolean"
},
ignoreStrings: {
type: "boolean",
default: false
type: "boolean"
},
ignoreUrls: {
type: "boolean",
default: false
type: "boolean"
},
ignoreTemplateLiterals: {
type: "boolean",
default: false
type: "boolean"
},
ignoreRegExpLiterals: {
type: "boolean",
default: false
type: "boolean"
},
ignoreTrailingComments: {
type: "boolean",
default: false
type: "boolean"
}
},
additionalProperties: false
Expand Down Expand Up @@ -141,14 +133,14 @@ module.exports = {
options.tabWidth = context.options[1];
}

const maxLength = options.code || 80,
tabWidth = options.tabWidth || 4,
ignoreComments = options.ignoreComments || false,
ignoreStrings = options.ignoreStrings || false,
ignoreTemplateLiterals = options.ignoreTemplateLiterals || false,
ignoreRegExpLiterals = options.ignoreRegExpLiterals || false,
ignoreTrailingComments = options.ignoreTrailingComments || options.ignoreComments || false,
ignoreUrls = options.ignoreUrls || false,
const maxLength = typeof options.code === "number" ? options.code : 80,
tabWidth = typeof options.tabWidth === "number" ? options.tabWidth : 4,
ignoreComments = !!options.ignoreComments,
ignoreStrings = !!options.ignoreStrings,
ignoreTemplateLiterals = !!options.ignoreTemplateLiterals,
ignoreRegExpLiterals = !!options.ignoreRegExpLiterals,
ignoreTrailingComments = !!options.ignoreTrailingComments || !!options.ignoreComments,
ignoreUrls = !!options.ignoreUrls,
maxCommentLength = options.comments;
let ignorePattern = options.ignorePattern || null;

Expand Down
20 changes: 8 additions & 12 deletions lib/rules/max-lines-per-function.js
View file
Expand Up @@ -19,20 +19,16 @@ const OPTIONS_SCHEMA = {
properties: {
max: {
type: "integer",
minimum: 0,
default: 50
minimum: 0
},
skipComments: {
type: "boolean",
default: false
type: "boolean"
},
skipBlankLines: {
type: "boolean",
default: false
type: "boolean"
},
IIFEs: {
type: "boolean",
default: false
type: "boolean"
}
},
additionalProperties: false
Expand Down Expand Up @@ -101,10 +97,10 @@ module.exports = {
let IIFEs = false;

if (typeof option === "object") {
maxLines = option.max;
skipComments = option.skipComments;
skipBlankLines = option.skipBlankLines;
IIFEs = option.IIFEs;
maxLines = typeof option.max === "number" ? option.max : 50;
skipComments = !!option.skipComments;
skipBlankLines = !!option.skipBlankLines;
IIFEs = !!option.IIFEs;
} else if (typeof option === "number") {
maxLines = option;
}
Expand Down
5 changes: 2 additions & 3 deletions lib/rules/one-var.js
View file
Expand Up @@ -32,8 +32,7 @@ module.exports = {
type: "object",
properties: {
separateRequires: {
type: "boolean",
default: false
type: "boolean"
},
var: {
enum: ["always", "never", "consecutive"]
Expand Down Expand Up @@ -77,7 +76,7 @@ module.exports = {
options.let = { uninitialized: mode, initialized: mode };
options.const = { uninitialized: mode, initialized: mode };
} else if (typeof mode === "object") { // options configuration is an object
options.separateRequires = mode.separateRequires;
options.separateRequires = !!mode.separateRequires;
options.var = { uninitialized: mode.var, initialized: mode.var };
options.let = { uninitialized: mode.let, initialized: mode.let };
options.const = { uninitialized: mode.const, initialized: mode.const };
Expand Down
15 changes: 14 additions & 1 deletion lib/testers/rule-tester.js
View file
Expand Up @@ -45,12 +45,13 @@ const lodash = require("lodash"),
path = require("path"),
util = require("util"),
validator = require("../config/config-validator"),
ajv = require("../util/ajv"),
Linter = require("../linter"),
Environments = require("../config/environments"),
SourceCodeFixer = require("../util/source-code-fixer"),
interpolate = require("../util/interpolate");

const ajv = require("../util/ajv")({ strictDefaults: true });

//------------------------------------------------------------------------------
// Private Members
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -383,6 +384,18 @@ class RuleTester {

throw new Error([`Schema for rule ${ruleName} is invalid:`, errors]);
}

/*
* `ajv.validateSchema` checks for errors in the structure of the schema (by comparing the schema against a "meta-schema"),
* and it reports those errors individually. However, there are other types of schema errors that only occur when compiling
* the schema (e.g. using invalid defaults in a schema), and only one of these errors can be reported at a time. As a result,
* the schema is compiled here separately from checking for `validateSchema` errors.
*/
try {
ajv.compile(schema);
} catch (err) {
throw new Error(`Schema for rule ${ruleName} is invalid: ${err.message}`);
}
}

validator.validate(config, ruleMap.get.bind(ruleMap), new Environments(), "rule-tester");
Expand Down
27 changes: 15 additions & 12 deletions lib/util/ajv.js
View file
Expand Up @@ -15,17 +15,20 @@ const Ajv = require("ajv"),
// Public Interface
//------------------------------------------------------------------------------

const ajv = new Ajv({
meta: false,
useDefaults: true,
validateSchema: false,
missingRefs: "ignore",
verbose: true,
schemaId: "auto"
});
module.exports = (additionalOptions = {}) => {
const ajv = new Ajv({
meta: false,
useDefaults: true,
validateSchema: false,
missingRefs: "ignore",
verbose: true,
schemaId: "auto",
...additionalOptions
});

ajv.addMetaSchema(metaSchema);
// eslint-disable-next-line no-underscore-dangle
ajv._opts.defaultMeta = metaSchema.id;
ajv.addMetaSchema(metaSchema);
// eslint-disable-next-line no-underscore-dangle
ajv._opts.defaultMeta = metaSchema.id;

module.exports = ajv;
return ajv;
};
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -36,7 +36,7 @@
"bugs": "https://github.com/eslint/eslint/issues/",
"dependencies": {
"@babel/code-frame": "^7.0.0",
"ajv": "^6.9.1",
"ajv": "^6.10.0",
"chalk": "^2.1.0",
"cross-spawn": "^6.0.5",
"debug": "^4.0.1",
Expand Down
5 changes: 3 additions & 2 deletions tests/conf/config-schema.js
View file
Expand Up @@ -9,10 +9,11 @@
// Requirements
//------------------------------------------------------------------------------

const ajv = require("../../lib/util/ajv"),
configSchema = require("../../conf/config-schema.js"),
const configSchema = require("../../conf/config-schema.js"),
assert = require("assert");

const ajv = require("../../lib/util/ajv")();

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------
Expand Down
19 changes: 19 additions & 0 deletions tests/lib/rules/max-len.js
View file
Expand Up @@ -184,6 +184,11 @@ ruleTester.run("max-len", rule, {
{
code: "var longNameLongName = '𝌆𝌆'",
options: [5, { ignorePattern: "𝌆{2}" }]
},

{
code: "\tfoo",
options: [4, { tabWidth: 0 }]
}
],

Expand Down Expand Up @@ -625,6 +630,20 @@ ruleTester.run("max-len", rule, {
column: 1
}
]
},

{
code: "a",
options: [0],
errors: [
{
messageId: "max",
data: { lineNumber: 1, maxLength: 0 },
type: "Program",
line: 1,
column: 1
}
]
}
]
});
9 changes: 9 additions & 0 deletions tests/lib/rules/max-lines-per-function.js
View file
Expand Up @@ -205,6 +205,15 @@ if ( x === y ) {
]
},

// Test that option defaults work as expected
{
code: `() => {${"foo\n".repeat(60)}}`,
options: [{}],
errors: [
{ messageId: "exceed", data: { name: "arrow function", lineCount: 61, maxLines: 50 } }
]
},

// Test skipBlankLines: false
{
code: "function name() {\nvar x = 5;\n\t\n \n\nvar x = 2;\n}",
Expand Down
37 changes: 37 additions & 0 deletions tests/lib/testers/rule-tester.js
View file
Expand Up @@ -659,6 +659,43 @@ describe("RuleTester", () => {

});

it("should disallow invalid defaults in rules", () => {
const ruleWithInvalidDefaults = {
meta: {
schema: [
{
oneOf: [
{ enum: ["foo"] },
{
type: "object",
properties: {
foo: {
enum: ["foo", "bar"],
default: "foo"
}
},
additionalProperties: false
}
]
}
]
},
create: () => ({})
};

assert.throws(() => {
ruleTester.run("invalid-defaults", ruleWithInvalidDefaults, {
valid: [
{
code: "foo",
options: [{}]
}
],
invalid: []
});
}, /Schema for rule invalid-defaults is invalid: default is ignored for: data1\.foo/u);
});

it("throw an error when an unknown config option is included", () => {
assert.throws(() => {
ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), {
Expand Down

0 comments on commit ef7801e

Please sign in to comment.