Skip to content

Commit

Permalink
Fix Sarif report for multiple results
Browse files Browse the repository at this point in the history
Each PMD rule violation should be reported
in a separate result. This adds a fix
to modify the sarif report afterwards
before processing it further.

Fixes #53
  • Loading branch information
adangel committed Feb 17, 2022
1 parent 0abbef9 commit 74178ba
Show file tree
Hide file tree
Showing 6 changed files with 258 additions and 7 deletions.
6 changes: 3 additions & 3 deletions dist/index.js

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ async function main() {
core.info(`PMD exited with ${execOutput.exitCode}`);

sarif.relativizeReport(reportFile);
sarif.fixResults(reportFile);

violations = sarif.countViolations(reportFile);
core.setOutput('violations', violations);
Expand Down
46 changes: 42 additions & 4 deletions lib/sarif.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
const fs = require('fs');
const path = require('path');
const core = require('@actions/core');
const semver = require('semver');

const countViolations = function (reportFile) {
let count = 0;

const report = loadReport(reportFile);
if (report !== null) {
const results = report.runs[0].results;
results.forEach(rule => {
count += rule.locations.length;
});
count = report.runs[0].results.length;
}

return count;
Expand Down Expand Up @@ -44,6 +42,46 @@ const relativizeReport = function (reportFile) {
fs.writeFileSync(reportFile, JSON.stringify(report));
}

/**
* Due to https://github.com/pmd/pmd/issues/3768 violations for a single rule are
* reported in a single result. This needs to be extracted, as each rule violation should
* be a separate result.
*
* Note: This will be fixed with PMD 6.43.0, so this fix here is only needed for earlier versions.
*
* @param {String} reportFile
*/
const fixResults = function (reportFile) {
const report = loadReport(reportFile);
if (report === null) {
return;
}

const pmdVersion = report.runs[0].tool.driver.version;
core.debug(`Sarif Report was created by PMD version ${pmdVersion}`);
if (semver.gte(pmdVersion, '6.43.0')) {
core.debug(`Sarif Report fix is not needed for PMD version ${pmdVersion}`);
return;
}

const originalResults = report.runs[0].results;
const fixedResults = [];
core.debug(`Fixing Sarif Report results: count before: ${originalResults.length}`);
originalResults.forEach(result => {
const originalLocations = result.locations;
delete result.locations;
originalLocations.forEach(location => {
const copy = Object.assign({}, result);
copy.locations = [location];
fixedResults.push(copy);
});
});
core.debug(`Fixing Sarif Report results: count after: ${fixedResults.length}`);
report.runs[0].results = fixedResults;
fs.writeFileSync(reportFile, JSON.stringify(report));
}

module.exports.countViolations = countViolations;
module.exports.loadReport = loadReport;
module.exports.relativizeReport = relativizeReport;
module.exports.fixResults = fixResults;
90 changes: 90 additions & 0 deletions tests/data/pmd-report-multiple-fixed.sarif
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "PMD",
"version": "6.40.0",
"informationUri": "https://pmd.github.io/pmd/",
"rules": [
{
"id": "UnusedLocalVariable",
"shortDescription": {
"text": "Variable 'x' defined but not used"
},
"fullDescription": {
"text": "\n Detects when a local variable is declared and/or assigned but not used.\n Second line.\n Third line with additional indentation.\n Fourth line with less indentation.\n "
},
"helpUri": "https://pmd.github.io/pmd-6.40.0/pmd_rules_apex_bestpractices.html#unusedlocalvariable",
"help": {
"text": "\nDetects when a local variable is declared and/or assigned but not used.\n "
},
"properties": {
"ruleset": "Best Practices",
"priority": 5,
"tags": [
"Best Practices"
]
}
}
]
}
},
"results": [
{
"ruleId": "UnusedLocalVariable",
"ruleIndex": 0,
"message": {
"text": "Variable 'x' defined but not used"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "/home/andreas/PMD/source/pmd-github-action-test/src/classes/UnusedLocalVariableSample.cls"
},
"region": {
"startLine": 3,
"startColumn": 16,
"endLine": 3,
"endColumn": 16
}
}
}
]
},
{
"ruleId": "UnusedLocalVariable",
"ruleIndex": 0,
"message": {
"text": "Variable 'x' defined but not used"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "/home/andreas/PMD/source/pmd-github-action-test/src/classes/UnusedLocalVariableSample.cls"
},
"region": {
"startLine": 4,
"startColumn": 16,
"endLine": 4,
"endColumn": 16
}
}
}
]
}
],
"invocations": [
{
"executionSuccessful": true,
"toolConfigurationNotifications": [],
"toolExecutionNotifications": []
}
]
}
]
}
81 changes: 81 additions & 0 deletions tests/data/pmd-report-multiple.sarif
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
{
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "PMD",
"version": "6.40.0",
"informationUri": "https://pmd.github.io/pmd/",
"rules": [
{
"id": "UnusedLocalVariable",
"shortDescription": {
"text": "Variable 'x' defined but not used"
},
"fullDescription": {
"text": "\n Detects when a local variable is declared and/or assigned but not used.\n Second line.\n Third line with additional indentation.\n Fourth line with less indentation.\n "
},
"helpUri": "https://pmd.github.io/pmd-6.40.0/pmd_rules_apex_bestpractices.html#unusedlocalvariable",
"help": {
"text": "\nDetects when a local variable is declared and/or assigned but not used.\n "
},
"properties": {
"ruleset": "Best Practices",
"priority": 5,
"tags": [
"Best Practices"
]
}
}
]
}
},
"results": [
{
"ruleId": "UnusedLocalVariable",
"ruleIndex": 0,
"message": {
"text": "Variable 'x' defined but not used"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "/home/andreas/PMD/source/pmd-github-action-test/src/classes/UnusedLocalVariableSample.cls"
},
"region": {
"startLine": 3,
"startColumn": 16,
"endLine": 3,
"endColumn": 16
}
}
},
{
"physicalLocation": {
"artifactLocation": {
"uri": "/home/andreas/PMD/source/pmd-github-action-test/src/classes/UnusedLocalVariableSample.cls"
},
"region": {
"startLine": 4,
"startColumn": 16,
"endLine": 4,
"endColumn": 16
}
}
}
]
}
],
"invocations": [
{
"executionSuccessful": true,
"toolConfigurationNotifications": [],
"toolExecutionNotifications": []
}
]
}
]
}
41 changes: 41 additions & 0 deletions tests/sarif.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const path = require('path');
const sarif = require('../lib/sarif');
const io = require('@actions/io');
const os = require('os');
const fs = require('fs');

