From 0f5a6351a5109f7d48787e9d00516ac2bfb96100 Mon Sep 17 00:00:00 2001 From: Chris Raynor Date: Wed, 9 Sep 2020 15:22:34 +0100 Subject: [PATCH 1/4] Only adding shortDescription to a rule if a description exists Otherwise the output is an invalid SARIF file with a missing text field --- src/ESLint.Formatter/sarif.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ESLint.Formatter/sarif.js b/src/ESLint.Formatter/sarif.js index 942626bae..305053c06 100644 --- a/src/ESLint.Formatter/sarif.js +++ b/src/ESLint.Formatter/sarif.js @@ -145,14 +145,16 @@ module.exports = function (results, data) { // Create a new entry in the rules dictionary. sarifRules[message.ruleId] = { id: message.ruleId, - shortDescription: { - text: meta.docs.description - }, helpUri: meta.docs.url, properties: { category: meta.docs.category } }; + if (meta.docs.description) { + sarifRules[message.ruleId].shortDescription = { + text: meta.docs.description + }; + } } } From e147c707bb43f9a1300c3fd630ca5f64d58eab36 Mon Sep 17 00:00:00 2001 From: Chris Raynor Date: Wed, 9 Sep 2020 16:06:12 +0100 Subject: [PATCH 2/4] Adding test for valid SARIF from a custom rule with no description --- src/ESLint.Formatter/test/sarif.js | 51 ++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/ESLint.Formatter/test/sarif.js b/src/ESLint.Formatter/test/sarif.js index 9f44e0b38..8b195d2c3 100644 --- a/src/ESLint.Formatter/test/sarif.js +++ b/src/ESLint.Formatter/test/sarif.js @@ -473,3 +473,54 @@ describe("formatter:sarif", () => { }); }); }); + +describe("formatter:sarif", () => { + describe("when passed a rule with no description", () => { + const ruleid = "custom-rule-no-description"; + + rules[ruleid] = { + type: "suggestion", + docs: { + category: "Possible Errors" + } + }; + const code = [{ + filePath: sourceFilePath1, + messages: [{ + message: "Custom error.", + ruleId: ruleid, + line: 42 + }] + }]; + it("should return a log with one file, one rule, and one result", () => { + const log = JSON.parse(formatter(code, { rulesMeta: rules })); + const rule = rules[ruleid]; + + assert.lengthOf(log.runs[0].artifacts, 1); + assert.lengthOf(log.runs[0].results, 1); + + assert.strictEqual(log.runs[0].tool.driver.rules[0].id, ruleid); + + assert(log.runs[0].artifacts[0].location.uri.startsWith(uriPrefix)); + assert(log.runs[0].artifacts[0].location.uri.endsWith(sourceFilePath1)); + + assert.strictEqual(log.runs[0].tool.driver.rules[0].id, ruleid); + assert.isUndefined(log.runs[0].tool.driver.rules[0].shortDescription); + assert.strictEqual(log.runs[0].tool.driver.rules[0].helpUri, rule.docs.url); + assert.strictEqual(log.runs[0].tool.driver.rules[0].properties.category, rule.docs.category); + + assert.strictEqual(log.runs[0].results[0].ruleId, ruleid); + + assert.strictEqual(log.runs[0].results[0].level, "warning"); + + assert.strictEqual(log.runs[0].results[0].message.text, "Custom error."); + + assert(log.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri.startsWith(uriPrefix)); + assert(log.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri.endsWith(sourceFilePath1)); + + assert.strictEqual(log.runs[0].results[0].locations[0].physicalLocation.region.startLine, 42); + assert.isUndefined(log.runs[0].results[0].locations[0].physicalLocation.region.startColumn); + assert.isUndefined(log.runs[0].results[0].locations[0].physicalLocation.region.snippet); + }); + }); +}); From a7aa894c307bdf32127f85499c98314d59115955 Mon Sep 17 00:00:00 2001 From: Chris Raynor Date: Wed, 9 Sep 2020 15:27:22 +0100 Subject: [PATCH 3/4] Only adding startLine / startColumn if they are > 0 --- src/ESLint.Formatter/sarif.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ESLint.Formatter/sarif.js b/src/ESLint.Formatter/sarif.js index 305053c06..16db1b19a 100644 --- a/src/ESLint.Formatter/sarif.js +++ b/src/ESLint.Formatter/sarif.js @@ -181,9 +181,12 @@ module.exports = function (results, data) { } if (message.line > 0 || message.column > 0) { - sarifRepresentation.locations[0].physicalLocation.region = { - startLine: message.line, - startColumn: message.column + sarifRepresentation.locations[0].physicalLocation.region = {}; + if (message.line > 0) { + sarifRepresentation.locations[0].physicalLocation.region.startLine = message.line; + } + if (message.column > 0) { + sarifRepresentation.locations[0].physicalLocation.region.startColumn = message.column; }; } From e67691af05d87a0b120602cc2492b9ace2916384 Mon Sep 17 00:00:00 2001 From: Chris Raynor Date: Wed, 9 Sep 2020 15:44:09 +0100 Subject: [PATCH 4/4] Adding test for invalid column number --- src/ESLint.Formatter/test/sarif.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/ESLint.Formatter/test/sarif.js b/src/ESLint.Formatter/test/sarif.js index 8b195d2c3..ab7a306c3 100644 --- a/src/ESLint.Formatter/test/sarif.js +++ b/src/ESLint.Formatter/test/sarif.js @@ -161,6 +161,28 @@ describe("formatter:sarif", () => { }); }); +describe("formatter:sarif", () => { + describe("when passed one message with line and invalid column", () => { + const code = [{ + filePath: sourceFilePath1, + messages: [{ + message: "Unexpected value.", + ruleId: testRuleId, + line: 10, + column: 0 + }] + }]; + + it("should return a log with one result whose location contains a region with line # and no column #", () => { + const log = JSON.parse(formatter(code)); + + assert.strictEqual(log.runs[0].results[0].locations[0].physicalLocation.region.startLine, code[0].messages[0].line); + assert.isUndefined(log.runs[0].results[0].locations[0].physicalLocation.region.startColumn); + assert.isUndefined(log.runs[0].results[0].locations[0].physicalLocation.region.snippet); + }); + }); +}); + describe("formatter:sarif", () => { describe("when passed one message with line and column but no source string", () => { const code = [{