Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: remove undocumented `Linter#rules` property (refs #9161) #11335

Merged
merged 3 commits into from Jan 30, 2019
Merged
Diff settings

Always

Just for now

Copy path View file
@@ -117,27 +117,6 @@ function fileType(extension) {
};
}

/**
* Generates a static file that includes each rule by name rather than dynamically
* looking up based on directory. This is used for the browser version of ESLint.
* @param {string} basedir The directory in which to look for code.
* @returns {void}
*/
function generateRulesIndex(basedir) {
let output = "module.exports = function() {\n";

output += " var rules = Object.create(null);\n";

find(`${basedir}rules/`).filter(fileType("js")).forEach(filename => {
const basename = path.basename(filename, ".js");

output += ` rules["${basename}"] = require("./rules/${basename}");\n`;
});

output += "\n return rules;\n};";
output.to(`${basedir}load-rules.js`);
}

/**
* Executes a command and returns the output instead of printing it to stdout.
* @param {string} cmd The command string to execute.
@@ -841,17 +820,8 @@ target.browserify = function() {
mkdir(BUILD_DIR);
}

// 2. copy files into temp directory
cp("-r", "lib/*", TEMP_DIR);

// 3. delete the load-rules.js file
rm("-rf", `${TEMP_DIR}load-rules.js`);

// 4. create new load-rule.js with hardcoded requires
generateRulesIndex(TEMP_DIR);

// 5. browserify the temp directory
This conversation was marked as resolved by not-an-aardvark

This comment has been minimized.

Copy link
@platinumazure

platinumazure Jan 30, 2019

Member

Could these comments be renumbered?

exec(`${getBinFile("browserify")} -x espree ${TEMP_DIR}linter.js -o ${BUILD_DIR}eslint.js -s eslint --global-transform [ babelify --presets [ es2015 ] ]`);
exec(`${getBinFile("browserify")} -x espree lib/linter.js -o ${BUILD_DIR}eslint.js -s eslint --global-transform [ babelify --presets [ es2015 ] ]`);

// 6. Browserify espree
exec(`${getBinFile("browserify")} -r espree -o ${TEMP_DIR}espree.js --global-transform [ babelify --presets [ es2015 ] ]`);
Copy path View file
@@ -29,7 +29,8 @@ const fs = require("fs"),
hash = require("./util/hash"),
ModuleResolver = require("./util/module-resolver"),
naming = require("./util/naming"),
pkg = require("../package.json");
pkg = require("../package.json"),
loadRules = require("./load-rules");

const debug = require("debug")("eslint:cli-engine");
const resolver = new ModuleResolver();
@@ -447,7 +448,7 @@ class CLIEngine {

this.options.rulePaths.forEach(rulesdir => {
debug(`Loading rules from ${rulesdir}`);
this.linter.rules.load(rulesdir, cwd);
this.linter.defineRules(loadRules(rulesdir, cwd));
});
}

Copy path View file
@@ -71,7 +71,7 @@ class Config {
const options = providedOptions || {};

this.linterContext = linterContext;
this.plugins = new Plugins(linterContext.environments, linterContext.rules);
this.plugins = new Plugins(linterContext.environments, linterContext.defineRule.bind(linterContext));

this.options = options;
this.ignore = options.ignore;
Copy path View file
@@ -24,12 +24,12 @@ class Plugins {
/**
* Creates the plugins context
* @param {Environments} envContext - env context
* @param {Rules} rulesContext - rules context
* @param {function(string, Rule): void} defineRule - Callback for when a plugin is defined which introduces rules
*/
constructor(envContext, rulesContext) {
constructor(envContext, defineRule) {
this._plugins = Object.create(null);
this._environments = envContext;
this._rules = rulesContext;
this._defineRule = defineRule;
}

/**
@@ -45,7 +45,15 @@ class Plugins {
// load up environments and rules
this._plugins[shortName] = plugin;
this._environments.importPlugin(plugin, shortName);
this._rules.importPlugin(plugin, shortName);

if (plugin.rules) {
Object.keys(plugin.rules).forEach(ruleId => {
const qualifiedRuleId = `${shortName}/${ruleId}`,
rule = plugin.rules[ruleId];

this._defineRule(qualifiedRuleId, rule);
});
}
}

/**
Copy path View file
@@ -758,6 +758,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser

const lastSourceCodes = new WeakMap();
const loadedParserMaps = new WeakMap();
const ruleMaps = new WeakMap();

//------------------------------------------------------------------------------
// Public Interface
@@ -772,9 +773,8 @@ module.exports = class Linter {
constructor() {
lastSourceCodes.set(this, null);
loadedParserMaps.set(this, new Map());
ruleMaps.set(this, new Rules());
this.version = pkg.version;

this.rules = new Rules();
this.environments = new Environments();
}

@@ -873,7 +873,7 @@ module.exports = class Linter {

const sourceCode = lastSourceCodes.get(this);
const commentDirectives = options.allowInlineConfig
? getDirectiveComments(options.filename, sourceCode.ast, ruleId => this.rules.get(ruleId))
? getDirectiveComments(options.filename, sourceCode.ast, ruleId => ruleMaps.get(this).get(ruleId))
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };

// augment global scope with declared global variables
@@ -891,7 +891,7 @@ module.exports = class Linter {
lintingProblems = runRules(
sourceCode,
configuredRules,
ruleId => this.rules.get(ruleId),
ruleId => ruleMaps.get(this).get(ruleId),
parserOptions,
parserName,
settings,
@@ -957,7 +957,7 @@ module.exports = class Linter {
* @returns {void}
*/
defineRule(ruleId, ruleModule) {
this.rules.define(ruleId, ruleModule);
ruleMaps.get(this).define(ruleId, ruleModule);
}

/**
@@ -976,7 +976,7 @@ module.exports = class Linter {
* @returns {Map} All loaded rules
*/
getRules() {
return this.rules.getAllLoadedRules();
return ruleMaps.get(this).getAllLoadedRules();
}

/**
Copy path View file
@@ -10,7 +10,6 @@
//------------------------------------------------------------------------------

const lodash = require("lodash");
const loadRules = require("./load-rules");
const ruleReplacements = require("../conf/replacements").rules;
const builtInRules = require("./built-in-rules-index");

@@ -60,7 +59,9 @@ function normalizeRule(rule) {
class Rules {
constructor() {
this._rules = Object.create(null);
this.defineAll(builtInRules);
Object.keys(builtInRules).forEach(ruleId => {
this.define(ruleId, builtInRules[ruleId]);
});
}

/**
@@ -73,46 +74,6 @@ class Rules {
this._rules[ruleId] = normalizeRule(ruleModule);
}

/**
* Loads and registers all rules from passed rules directory.
* @param {string} [rulesDir] Path to rules directory, may be relative. Defaults to `lib/rules`.
* @param {string} cwd Current working directory
* @returns {void}
*/
load(rulesDir, cwd) {
const newRules = loadRules(rulesDir, cwd);

this.defineAll(newRules);
}

/**
* Pulls a Map of new rules to the defined ones of this instance.
* @param {Object} newRules Expects to have an object here that maps the rule ID to the rule definition.
* @returns {void}
*/
defineAll(newRules) {
Object.keys(newRules).forEach(ruleId => {
this.define(ruleId, newRules[ruleId]);
});
}

/**
* Registers all given rules of a plugin.
* @param {Object} plugin The plugin object to import.
* @param {string} pluginName The name of the plugin without prefix (`eslint-plugin-`).
* @returns {void}
*/
importPlugin(plugin, pluginName) {
if (plugin.rules) {
Object.keys(plugin.rules).forEach(ruleId => {
const qualifiedRuleId = `${pluginName}/${ruleId}`,
rule = plugin.rules[ruleId];

this.define(qualifiedRuleId, rule);
});
}
}

/**
* Access rule handler by id (file name).
* @param {string} ruleId Rule id (file name).
@@ -11,7 +11,8 @@

const assert = require("chai").assert,
Linter = require("../../../lib/linter"),
validator = require("../../../lib/config/config-validator");
validator = require("../../../lib/config/config-validator"),
Rules = require("../../../lib/rules");
const linter = new Linter();

//------------------------------------------------------------------------------
@@ -96,7 +97,7 @@ describe("Validator", () => {
* @returns {{create: Function}} The loaded rule
*/
function ruleMapper(ruleId) {
return linter.getRules().get(ruleId);
return linter.getRules().get(ruleId) || new Rules().get(ruleId);
}

beforeEach(() => {
@@ -403,12 +404,12 @@ describe("Validator", () => {
describe("getRuleOptionsSchema", () => {

it("should return null for a missing rule", () => {
assert.strictEqual(validator.getRuleOptionsSchema(linter.rules.get("non-existent-rule")), null);
assert.strictEqual(validator.getRuleOptionsSchema(ruleMapper("non-existent-rule")), null);
});

it("should not modify object schema", () => {
linter.defineRule("mock-object-rule", mockObjectRule);
assert.deepStrictEqual(validator.getRuleOptionsSchema(linter.rules.get("mock-object-rule")), {
assert.deepStrictEqual(validator.getRuleOptionsSchema(ruleMapper("mock-object-rule")), {
enum: ["first", "second"]
});
});
@@ -418,43 +419,43 @@ describe("Validator", () => {
describe("validateRuleOptions", () => {

it("should throw for incorrect warning level number", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("mock-rule"), "mock-rule", 3, "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "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 = warn, 2 = error (you passed '3').\n");
});

it("should throw for incorrect warning level string", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("mock-rule"), "mock-rule", "booya", "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "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 = warn, 2 = error (you passed '\"booya\"').\n");
});

it("should throw for invalid-type warning level", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("mock-rule"), "mock-rule", [["error"]], "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "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 = warn, 2 = error (you passed '[ \"error\" ]').\n");
});

it("should only check warning level for nonexistent rules", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("non-existent-rule"), "non-existent-rule", [3, "foobar"], "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("non-existent-rule"), "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 = warn, 2 = error (you passed '3').\n");
});

it("should only check warning level for plugin rules", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("plugin/rule"), "plugin/rule", 3, "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("plugin/rule"), "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 = warn, 2 = error (you passed '3').\n");
});

it("should throw for incorrect configuration values", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("mock-rule"), "mock-rule", [2, "frist"], "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "mock-rule", [2, "frist"], "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tValue \"frist\" should be equal to one of the allowed values.\n");
});

it("should throw for too many configuration values", () => {
const fn = validator.validateRuleOptions.bind(null, linter.rules.get("mock-rule"), "mock-rule", [2, "first", "second"], "tests");
const fn = validator.validateRuleOptions.bind(null, ruleMapper("mock-rule"), "mock-rule", [2, "first", "second"], "tests");

assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tValue [\"first\",\"second\"] should NOT have more than 1 items.\n");
});
Copy path View file
@@ -10,7 +10,6 @@

const assert = require("chai").assert,
Plugins = require("../../../lib/config/plugins"),
Rules = require("../../../lib/rules"),
Environments = require("../../../lib/config/environments");

const proxyquire = require("proxyquire").noCallThru().noPreserveCache();
@@ -24,7 +23,7 @@ describe("Plugins", () => {
describe("get", () => {

it("should return null when plugin doesn't exist", () => {
assert.isNull((new Plugins(new Environments(), new Rules())).get("foo"));
assert.isNull((new Plugins(new Environments(), () => {})).get("foo"));
});
});

@@ -39,8 +38,8 @@ describe("Plugins", () => {
beforeEach(() => {
plugin = {};
scopedPlugin = {};
rules = new Rules();
environments = new Environments();
rules = new Map();
StubbedPlugins = new (proxyquire("../../../lib/config/plugins", {
"eslint-plugin-example": plugin,
"@scope/eslint-plugin-example": scopedPlugin,
@@ -49,7 +48,7 @@ describe("Plugins", () => {
throw new Error("error thrown while loading this module");
}
}
}))(environments, rules);
}))(environments, rules.set.bind(rules));
});

it("should load a plugin when referenced by short name", () => {
@@ -180,7 +179,7 @@ describe("Plugins", () => {
};
StubbedPlugins.load("@scope/eslint-plugin-example");

assert.isFalse(rules.getAllLoadedRules().has("example/foo"));
assert.isFalse(rules.has("example/foo"));
});
});
});
@@ -189,17 +188,18 @@ describe("Plugins", () => {

let StubbedPlugins,
plugin1,
plugin2;
const rules = new Rules(),
environments = new Environments();
plugin2,
rules;
const environments = new Environments();

beforeEach(() => {
plugin1 = {};
plugin2 = {};
rules = new Map();
StubbedPlugins = new (proxyquire("../../../lib/config/plugins", {
"eslint-plugin-example1": plugin1,
"eslint-plugin-example2": plugin2
}))(environments, rules);
}))(environments, rules.set.bind(rules));
});

it("should load plugins when passed multiple plugins", () => {
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.