Chore: Revert "avoid handling Rules instances in config-validator" (#…

…9295)

This reverts commit 7c95d5d to avoid the performance impact of iterating through all ~200 core rules every time Linter#verify is called.

lib/cli-engine.js

@@ -394,13 +394,9 @@ class CLIEngine {
             });
         }
 
-        if (this.options.rules && Object.keys(this.options.rules).length) {
-            const loadedRules = this.linter.getRules();
-
-            Object.keys(this.options.rules).filter(ruleId => loadedRules.has(ruleId)).forEach(name => {
-                validator.validateRuleOptions(loadedRules.get(name), name, this.options.rules[name], "CLI");
-            });
-        }
+        Object.keys(this.options.rules || {}).forEach(name => {
+            validator.validateRuleOptions(name, this.options.rules[name], "CLI", this.linter.rules);
+        });
 
         this.config = new Config(this.options, this.linter);
     }

lib/config/config-file.js

@@ -546,7 +546,7 @@ function loadFromDisk(resolvedPath, configContext) {
         }
 
         // validate the configuration before continuing
-        validator.validate(config, resolvedPath.configFullName, configContext.linterContext.getRules(), configContext.linterContext.environments);
+        validator.validate(config, resolvedPath.configFullName, configContext.linterContext.rules, configContext.linterContext.environments);
 
         /*
          * If an `extends` property is defined, it represents a configuration file to use as

lib/config/config-validator.js

@@ -14,7 +14,9 @@ const ajv = require("../util/ajv"),
     configSchema = require("../../conf/config-schema.js"),
     util = require("util");
 
-const ruleValidators = new WeakMap();
+const validators = {
+    rules: Object.create(null)
+};
 
 //------------------------------------------------------------------------------
 // Private
@@ -23,11 +25,13 @@ let validateSchema;
 
 /**
  * Gets a complete options schema for a rule.
- * @param {{create: Function, schema: (Array|null)}} rule A new-style rule object
+ * @param {string} id The rule's unique name.
+ * @param {Rules} rulesContext Rule context
  * @returns {Object} JSON Schema for the rule's options.
  */
