Skip to content

Commit

Permalink
feat!: Set default schema: [], drop support for function-style rules (
Browse files Browse the repository at this point in the history
#139)

* drop support for function-style rules

* implement schema changes

* add unit tests for ConfigValidator#getRuleOptionsSchema

* add regression tests for ConfigValidator#validateRuleOptions

* add tests for error code

* update fixture rules to use object-style
  • Loading branch information
mdjermanovic committed Dec 26, 2023
1 parent 01db002 commit a6c240d
Show file tree
Hide file tree
Showing 8 changed files with 374 additions and 76 deletions.
19 changes: 3 additions & 16 deletions lib/config-array/config-array.js
Expand Up @@ -319,31 +319,18 @@ function createConfig(instance, indices) {
* @param {string} pluginId The plugin ID for prefix.
* @param {Record<string,T>} defs The definitions to collect.
* @param {Map<string, U>} map The map to output.
* @param {function(T): U} [normalize] The normalize function for each value.
* @returns {void}
*/
function collect(pluginId, defs, map, normalize) {
function collect(pluginId, defs, map) {
if (defs) {
const prefix = pluginId && `${pluginId}/`;

for (const [key, value] of Object.entries(defs)) {
map.set(
`${prefix}${key}`,
normalize ? normalize(value) : value
);
map.set(`${prefix}${key}`, value);
}
}
}

/**
* Normalize a rule definition.
* @param {Function|Rule} rule The rule definition to normalize.
* @returns {Rule} The normalized rule definition.
*/
function normalizePluginRule(rule) {
return typeof rule === "function" ? { create: rule } : rule;
}

/**
* Delete the mutation methods from a given map.
* @param {Map<any, any>} map The map object to delete.
Expand Down Expand Up @@ -385,7 +372,7 @@ function initPluginMemberMaps(elements, slots) {

collect(pluginId, plugin.environments, slots.envMap);
collect(pluginId, plugin.processors, slots.processorMap);
collect(pluginId, plugin.rules, slots.ruleMap, normalizePluginRule);
collect(pluginId, plugin.rules, slots.ruleMap);
}
}

Expand Down
81 changes: 63 additions & 18 deletions lib/shared/config-validator.js
Expand Up @@ -5,6 +5,12 @@

/* eslint class-methods-use-this: "off" */

//------------------------------------------------------------------------------
// Typedefs
//------------------------------------------------------------------------------

/** @typedef {import("../shared/types").Rule} Rule */

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -33,6 +39,13 @@ const severityMap = {

const validated = new WeakSet();

// JSON schema that disallows passing any options
const noOptionsSchema = Object.freeze({
type: "array",
minItems: 0,
maxItems: 0
});

//-----------------------------------------------------------------------------
// Exports
//-----------------------------------------------------------------------------
Expand All @@ -44,17 +57,36 @@ export default class ConfigValidator {

/**
* Gets a complete options schema for a rule.
* @param {{create: Function, schema: (Array|null)}} rule A new-style rule object
* @returns {Object} JSON Schema for the rule's options.
* @param {Rule} rule A rule object
* @throws {TypeError} If `meta.schema` is specified but is not an array, object or `false`.
* @returns {Object|null} JSON Schema for the rule's options.
* `null` if rule wasn't passed or its `meta.schema` is `false`.
*/
getRuleOptionsSchema(rule) {
if (!rule) {
return null;
}

const schema = rule.schema || rule.meta && rule.meta.schema;
if (!rule.meta) {
return { ...noOptionsSchema }; // default if `meta.schema` is not specified
}

// Given a tuple of schemas, insert warning level at the beginning
const schema = rule.meta.schema;

if (typeof schema === "undefined") {
return { ...noOptionsSchema }; // default if `meta.schema` is not specified
}

// `schema:false` is an allowed explicit opt-out of options validation for the rule
if (schema === false) {
return null;
}

if (typeof schema !== "object" || schema === null) {
throw new TypeError("Rule's `meta.schema` must be an array or object");
}

// ESLint-specific array form needs to be converted into a valid JSON Schema definition
if (Array.isArray(schema)) {
if (schema.length) {
return {
Expand All @@ -64,16 +96,13 @@ export default class ConfigValidator {
maxItems: schema.length
};
}
return {
type: "array",
minItems: 0,
maxItems: 0
};

// `schema:[]` is an explicit way to specify that the rule does not accept any options
return { ...noOptionsSchema };
}

// Given a full schema, leave it alone
return schema || null;
// `schema:<object>` is assumed to be a valid JSON Schema definition
return schema;
}

/**
Expand Down Expand Up @@ -101,10 +130,18 @@ export default class ConfigValidator {
*/
validateRuleSchema(rule, localOptions) {
if (!ruleValidators.has(rule)) {
const schema = this.getRuleOptionsSchema(rule);
try {
const schema = this.getRuleOptionsSchema(rule);

if (schema) {
ruleValidators.set(rule, ajv.compile(schema));
}
} catch (err) {
const errorWithCode = new Error(err.message, { cause: err });

if (schema) {
ruleValidators.set(rule, ajv.compile(schema));
errorWithCode.code = "ESLINT_INVALID_RULE_OPTIONS_SCHEMA";

throw errorWithCode;
}
}

Expand Down Expand Up @@ -137,13 +174,21 @@ export default class ConfigValidator {
this.validateRuleSchema(rule, Array.isArray(options) ? options.slice(1) : []);
}
} catch (err) {
const enhancedMessage = `Configuration for rule "${ruleId}" is invalid:\n${err.message}`;
let enhancedMessage = err.code === "ESLINT_INVALID_RULE_OPTIONS_SCHEMA"
? `Error while processing options validation schema of rule '${ruleId}': ${err.message}`
: `Configuration for rule "${ruleId}" is invalid:\n${err.message}`;

if (typeof source === "string") {
throw new Error(`${source}:\n\t${enhancedMessage}`);
} else {
throw new Error(enhancedMessage);
enhancedMessage = `${source}:\n\t${enhancedMessage}`;
}

const enhancedError = new Error(enhancedMessage, { cause: err });

if (err.code) {
enhancedError.code = err.code;
}

throw enhancedError;
}
}

Expand Down
24 changes: 13 additions & 11 deletions tests/fixtures/rules/custom-rule.cjs
@@ -1,15 +1,17 @@
module.exports = function(context) {
"use strict";

"use strict";
module.exports = {
meta: {
schema: []
},

return {
"Identifier": function(node) {
if (node.name === "foo") {
context.report(node, "Identifier cannot be named 'foo'.");
create(context) {
return {
"Identifier": function(node) {
if (node.name === "foo") {
context.report(node, "Identifier cannot be named 'foo'.");
}
}
}
};

};
}
};

module.exports.schema = [];
17 changes: 9 additions & 8 deletions tests/fixtures/rules/dir1/no-strings.cjs
@@ -1,14 +1,15 @@
"use strict";

module.exports = function(context) {
module.exports = {
create(context) {
return {

return {
"Literal": function(node) {
if (typeof node.value === 'string') {
context.report(node, "String!");
}

"Literal": function(node) {
if (typeof node.value === 'string') {
context.report(node, "String!");
}

}
};
};
}
};
15 changes: 8 additions & 7 deletions tests/fixtures/rules/dir2/no-literals.cjs
@@ -1,11 +1,12 @@
"use strict";

module.exports = function(context) {
module.exports = {
create (context) {
return {

return {

"Literal": function(node) {
context.report(node, "Literal!");
}
};
"Literal": function(node) {
context.report(node, "Literal!");
}
};
}
};
30 changes: 17 additions & 13 deletions tests/fixtures/rules/make-syntax-error-rule.cjs
@@ -1,14 +1,18 @@
module.exports = function(context) {
return {
Program: function(node) {
context.report({
node: node,
message: "ERROR",
fix: function(fixer) {
return fixer.insertTextAfter(node, "this is a syntax error.");
}
});
}
};
module.exports = {
meta: {
schema: []
},
create(context) {
return {
Program: function(node) {
context.report({
node: node,
message: "ERROR",
fix: function(fixer) {
return fixer.insertTextAfter(node, "this is a syntax error.");
}
});
}
};
}
};
module.exports.schema = [];
8 changes: 5 additions & 3 deletions tests/fixtures/rules/wrong/custom-rule.cjs
@@ -1,5 +1,7 @@
module.exports = function() {
"use strict";

"use strict";
return (null).something;
module.exports = {
create() {
return (null).something;
}
};

0 comments on commit a6c240d

Please sign in to comment.