Skip to content

Commit

Permalink
replace rulesMeta by getRuleMeta (refs eslint/rfcs#47)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed Feb 16, 2020
1 parent d29f613 commit 0d6afaf
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 30 deletions.
103 changes: 103 additions & 0 deletions lib/cli-engine/formatter-metadata.js
@@ -0,0 +1,103 @@
/**
* @fileoverview `FormatterMetadata` class.
*
* `FormatterMetadata` class is the type for the second parameter of formatters.
*
* @author Toru Nagashima <https://github.com/mysticatea>
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const builtInRules = require("../rules");
const { emitDeprecationWarning } = require("../shared/deprecation-warnings");
const { getCLIEngineInternalSlots } = require("./cli-engine");

/** @typedef {import("../shared/types").RuleMeta} RuleMeta */
/** @typedef {InstanceType<import("./cli-engine")["CLIEngine"]>} CLIEngine */

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* @typedef {Object} FormatterMetadataInternalSlots
* @property {CLIEngine} engine The CLIEngine instance for this formatting.
*/

/** @type {WeakMap<FormatterMetadata, FormatterMetadataInternalSlots>} */
const internalSlots = new WeakMap();

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

/**
* The class for the second parameter of formatters.
*/
class FormatterMetadata {

/**
* Initialize this instance.
* @param {CLIEngine} engine The CLIEngine instance.
*/
constructor(engine) {
internalSlots.set(this, { engine });

/*
* `rulesMeta` must be an instance property for backward compatibility.
* Otherwise, `JSON.stringify` ignores `rulesMeta`.
*/
Object.defineProperty(
this,
"rulesMeta",
Object.getOwnPropertyDescriptor(FormatterMetadata.prototype, "rulesMeta")
);
}

/**
* Get a rule meta.
* @param {string} ruleId The rule ID to get.
* @param {string} filePath The path to a target file to determine configuration.
* @returns {RuleMeta | undefined} The metadata of the rule.
*/
getRuleMeta(ruleId, filePath) {
const { engine } = internalSlots.get(this);

// To avoid using `getRulesForFile(filePath)` because it merges all rules into a map.
const { configArrayFactory } = getCLIEngineInternalSlots(engine);
const configArray =
configArrayFactory.getConfigArrayForFile(
filePath,
{ ignoreNotFoundError: true }
);
const rule =
configArray.pluginRules.get(ruleId) ||
builtInRules.get(ruleId);

return rule && rule.meta;
}

/**
* Get the metadata of all rules.
* @returns {Record<string, RuleMeta>} The metadata of rules.
* @deprecated
*/
get rulesMeta() {
emitDeprecationWarning("rulesMeta", "ESLINT_LEGACY_RULES_META");

const { engine } = internalSlots.get(this);
const rulesMeta = {};

for (const [ruleId, rule] of engine.getRules()) {
rulesMeta[ruleId] = rule.meta;
}

return rulesMeta;
}
}
Object.defineProperty(FormatterMetadata.prototype, "rulesMeta", { enumerable: true });

module.exports = { FormatterMetadata };
4 changes: 3 additions & 1 deletion lib/cli-engine/index.js
@@ -1,7 +1,9 @@
"use strict";

const { CLIEngine } = require("./cli-engine");
const { FormatterMetadata } = require("./formatter-metadata");

module.exports = {
CLIEngine
CLIEngine,
FormatterMetadata
};
15 changes: 2 additions & 13 deletions lib/cli.js
Expand Up @@ -17,7 +17,7 @@

const fs = require("fs"),
path = require("path"),
{ CLIEngine } = require("./cli-engine"),
{ CLIEngine, FormatterMetadata } = require("./cli-engine"),
options = require("./options"),
log = require("./shared/logging"),
RuntimeInfo = require("./shared/runtime-info");
Expand Down Expand Up @@ -83,7 +83,6 @@ function translateOptions(cliOptions) {
*/
function printResults(engine, results, format, outputFile) {
let formatter;
let rulesMeta;

try {
formatter = engine.getFormatter(format);
Expand All @@ -92,17 +91,7 @@ function printResults(engine, results, format, outputFile) {
return false;
}

const output = formatter(results, {
get rulesMeta() {
if (!rulesMeta) {
rulesMeta = {};
for (const [ruleId, rule] of engine.getRules()) {
rulesMeta[ruleId] = rule.meta;
}
}
return rulesMeta;
}
});
const output = formatter(results, new FormatterMetadata(engine));

if (output) {
if (outputFile) {
Expand Down
19 changes: 17 additions & 2 deletions lib/shared/deprecation-warnings.js
Expand Up @@ -31,7 +31,13 @@ const deprecationWarningMessages = {
"ESLint may use plugins that have the same name but different " +
"implementations in each target file. This method will be confused in " +
"such a case. " +
"Please use 'CLIEngine::getRulesForFile(filePath)' method instead."
"Please use 'CLIEngine::getRulesForFile(filePath)' method instead.",
ESLINT_LEGACY_RULES_META:
"'metadata.rulesMeta' property in formatters has been deprecated. " +
"ESLint may use plugins that have the same name but different " +
"implementations in each target file. This method will be confused in " +
"such a case. " +
"Please use 'metadata.getRuleMeta(ruleId, filePath)' method instead."
};

/**
Expand All @@ -53,10 +59,19 @@ const emitDeprecationWarning = lodash.memoize((source, errorCode) => {
);
}, (...args) => JSON.stringify(args));

/**
* Reset warning emission counts.
* @returns {void}
*/
function resetDeprecationWarning() {
emitDeprecationWarning.cache.clear();
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

module.exports = {
emitDeprecationWarning
emitDeprecationWarning,
resetDeprecationWarning
};
2 changes: 2 additions & 0 deletions tests/lib/cli-engine/cli-engine.js
Expand Up @@ -18,6 +18,7 @@ const assert = require("chai").assert,
os = require("os"),
hash = require("../../../lib/cli-engine/hash"),
{ CascadingConfigArrayFactory } = require("../../../lib/cli-engine/cascading-config-array-factory"),
{ resetDeprecationWarning } = require("../../../lib/shared/deprecation-warnings"),
{ unIndent } = require("../_utils"),
{ defineCLIEngineWithInMemoryFileSystem } = require("./_utils");

Expand Down Expand Up @@ -4553,6 +4554,7 @@ describe("CLIEngine", () => {

describe("getRules()", () => {
it("should emit deprecation warning on called.", async() => {
resetDeprecationWarning();
const warningListener = sinon.spy();

process.on("warning", warningListener);
Expand Down
51 changes: 37 additions & 14 deletions tests/lib/cli.js
Expand Up @@ -15,7 +15,8 @@
//------------------------------------------------------------------------------

const assert = require("chai").assert,
CLIEngine = require("../../lib/cli-engine/index").CLIEngine,
{ CLIEngine, FormatterMetadata } = require("../../lib/cli-engine"),
{ resetDeprecationWarning } = require("../../lib/shared/deprecation-warnings"),
path = require("path"),
sinon = require("sinon"),
fs = require("fs"),
Expand Down Expand Up @@ -59,7 +60,7 @@ describe("cli", () => {
sinon.stub(fakeCLIEngine.prototype, "getFormatter").returns(sinon.spy());

const localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -231,6 +232,28 @@ describe("cli", () => {
});

describe("when given a valid built-in formatter name that uses rules meta.", () => {
it("should emit deprecation warning on 'rulesMeta' is used.", async() => {
resetDeprecationWarning();
const warningListener = sinon.spy();

process.on("warning", warningListener);
try {
const filePath = getFixturePath("passing.js");

cli.execute(`-f json-with-metadata ${filePath} --no-eslintrc`);

// Wait for event emission.
await new Promise(resolve => setTimeout(resolve, 0));

// deprecated `rulesMeta` calls deprecated `getRules()` as well.
assert.strictEqual(warningListener.callCount, 2);
assert.strictEqual(warningListener.args[0][0].code, "ESLINT_LEGACY_RULES_META");
assert.strictEqual(warningListener.args[1][0].code, "ESLINT_LEGACY_GET_RULES");
} finally {
process.removeListener("warning", warningListener);
}
});

it("should execute without any errors", () => {
const filePath = getFixturePath("passing.js");
const exit = cli.execute(`-f json-with-metadata ${filePath} --no-eslintrc`);
Expand Down Expand Up @@ -811,7 +834,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.stub();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand All @@ -834,7 +857,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.stub();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -869,7 +892,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().once();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -907,7 +930,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().withExactArgs(report);

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -945,7 +968,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().withExactArgs(report);

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand All @@ -961,7 +984,7 @@ describe("cli", () => {
const fakeCLIEngine = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -995,7 +1018,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -1026,7 +1049,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.stub();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -1062,7 +1085,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -1100,7 +1123,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down Expand Up @@ -1137,7 +1160,7 @@ describe("cli", () => {
fakeCLIEngine.outputFixes = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand All @@ -1152,7 +1175,7 @@ describe("cli", () => {
const fakeCLIEngine = sinon.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine/index": { CLIEngine: fakeCLIEngine },
"./cli-engine/index": { CLIEngine: fakeCLIEngine, FormatterMetadata },
"./shared/logging": log
});

Expand Down

0 comments on commit 0d6afaf

Please sign in to comment.