-function getRuleOptionsSchema(rule) {
-    const schema = rule.schema || rule.meta && rule.meta.schema;
+function getRuleOptionsSchema(id, rulesContext) {
+    const rule = rulesContext.get(id),
+        schema = rule && rule.schema || rule && rule.meta && rule.meta.schema;
 
     // Given a tuple of schemas, insert warning level at the beginning
     if (Array.isArray(schema)) {
@@ -68,20 +72,19 @@ function validateRuleSeverity(options) {
 
 /**
 * Validates the non-severity options passed to a rule, based on its schema.
-* @param {{create: Function}} rule The rule to validate
+* @param {string} id The rule's unique name
 * @param {array} localOptions The options for the rule, excluding severity
+* @param {Rules} rulesContext Rule context
 * @returns {void}
 */
-function validateRuleSchema(rule, localOptions) {
-    if (!ruleValidators.has(rule)) {
-        const schema = getRuleOptionsSchema(rule);
+function validateRuleSchema(id, localOptions, rulesContext) {
+    const schema = getRuleOptionsSchema(id, rulesContext);
 
-        if (schema) {
-            ruleValidators.set(rule, ajv.compile(schema));
-        }
+    if (!validators.rules[id] && schema) {
+        validators.rules[id] = ajv.compile(schema);
     }
 
-    const validateRule = ruleValidators.get(rule);
+    const validateRule = validators.rules[id];
 
     if (validateRule) {
         validateRule(localOptions);
@@ -93,21 +96,21 @@ function validateRuleSchema(rule, localOptions) {
 
 /**
  * Validates a rule's options against its schema.
- * @param {{create: Function}} rule The rule that the config is being validated for
- * @param {string} ruleId The rule's unique name.
+ * @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 to report in any errors.
+ * @param {Rules} rulesContext Rule context
  * @returns {void}
  */
-function validateRuleOptions(rule, ruleId, options, source) {
+function validateRuleOptions(id, options, source, rulesContext) {
     try {
         const severity = validateRuleSeverity(options);
 
         if (severity !== 0 && !(typeof severity === "string" && severity.toLowerCase() === "off")) {
-            validateRuleSchema(rule, Array.isArray(options) ? options.slice(1) : []);
+            validateRuleSchema(id, Array.isArray(options) ? options.slice(1) : [], rulesContext);
         }
     } catch (err) {
-        throw new Error(`${source}:\n\tConfiguration for rule "${ruleId}" is invalid:\n${err.message}`);
+        throw new Error(`${source}:\n\tConfiguration for rule "${id}" is invalid:\n${err.message}`);
     }
 }
 
@@ -138,16 +141,16 @@ function validateEnvironment(environment, source, envContext) {
  * Validates a rules config object
  * @param {Object} rulesConfig The rules config object to validate.
  * @param {string} source The name of the configuration source to report in any errors.
- * @param {Map<string, {create: Function}>} rulesMap A map from strings to loaded rules
+ * @param {Rules} rulesContext Rule context
  * @returns {void}
  */
-function validateRules(rulesConfig, source, rulesMap) {
+function validateRules(rulesConfig, source, rulesContext) {
     if (!rulesConfig) {
         return;
     }
 
-    Object.keys(rulesConfig).filter(id => rulesMap.has(id)).forEach(id => {
-        validateRuleOptions(rulesMap.get(id), id, rulesConfig[id], source);
+    Object.keys(rulesConfig).forEach(id => {
+        validateRuleOptions(id, rulesConfig[id], source, rulesContext);
     });
 }
 
@@ -218,13 +221,13 @@ function validateConfigSchema(config, source) {
  * Validates an entire config object.
  * @param {Object} config The config object to validate.
  * @param {string} source The name of the configuration source to report in any errors.
- * @param {Map<string, {create: Function}>} ruleMap A map from rule IDs to defined rules
+ * @param {Rules} rulesContext The rules context
  * @param {Environments} envContext The env context
  * @returns {void}
  */
-function validate(config, source, ruleMap, envContext) {
+function validate(config, source, rulesContext, envContext) {
     validateConfigSchema(config, source);
-    validateRules(config.rules, source, ruleMap);
+    validateRules(config.rules, source, rulesContext);
     validateEnvironment(config.env, source, envContext);
 }
 

lib/linter.js

@@ -275,7 +275,7 @@ function createDisableDirectives(type, loc, value) {
  * @param {string} filename The file being checked.
  * @param {ASTNode} ast The top node of the AST.
  * @param {Object} config The existing configuration data.
- * @param {Map<string, {create: Function}>} ruleMap A map from rule IDs to defined rules
+ * @param {Linter} linterContext Linter context object
  * @returns {{
  *      config: Object,
  *      problems: Problem[],
@@ -288,9 +288,9 @@ function createDisableDirectives(type, loc, value) {
  * }} Modified config object, along with any problems encountered
  * while parsing config comments
  */
-function modifyConfigsFromComments(filename, ast, config, ruleMap) {
+function modifyConfigsFromComments(filename, ast, config, linterContext) {
 
-    const commentConfig = {
+    let commentConfig = {
         exported: {},
         astGlobals: {},
         rules: {},
@@ -338,9 +338,7 @@ function modifyConfigsFromComments(filename, ast, config, ruleMap) {
                             Object.keys(parseResult.config).forEach(name => {
                                 const ruleValue = parseResult.config[name];
 
-                                if (ruleMap.has(name)) {
-                                    validator.validateRuleOptions(ruleMap.get(name), name, ruleValue, `${filename} line ${comment.loc.start.line}`);
-                                }
+                                validator.validateRuleOptions(name, ruleValue, `${filename} line ${comment.loc.start.line}`, linterContext.rules);
                                 commentRules[name] = ruleValue;
                             });
                         } else {
@@ -362,6 +360,14 @@ function modifyConfigsFromComments(filename, ast, config, ruleMap) {
         }
     });
 
+    // apply environment configs
+    Object.keys(commentConfig.env).forEach(name => {
+        const env = linterContext.environments.get(name);
+
+        if (env) {
+            commentConfig = ConfigOps.merge(commentConfig, env);
+        }
+    });
     Object.assign(commentConfig.rules, commentRules);
 
     return {
@@ -801,7 +807,7 @@ module.exports = class Linter {
 
         // parse global comments and modify config
         if (allowInlineConfig !== false) {
-            const modifyConfigResult = modifyConfigsFromComments(filename, sourceCode.ast, config, this.getRules());
+            const modifyConfigResult = modifyConfigsFromComments(filename, sourceCode.ast, config, this);
 
             config = modifyConfigResult.config;
             modifyConfigResult.problems.forEach(problem => problems.push(problem));

lib/testers/rule-tester.js

@@ -319,7 +319,7 @@ class RuleTester {
                 }
             }));
 
-            const schema = validator.getRuleOptionsSchema(rule);
+            const schema = validator.getRuleOptionsSchema(ruleName, linter.rules);
 
             if (schema) {
                 ajv.validateSchema(schema);
@@ -335,7 +335,7 @@ class RuleTester {
                 }
             }
 
-            validator.validate(config, "rule-tester", linter.getRules(), new Environments());
+            validator.validate(config, "rule-tester", linter.rules, new Environments());
 
             /*
              * Setup AST getters.