const tempPath = path.join(__dirname, 'TEMP');

Expand All @@ -25,6 +26,11 @@ describe('pmd-github-action-sarif', function () {
expect(count).toBe(1);
})

it('can count violations multiple', () => {
const count = sarif.countViolations(path.join(__dirname, 'data', 'pmd-report-multiple.sarif'));
expect(count).toBe(1); // still only one violation, but with two locations
})

it('can deal with empty report', () => {
const count = sarif.countViolations(path.join(__dirname, 'data', 'pmd-report-empty.sarif'));
expect(count).toBe(0);
Expand Down Expand Up @@ -80,4 +86,39 @@ describe('pmd-github-action-sarif', function () {
const reportAfter = sarif.loadReport(reportPath);
expect(reportAfter.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri).toBe('src/classes/UnusedLocalVariableSample.cls');
})

test('sarif report result fix is skipped when no report', async () => {
const reportPath = path.join(tempPath, 'pmd-report-not-existing.sarif');
sarif.fixResults(reportPath);
})

test('sarif report results are fixed', async () => {
const reportPath = path.join(tempPath, 'pmd-report-multiple.sarif');
await io.cp(path.join(__dirname, 'data', 'pmd-report-multiple.sarif'), reportPath);

const reportBefore = sarif.loadReport(reportPath);
expect(reportBefore.runs[0].results.length).toBe(1);
sarif.fixResults(reportPath);
const reportAfter = sarif.loadReport(reportPath);
expect(reportAfter.runs[0].results.length).toBe(2);

const expectedReport = JSON.parse(fs.readFileSync(path.join(__dirname, 'data', 'pmd-report-multiple-fixed.sarif')));
expect(reportAfter).toStrictEqual(expectedReport);
})

test('sarif report results are not fixed for PMD >= 6.43.0', async () => {
const reportPath = path.join(tempPath, 'pmd-report-multiple.sarif');
await io.cp(path.join(__dirname, 'data', 'pmd-report-multiple.sarif'), reportPath);

const reportBefore = sarif.loadReport(reportPath);
reportBefore.runs[0].tool.driver.version = '6.43.0';
fs.writeFileSync(reportPath, JSON.stringify(reportBefore));

expect(reportBefore.runs[0].results.length).toBe(1);
sarif.fixResults(reportPath);
const reportAfter = sarif.loadReport(reportPath);
expect(reportAfter.runs[0].results.length).toBe(1);

expect(reportAfter).toStrictEqual(reportBefore);
})
});

0 comments on commit 74178ba

Please sign in to comment.