From c71e75a4b4836a0d718e6cb35c5b475317c50666 Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Sun, 24 Mar 2019 20:14:49 -0700 Subject: [PATCH 1/6] Update: pass rule meta to formatters (RFC #10) --- lib/cli.js | 10 ++- lib/formatters/html-template-message.html | 2 +- lib/formatters/html.js | 10 ++- lib/formatters/json-with-metadata.js | 16 +++++ tests/lib/formatters/json-with-metadata.js | 79 ++++++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 lib/formatters/json-with-metadata.js create mode 100644 tests/lib/formatters/json-with-metadata.js diff --git a/lib/cli.js b/lib/cli.js index f67eb7274ff..9ce81e55425 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -81,15 +81,23 @@ function translateOptions(cliOptions) { */ function printResults(engine, results, format, outputFile) { let formatter; + let rules; try { formatter = engine.getFormatter(format); + rules = engine.getRules(); } catch (e) { log.error(e.message); return false; } - const output = formatter(results); + const rulesMeta = {}; + + rules.forEach((rule, ruleId) => { + rulesMeta[ruleId] = rule.meta; + }); + + const output = formatter(results, { rulesMeta }); if (output) { if (outputFile) { diff --git a/lib/formatters/html-template-message.html b/lib/formatters/html-template-message.html index bc353502050..93795a1bdc8 100644 --- a/lib/formatters/html-template-message.html +++ b/lib/formatters/html-template-message.html @@ -3,6 +3,6 @@ <%= severityName %> <%- message %> - <%= ruleId %> + <%= ruleId %> diff --git a/lib/formatters/html.js b/lib/formatters/html.js index d450f9dee24..5d218da6f07 100644 --- a/lib/formatters/html.js +++ b/lib/formatters/html.js @@ -15,6 +15,7 @@ const path = require("path"); const pageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-page.html"), "utf-8")); const messageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-message.html"), "utf-8")); const resultTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-result.html"), "utf-8")); +let rulesMetadata; /** * Given a word and a count, append an s if count is not one. @@ -74,6 +75,8 @@ function renderMessages(messages, parentIndex) { return lodash.map(messages, message => { const lineNumber = message.line || 0; const columnNumber = message.column || 0; + const meta = rulesMetadata[message.ruleId]; + const ruleUrl = lodash.get(meta, "docs.url", null); return messageTemplate({ parentIndex, @@ -82,7 +85,8 @@ function renderMessages(messages, parentIndex) { severityNumber: message.severity, severityName: message.severity === 1 ? "Warning" : "Error", message: message.message, - ruleId: message.ruleId + ruleId: message.ruleId, + ruleUrl: ruleUrl !== null ? ruleUrl : "https://eslint.org/docs/rules" }); }).join("\n"); } @@ -105,13 +109,15 @@ function renderResults(results) { // Public Interface //------------------------------------------------------------------------------ -module.exports = function(results) { +module.exports = function(results, data) { let totalErrors, totalWarnings; totalErrors = 0; totalWarnings = 0; + rulesMetadata = data.rulesMetadata; + // Iterate over results to get totals results.forEach(result => { totalErrors += result.errorCount; diff --git a/lib/formatters/json-with-metadata.js b/lib/formatters/json-with-metadata.js new file mode 100644 index 00000000000..949e767da3d --- /dev/null +++ b/lib/formatters/json-with-metadata.js @@ -0,0 +1,16 @@ +/** + * @fileoverview JSON reporter + * @author Burak Yigit Kaya aka BYK + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +module.exports = function(results, data) { + return JSON.stringify({ + results, + rulesMeta: data.rulesMeta + }); +}; diff --git a/tests/lib/formatters/json-with-metadata.js b/tests/lib/formatters/json-with-metadata.js new file mode 100644 index 00000000000..79775b3f8de --- /dev/null +++ b/tests/lib/formatters/json-with-metadata.js @@ -0,0 +1,79 @@ +/** + * @fileoverview Tests for JSON reporter. + * @author Chris Meyer + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const assert = require("chai").assert, + formatter = require("../../../lib/formatters/json-with-metadata"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +describe("formatter:json", () => { + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + }, + bar: { + type: "suggestion", + + docs: { + description: "This is rule 'bar'", + category: "error", + recommended: false + }, + + messages: { + message1: "This is a message for rule 'bar'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + messages: [{ + message: "Unexpected foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo" + }] + }, { + filePath: "bar.js", + messages: [{ + message: "Unexpected bar.", + severity: 1, + line: 6, + column: 11, + ruleId: "bar" + }] + }], + rulesMeta + }; + + it("should return passed results and data as a JSON string without any modification", () => { + const result = JSON.parse(formatter(code.results, { rulesMeta })); + + assert.deepStrictEqual(result, code); + }); +}); From 6543f0138ae72d042d591b5057b7fe1b6c478c81 Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Sun, 24 Mar 2019 21:32:42 -0700 Subject: [PATCH 2/6] Fix html formatter & unit tests --- lib/formatters/html.js | 13 +- tests/lib/formatters/html.js | 513 ++++++++++++++++++++++++----------- 2 files changed, 367 insertions(+), 159 deletions(-) diff --git a/lib/formatters/html.js b/lib/formatters/html.js index 5d218da6f07..f2639de109a 100644 --- a/lib/formatters/html.js +++ b/lib/formatters/html.js @@ -15,7 +15,7 @@ const path = require("path"); const pageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-page.html"), "utf-8")); const messageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-message.html"), "utf-8")); const resultTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-result.html"), "utf-8")); -let rulesMetadata; +let rulesMeta; /** * Given a word and a count, append an s if count is not one. @@ -75,8 +75,13 @@ function renderMessages(messages, parentIndex) { return lodash.map(messages, message => { const lineNumber = message.line || 0; const columnNumber = message.column || 0; - const meta = rulesMetadata[message.ruleId]; - const ruleUrl = lodash.get(meta, "docs.url", null); + let ruleUrl; + + if (rulesMeta) { + const meta = rulesMeta[message.ruleId]; + + ruleUrl = lodash.get(meta, "docs.url", null); + } return messageTemplate({ parentIndex, @@ -116,7 +121,7 @@ module.exports = function(results, data) { totalErrors = 0; totalWarnings = 0; - rulesMetadata = data.rulesMetadata; + rulesMeta = data.rulesMeta; // Iterate over results to get totals results.forEach(result => { diff --git a/tests/lib/formatters/html.js b/tests/lib/formatters/html.js index fc76796cb56..ab829234e99 100644 --- a/tests/lib/formatters/html.js +++ b/tests/lib/formatters/html.js @@ -67,23 +67,43 @@ function checkContentRow($, rowObject, args) { describe("formatter:html", () => { describe("when passed a single error message", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 0, - messages: [{ - message: "Unexpected foo.", - severity: 2, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 0, + messages: [{ + message: "Unexpected foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 1 issue in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -98,23 +118,43 @@ describe("formatter:html", () => { }); describe("when passed a single warning message", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 0, - warningCount: 1, - messages: [{ - message: "Unexpected foo.", - severity: 1, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 0, + warningCount: 1, + messages: [{ + message: "Unexpected foo.", + severity: 1, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 1 issue in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -129,23 +169,43 @@ describe("formatter:html", () => { }); describe("when passed a single error message", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 0, - messages: [{ - message: "Unexpected foo.", - severity: 2, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 0, + messages: [{ + message: "Unexpected foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 1 issue in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -160,16 +220,17 @@ describe("formatter:html", () => { }); describe("when passed no error/warning messages", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 0, - warningCount: 0, - messages: [] - }]; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 0, + warningCount: 0, + messages: [] + }] + }; it("should return a string in HTML format with 0 issues in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, {}); const $ = cheerio.load(result); // Check overview @@ -182,29 +243,63 @@ describe("formatter:html", () => { }); describe("when passed multiple messages", () => { - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 1, - messages: [{ - message: "Unexpected foo.", - severity: 2, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }, { - message: "Unexpected bar.", - severity: 1, - line: 6, - column: 11, - ruleId: "bar", - source: "bar" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + }, + bar: { + type: "suggestion", + + docs: { + description: "This is rule 'bar'", + category: "error", + recommended: false + }, + + messages: { + message1: "This is a message for rule 'bar'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 1, + messages: [{ + message: "Unexpected foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }, { + message: "Unexpected bar.", + severity: 1, + line: 6, + column: 11, + ruleId: "bar", + source: "bar" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 2 issues in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -220,34 +315,68 @@ describe("formatter:html", () => { }); describe("when passed multiple files with 1 error & warning message respectively", () => { - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 0, - messages: [{ - message: "Unexpected foo.", - severity: 2, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }, { - filePath: "bar.js", - errorCount: 0, - warningCount: 1, - messages: [{ - message: "Unexpected bar.", - severity: 1, - line: 6, - column: 11, - ruleId: "bar", - source: "bar" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + }, + bar: { + type: "suggestion", + + docs: { + description: "This is rule 'bar'", + category: "error", + recommended: false + }, + + messages: { + message1: "This is a message for rule 'bar'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 0, + messages: [{ + message: "Unexpected foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }, { + filePath: "bar.js", + errorCount: 0, + warningCount: 1, + messages: [{ + message: "Unexpected bar.", + severity: 1, + line: 6, + column: 11, + ruleId: "bar", + source: "bar" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 2 issues in 2 files and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -264,34 +393,68 @@ describe("formatter:html", () => { }); describe("when passed multiple files with 1 warning message each", () => { - const code = [{ - filePath: "foo.js", - errorCount: 0, - warningCount: 1, - messages: [{ - message: "Unexpected foo.", - severity: 1, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }, { - filePath: "bar.js", - errorCount: 0, - warningCount: 1, - messages: [{ - message: "Unexpected bar.", - severity: 1, - line: 6, - column: 11, - ruleId: "bar", - source: "bar" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + }, + bar: { + type: "suggestion", + + docs: { + description: "This is rule 'bar'", + category: "error", + recommended: false + }, + + messages: { + message1: "This is a message for rule 'bar'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 0, + warningCount: 1, + messages: [{ + message: "Unexpected foo.", + severity: 1, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }, { + filePath: "bar.js", + errorCount: 0, + warningCount: 1, + messages: [{ + message: "Unexpected bar.", + severity: 1, + line: 6, + column: 11, + ruleId: "bar", + source: "bar" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 2 issues in 2 files and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); // Check overview @@ -308,23 +471,43 @@ describe("formatter:html", () => { }); describe("when passing a single message with illegal characters", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 0, - messages: [{ - message: "Unexpected <&\"'> foo.", - severity: 2, - line: 5, - column: 10, - ruleId: "foo", - source: "foo" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 0, + messages: [{ + message: "Unexpected <&\"'> foo.", + severity: 2, + line: 5, + column: 10, + ruleId: "foo", + source: "foo" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 1 issue in 1 file", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); checkContentRow($, $("tr")[1], { group: "f-0", lineCol: "5:10", color: "clr-2", message: "Unexpected <&"'> foo.", ruleId: "foo" }); @@ -344,7 +527,7 @@ describe("formatter:html", () => { }]; it("should return a string in HTML format with 1 issue in 1 file", () => { - const result = formatter(code); + const result = formatter(code, {}); const $ = cheerio.load(result); checkContentRow($, $("tr")[1], { group: "f-0", lineCol: "5:10", color: "clr-2", message: "", ruleId: "" }); @@ -352,21 +535,41 @@ describe("formatter:html", () => { }); describe("when passed a single message with no line or column", () => { - - const code = [{ - filePath: "foo.js", - errorCount: 1, - warningCount: 0, - messages: [{ - message: "Unexpected foo.", - severity: 2, - ruleId: "foo", - source: "foo" - }] - }]; + const rulesMeta = { + foo: { + type: "problem", + + docs: { + description: "This is rule 'foo'", + category: "error", + recommended: true, + url: "https://eslint.org/docs/rules/foo" + }, + + fixable: "code", + + messages: { + message1: "This is a message for rule 'foo'." + } + } + }; + const code = { + results: [{ + filePath: "foo.js", + errorCount: 1, + warningCount: 0, + messages: [{ + message: "Unexpected foo.", + severity: 2, + ruleId: "foo", + source: "foo" + }] + }], + rulesMeta + }; it("should return a string in HTML format with 1 issue in 1 file and styled accordingly", () => { - const result = formatter(code); + const result = formatter(code.results, { rulesMeta }); const $ = cheerio.load(result); checkContentRow($, $("tr")[1], { group: "f-0", lineCol: "0:0", color: "clr-2", message: "Unexpected foo.", ruleId: "foo" }); From 3968685dc6e406390daec9b14e264ab965947f9f Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Sun, 24 Mar 2019 23:06:42 -0700 Subject: [PATCH 3/6] Update cli unit tests to stub getRules --- tests/lib/cli.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 110cc325ff1..65c98836550 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -726,6 +726,7 @@ describe("cli", () => { results: [] }); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.stub(); localCLI = proxyquire("../../lib/cli", { @@ -762,6 +763,7 @@ describe("cli", () => { results: [] }); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.mock().once(); localCLI = proxyquire("../../lib/cli", { @@ -799,6 +801,7 @@ describe("cli", () => { fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype); sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.mock().withExactArgs(report); localCLI = proxyquire("../../lib/cli", { @@ -835,6 +838,7 @@ describe("cli", () => { fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype); sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.getErrorResults = sandbox.stub().returns([]); fakeCLIEngine.outputFixes = sandbox.mock().withExactArgs(report); @@ -886,6 +890,7 @@ describe("cli", () => { results: [] }); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.mock().never(); localCLI = proxyquire("../../lib/cli", { @@ -916,6 +921,7 @@ describe("cli", () => { results: [] }); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.stub(); localCLI = proxyquire("../../lib/cli", { @@ -951,6 +957,7 @@ describe("cli", () => { fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype); sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.mock().never(); localCLI = proxyquire("../../lib/cli", { @@ -987,6 +994,7 @@ describe("cli", () => { fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype); sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.getErrorResults = sandbox.stub().returns([]); fakeCLIEngine.outputFixes = sandbox.mock().never(); @@ -1024,6 +1032,7 @@ describe("cli", () => { fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype); sandbox.stub(fakeCLIEngine.prototype, "executeOnText").returns(report); sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done"); + sandbox.stub(fakeCLIEngine.prototype, "getRules").returns(new Map()); fakeCLIEngine.outputFixes = sandbox.mock().never(); localCLI = proxyquire("../../lib/cli", { From 1a3a1f147709b32c1867a6d38aec3d4243849c58 Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Mon, 25 Mar 2019 10:02:26 -0700 Subject: [PATCH 4/6] Update working-with-custom-formatters.md --- .../working-with-custom-formatters.md | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/docs/developer-guide/working-with-custom-formatters.md b/docs/developer-guide/working-with-custom-formatters.md index 58fe5593678..39ea76b8837 100644 --- a/docs/developer-guide/working-with-custom-formatters.md +++ b/docs/developer-guide/working-with-custom-formatters.md @@ -19,6 +19,34 @@ eslint -f ./my-awesome-formatter.js src/ In order to use a local file as a custom formatter, you must begin the filename with a dot (such as `./my-awesome-formatter.js` or `../formatters/my-awesome-formatter.js`). +### The `data` Argument +The exported function receives an optional second argument named `data`. The `data` object provides extended information related to the analysis results. Currently, the `data` object consists of a single property named `rulesMeta`. This property is a dictionary of `Rule` metadata, keyed with `ruleId`. The value for each entry is the `meta` property from the corresponding `Rule` object. The dictionary contains an entry for each `Rule` that was run during the analysis. + +Here's what the `data` object would look like if one rule, `no-extra-semi`, had been run: + +```js +{ + rulesMeta: { + "no-extra-semi": { + type: "suggestion", + docs: { + description: "disallow unnecessary semicolons", + category: "Possible Errors", + recommended: true, + url: "https://eslint.org/docs/rules/no-extra-semi" + }, + fixable: "code", + schema: [], + messages: { + unexpected: "Unnecessary semicolon." + } + } + } +} +``` + +The [Using Rule metadata](#using-rule-metadata) example shows how to use the `data` object in a custom formatter. See the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) page for more information about the `Rule` object. + ## Packaging the Custom Formatter Custom formatters can also be distributed through npm packages. To do so, create an npm package with a name in the format of `eslint-formatter-*`, where `*` is the name of your formatter (such as `eslint-formatter-awesome`). Projects should then install the package and can use the custom formatter with the `-f` (or `--formatter`) flag like this: @@ -235,6 +263,76 @@ warning no-unused-vars src/configs/clean.js:3:6 ``` +### Using Rule metadata + +A formatter that summarizes the rules that were triggered might look like this: + +```javascript +module.exports = function(results, data) { + var results = results || []; + var rulesMeta = data.rulesMeta; + + var summary = results.reduce( + function(seq, current) { + current.messages.forEach(function(msg) { + if (!seq.contains(msg.ruleId)) { + seq.push(msg.ruleId); + } + }); + }, + { + rules: [] + } + ); + + if (summary.rules.length > 0) { + var lines = summary.rules.map(function (ruleId) { + var ruleMeta = rulesMeta[ruleId]; + var text = ruleId; + + if (ruleMeta) { + if (ruleMeta.type) { + text += " (" + ruleMeta.type + ")"; + } + if (ruleMeta.docs) { + if (ruleMeta.docs.description) { + text += "\n " + ruleMeta.docs.description; + } + if (ruleMeta.docs.url) { + text += "\n " + ruleMeta.docs.url; + } + } + } + + return text; + } + .join("\n\n"); + + return lines + "\n"; + } +} +``` + +The output will be + +```bash +space-infix-ops (layout) + require spacing around infix operators + https://eslint.org/docs/rules/space-infix-ops + +semi (layout) + require or disallow semicolons instead of ASI + https://eslint.org/docs/rules/semi + +no-unused-vars (problem) + disallow unused variables + https://eslint.org/docs/rules/no-unused-vars + +no-shadow (suggestion) + disallow variable declarations from shadowing variables declared in the outer scope + https://eslint.org/docs/rules/no-shadow +``` + ## Passing Arguments to Formatters While custom formatter do not receive arguments in addition to the results object, it is possible to pass additional data into formatters. From f9892f08880a4894156c9066d611439a84da9a42 Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Mon, 25 Mar 2019 10:44:35 -0700 Subject: [PATCH 5/6] Update working-with-custom-formatters.md --- docs/developer-guide/working-with-custom-formatters.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/developer-guide/working-with-custom-formatters.md b/docs/developer-guide/working-with-custom-formatters.md index 39ea76b8837..0a306471988 100644 --- a/docs/developer-guide/working-with-custom-formatters.md +++ b/docs/developer-guide/working-with-custom-formatters.md @@ -20,6 +20,7 @@ eslint -f ./my-awesome-formatter.js src/ In order to use a local file as a custom formatter, you must begin the filename with a dot (such as `./my-awesome-formatter.js` or `../formatters/my-awesome-formatter.js`). ### The `data` Argument + The exported function receives an optional second argument named `data`. The `data` object provides extended information related to the analysis results. Currently, the `data` object consists of a single property named `rulesMeta`. This property is a dictionary of `Rule` metadata, keyed with `ruleId`. The value for each entry is the `meta` property from the corresponding `Rule` object. The dictionary contains an entry for each `Rule` that was run during the analysis. Here's what the `data` object would look like if one rule, `no-extra-semi`, had been run: @@ -293,11 +294,11 @@ module.exports = function(results, data) { if (ruleMeta) { if (ruleMeta.type) { text += " (" + ruleMeta.type + ")"; - } + } if (ruleMeta.docs) { if (ruleMeta.docs.description) { text += "\n " + ruleMeta.docs.description; - } + } if (ruleMeta.docs.url) { text += "\n " + ruleMeta.docs.url; } From 7721c5cd87fb608a2b886b5f4ee8c7181509faeb Mon Sep 17 00:00:00 2001 From: Chris Meyer Date: Wed, 27 Mar 2019 10:17:56 -0700 Subject: [PATCH 6/6] PR feedback --- .../working-with-custom-formatters.md | 91 +++---------------- lib/formatters/html.js | 15 ++- lib/formatters/json-with-metadata.js | 6 +- tests/lib/formatters/json-with-metadata.js | 6 +- 4 files changed, 25 insertions(+), 93 deletions(-) diff --git a/docs/developer-guide/working-with-custom-formatters.md b/docs/developer-guide/working-with-custom-formatters.md index 0a306471988..8cb6738e828 100644 --- a/docs/developer-guide/working-with-custom-formatters.md +++ b/docs/developer-guide/working-with-custom-formatters.md @@ -21,7 +21,7 @@ In order to use a local file as a custom formatter, you must begin the filename ### The `data` Argument -The exported function receives an optional second argument named `data`. The `data` object provides extended information related to the analysis results. Currently, the `data` object consists of a single property named `rulesMeta`. This property is a dictionary of `Rule` metadata, keyed with `ruleId`. The value for each entry is the `meta` property from the corresponding `Rule` object. The dictionary contains an entry for each `Rule` that was run during the analysis. +The exported function receives an optional second argument named `data`. The `data` object provides extended information related to the analysis results. Currently, the `data` object consists of a single property named `rulesMeta`. This property is a dictionary of rule metadata, keyed with `ruleId`. The value for each entry is the `meta` property from the corresponding rule object. The dictionary contains an entry for each rule that was run during the analysis. Here's what the `data` object would look like if one rule, `no-extra-semi`, had been run: @@ -46,7 +46,7 @@ Here's what the `data` object would look like if one rule, `no-extra-semi`, had } ``` -The [Using Rule metadata](#using-rule-metadata) example shows how to use the `data` object in a custom formatter. See the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) page for more information about the `Rule` object. +The [Using Rule metadata](#using-rule-metadata) example shows how to use the `data` object in a custom formatter. See the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) page for more information about rules. ## Packaging the Custom Formatter @@ -186,7 +186,7 @@ Errors: 2, Warnings: 4 A more complex report will look something like this: ```javascript -module.exports = function(results) { +module.exports = function(results, data) { var results = results || []; var summary = results.reduce( @@ -195,6 +195,7 @@ module.exports = function(results) { var logMessage = { filePath: current.filePath, ruleId: msg.ruleId, + ruleUrl: data.rulesMeta[msg.ruleId].url, message: msg.message, line: msg.line, column: msg.column @@ -225,7 +226,7 @@ module.exports = function(results) { "\n" + msg.type + " " + - msg.ruleId + + msg.ruleId + (msg.ruleUrl ? " (" + msg.ruleUrl + ")" : "" "\n " + msg.filePath + ":" + @@ -250,90 +251,20 @@ eslint -f ./my-awesome-formatter.js src/ The output will be ```bash -error space-infix-ops +error space-infix-ops (https://eslint.org/docs/rules/space-infix-ops) src/configs/bundler.js:6:8 -error semi +error semi (https://eslint.org/docs/rules/semi) src/configs/bundler.js:6:10 -warning no-unused-vars +warning no-unused-vars (https://eslint.org/docs/rules/no-unused-vars) src/configs/bundler.js:5:6 -warning no-unused-vars +warning no-unused-vars (https://eslint.org/docs/rules/no-unused-vars) src/configs/bundler.js:6:6 -warning no-shadow +warning no-shadow (https://eslint.org/docs/rules/no-shadow) src/configs/bundler.js:65:32 -warning no-unused-vars +warning no-unused-vars (https://eslint.org/docs/rules/no-unused-vars) src/configs/clean.js:3:6 ``` -### Using Rule metadata - -A formatter that summarizes the rules that were triggered might look like this: - -```javascript -module.exports = function(results, data) { - var results = results || []; - var rulesMeta = data.rulesMeta; - - var summary = results.reduce( - function(seq, current) { - current.messages.forEach(function(msg) { - if (!seq.contains(msg.ruleId)) { - seq.push(msg.ruleId); - } - }); - }, - { - rules: [] - } - ); - - if (summary.rules.length > 0) { - var lines = summary.rules.map(function (ruleId) { - var ruleMeta = rulesMeta[ruleId]; - var text = ruleId; - - if (ruleMeta) { - if (ruleMeta.type) { - text += " (" + ruleMeta.type + ")"; - } - if (ruleMeta.docs) { - if (ruleMeta.docs.description) { - text += "\n " + ruleMeta.docs.description; - } - if (ruleMeta.docs.url) { - text += "\n " + ruleMeta.docs.url; - } - } - } - - return text; - } - .join("\n\n"); - - return lines + "\n"; - } -} -``` - -The output will be - -```bash -space-infix-ops (layout) - require spacing around infix operators - https://eslint.org/docs/rules/space-infix-ops - -semi (layout) - require or disallow semicolons instead of ASI - https://eslint.org/docs/rules/semi - -no-unused-vars (problem) - disallow unused variables - https://eslint.org/docs/rules/no-unused-vars - -no-shadow (suggestion) - disallow variable declarations from shadowing variables declared in the outer scope - https://eslint.org/docs/rules/no-shadow -``` - ## Passing Arguments to Formatters While custom formatter do not receive arguments in addition to the results object, it is possible to pass additional data into formatters. diff --git a/lib/formatters/html.js b/lib/formatters/html.js index f2639de109a..71b46cf5cfc 100644 --- a/lib/formatters/html.js +++ b/lib/formatters/html.js @@ -15,7 +15,6 @@ const path = require("path"); const pageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-page.html"), "utf-8")); const messageTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-message.html"), "utf-8")); const resultTemplate = lodash.template(fs.readFileSync(path.join(__dirname, "html-template-result.html"), "utf-8")); -let rulesMeta; /** * Given a word and a count, append an s if count is not one. @@ -63,9 +62,10 @@ function renderColor(totalErrors, totalWarnings) { * Get HTML (table rows) describing the messages. * @param {Array} messages Messages. * @param {int} parentIndex Index of the parent HTML row. + * @param {Object} rulesMeta Dictionary containing metadata for each rule executed by the analysis. * @returns {string} HTML (table rows) describing the messages. */ -function renderMessages(messages, parentIndex) { +function renderMessages(messages, parentIndex, rulesMeta) { /** * Get HTML (table row) describing a message. @@ -91,23 +91,24 @@ function renderMessages(messages, parentIndex) { severityName: message.severity === 1 ? "Warning" : "Error", message: message.message, ruleId: message.ruleId, - ruleUrl: ruleUrl !== null ? ruleUrl : "https://eslint.org/docs/rules" + ruleUrl }); }).join("\n"); } /** * @param {Array} results Test results. + * @param {Object} rulesMeta Dictionary containing metadata for each rule executed by the analysis. * @returns {string} HTML string describing the results. */ -function renderResults(results) { +function renderResults(results, rulesMeta) { return lodash.map(results, (result, index) => resultTemplate({ index, color: renderColor(result.errorCount, result.warningCount), filePath: result.filePath, summary: renderSummary(result.errorCount, result.warningCount) - }) + renderMessages(result.messages, index)).join("\n"); + }) + renderMessages(result.messages, index, rulesMeta)).join("\n"); } //------------------------------------------------------------------------------ @@ -121,8 +122,6 @@ module.exports = function(results, data) { totalErrors = 0; totalWarnings = 0; - rulesMeta = data.rulesMeta; - // Iterate over results to get totals results.forEach(result => { totalErrors += result.errorCount; @@ -133,6 +132,6 @@ module.exports = function(results, data) { date: new Date(), reportColor: renderColor(totalErrors, totalWarnings), reportSummary: renderSummary(totalErrors, totalWarnings), - results: renderResults(results) + results: renderResults(results, data.rulesMeta) }); }; diff --git a/lib/formatters/json-with-metadata.js b/lib/formatters/json-with-metadata.js index 949e767da3d..6899471547a 100644 --- a/lib/formatters/json-with-metadata.js +++ b/lib/formatters/json-with-metadata.js @@ -1,6 +1,6 @@ /** - * @fileoverview JSON reporter - * @author Burak Yigit Kaya aka BYK + * @fileoverview JSON reporter, including rules metadata + * @author Chris Meyer */ "use strict"; @@ -11,6 +11,6 @@ module.exports = function(results, data) { return JSON.stringify({ results, - rulesMeta: data.rulesMeta + metadata: data }); }; diff --git a/tests/lib/formatters/json-with-metadata.js b/tests/lib/formatters/json-with-metadata.js index 79775b3f8de..cf0692ed036 100644 --- a/tests/lib/formatters/json-with-metadata.js +++ b/tests/lib/formatters/json-with-metadata.js @@ -68,11 +68,13 @@ describe("formatter:json", () => { ruleId: "bar" }] }], - rulesMeta + metadata: { + rulesMeta + } }; it("should return passed results and data as a JSON string without any modification", () => { - const result = JSON.parse(formatter(code.results, { rulesMeta })); + const result = JSON.parse(formatter(code.results, code.metadata)); assert.deepStrictEqual(result, code); });