Skip to content

Commit b9debba

Browse files
adangeladangel
committed
Merge pull request #12 from features/annotation
Create annotations from sarif report #12 * pr-12: Remove empty last line in rule description Remove indentation from rule descriptions Add rule name, priority and ruleset Consider endLine as well Improve annotations text Add debug logging Make the file paths relative Create annotations from sarif report
2 parents c3b3f6d + c00c97c commit b9debba

File tree

8 files changed

+363
-15
lines changed

8 files changed

+363
-15
lines changed

‎dist/index.js‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/annotations.js‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
const core = require('@actions/core');
2+
3+
const processSarifReport = function (report) {
4+
if (!report) {
5+
return;
6+
}
7+
8+
const basedir = process.cwd();
9+
const rules = report.runs[0].tool.driver.rules;
10+
const results = report.runs[0].results;
11+
12+
core.startGroup('PMD Results');
13+
14+
core.debug(`processing sarif report. basedir=${basedir}`);
15+
core.debug(`rules: ${rules.length}`);
16+
core.debug(`results: ${results.length}`);
17+
18+
19+
results.forEach(violation => {
20+
const rule = rules[violation.ruleIndex];
21+
const logFunction = mapPriority(rule.properties.priority);
22+
violation.locations.forEach(location => {
23+
const annotation = createAnnotation(location.physicalLocation, basedir, violation.message.text);
24+
core.info(`\n${annotation.file}:${annotation.startLine}:${rule.id} (Priority: ${rule.properties.priority}):${violation.message.text}`);
25+
logFunction(createDescription(rule), annotation);
26+
});
27+
});
28+
29+
core.endGroup();
30+
}
31+
32+
function mapPriority(pmdPriority) {
33+
switch (pmdPriority) {
34+
case 1: // net.sourceforge.pmd.RulePriority.HIGH
35+
case 2: // net.sourceforge.pmd.RulePriority.MEDIUM_HIGH
36+
return core.error;
37+
case 3: // net.sourceforge.pmd.RulePriority.MEDIUM
38+
case 4: // net.sourceforge.pmd.RulePriority.MEDIUM_LOW
39+
return core.warning;
40+
default: // net.sourceforge.pmd.RulePriority.LOW (5)
41+
return core.notice;
42+
}
43+
}
44+
45+
// AnnotationProperties from @actions/core
46+
function createAnnotation(location, basedir, title) {
47+
return {
48+
title: title,
49+
file: makeRelative(location.artifactLocation.uri, basedir),
50+
startLine: location.region.startLine,
51+
endLine: location.region.endLine
52+
};
53+
}
54+
55+
function makeRelative(fullPath, basedir) {
56+
if (fullPath.startsWith(`${basedir}/`)) {
57+
return fullPath.substr(basedir.length + 1);
58+
}
59+
return fullPath;
60+
}
61+
62+
function createDescription(rule) {
63+
const lines = rule.fullDescription.text.split(/\n|\r\n/);
64+
// remove empty first line
65+
if (lines.length > 1 && lines[0] === '') {
66+
lines.splice(0, 1);
67+
}
68+
let indentation = '';
69+
const matchResult = lines[0].match(/^([ \t]+).*$/);
70+
if (matchResult !== null) {
71+
indentation = matchResult[1];
72+
}
73+
for (let i = 0; i < lines.length; i++) {
74+
if (lines[i].startsWith(indentation)) {
75+
lines[i] = lines[i].substr(indentation.length);
76+
}
77+
}
78+
// remove empty last line
79+
if (lines.length > 0 && lines[lines.length - 1].trim() === '') {
80+
lines.splice(lines.length - 1, 1);
81+
}
82+
const description = lines.join('\n');
83+
const desc =
84+
`${description}
85+
86+
${rule.id} (Priority: ${rule.properties.priority}, Ruleset: ${rule.properties.ruleset})
87+
${rule.helpUri.trim()}`;
88+
return desc;
89+
}
90+
91+
module.exports.processSarifReport = processSarifReport;

‎lib/index.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ const core = require('@actions/core');
22
const artifact = require('@actions/artifact');
33
const util = require('./util');
44
const sarif = require('./sarif');
5-
const validator = require('../lib/validator');
5+
const validator = require('./validator');
6+
const annotations = require('./annotations');
67

78
const reportFormat = 'sarif';
89
const reportFile = 'pmd-report.sarif'
@@ -23,6 +24,9 @@ async function run() {
2324
core.setOutput('violations', violations);
2425
core.info(`PMD detected ${violations} violations.`);
2526

27+
const report = sarif.loadReport(reportFile);
28+
annotations.processSarifReport(report);
29+
2630
const artifactClient = artifact.create();
2731
await artifactClient.uploadArtifact('PMD Report', [reportFile], '.', {
2832
continueOnError: false

‎lib/sarif.js‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,26 @@
11
const fs = require('fs');
22

3-
const countViolations = function(reportFile) {
4-
if (!fs.existsSync(reportFile)) {
5-
return 0;
3+
const countViolations = function (reportFile) {
4+
let count = 0;
5+
6+
const report = loadReport(reportFile);
7+
if (report !== null) {
8+
const results = report.runs[0].results;
9+
results.forEach(rule => {
10+
count += rule.locations.length;
11+
});
612
}
713

8-
const report = JSON.parse(fs.readFileSync(reportFile));
9-
const results = report.runs[0].results;
10-
let count = 0;
11-
results.forEach(rule => {
12-
count += rule.locations.length;
13-
});
1414
return count;
1515
}
1616

17+
const loadReport = function (reportFile) {
18+
if (!fs.existsSync(reportFile)) {
19+
return null;
20+
}
21+
22+
return JSON.parse(fs.readFileSync(reportFile));
23+
}
24+
1725
module.exports.countViolations = countViolations;
26+
module.exports.loadReport = loadReport;

‎tests/annotations.test.js‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
const core = require('@actions/core');
2+
const path = require('path');
3+
const sarif = require('../lib/sarif');
4+
const annotations = require('../lib/annotations');
5+
6+
core.error = jest.fn();
7+
core.warning = jest.fn();
8+
core.notice = jest.fn();
9+
process.cwd = jest.fn();
10+
11+
describe('pmd-github-action-annotations', function () {
12+
13+
beforeEach(() => {
14+
core.error.mockClear();
15+
core.warning.mockClear();
16+
core.notice.mockClear();
17+
process.cwd.mockClear();
18+
process.cwd.mockReturnValue('/folder');
19+
});
20+
21+
it('can create annotation', () => {
22+
const report = sarif.loadReport(path.join(__dirname, 'data', 'pmd-report.sarif'));
23+
24+
annotations.processSarifReport(report);
25+
26+
expect(core.notice).toHaveBeenCalledTimes(1);
27+
expect(core.notice).toHaveBeenCalledWith(`Detects when a local variable is declared and/or assigned but not used.
28+
Second line.
29+
Third line with additional indentation.
30+
Fourth line with less indentation.
31+
32+
UnusedLocalVariable (Priority: 5, Ruleset: Best Practices)
33+
https://pmd.github.io/pmd-6.40.0/pmd_rules_apex_bestpractices.html#unusedlocalvariable`, {
34+
title: 'Variable \'x\' defined but not used',
35+
file: '/home/andreas/PMD/source/pmd-github-action-test/src/classes/UnusedLocalVariableSample.cls',
36+
startLine: 3,
37+
endLine: 3
38+
});
39+
expect(core.error).not.toHaveBeenCalled();
40+
expect(core.warning).not.toHaveBeenCalled();
41+
});
42+
43+
it('can deal with null report', () => {
44+
annotations.processSarifReport(null);
45+
expect(core.notice).not.toHaveBeenCalled();
46+
});
47+
48+
it('can deal with error, warning and notice', () => {
49+
const report = sarif.loadReport(path.join(__dirname, 'data', 'pmd-report-priorities.sarif'));
50+
annotations.processSarifReport(report);
51+
52+
expect(core.error).toHaveBeenCalledTimes(2);
53+
expect(core.error).toHaveBeenNthCalledWith(1, 'Full description for High Prio Rule\n\n0 - high prio rule (Priority: 1, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleHighPrio', { title: 'High Prio Rule', file: 'file1.txt', startLine: 4, endLine: 5 });
54+
expect(core.error).toHaveBeenNthCalledWith(2, 'Full description for Medium High Prio Rule\n\n1 - medium high prio rule (Priority: 2, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleMediumHighPrio', { title: 'Medium High Prio Rule', file: 'dir/file2.txt', startLine: 5 });
55+
expect(core.warning).toHaveBeenCalledTimes(2);
56+
expect(core.warning).toHaveBeenNthCalledWith(1, 'Full description for Medium Prio Rule\n\n2 - medium prio rule (Priority: 3, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleMediumPrio', { title: 'Medium Prio Rule', file: 'file3.txt', startLine: 6 });
57+
expect(core.warning).toHaveBeenNthCalledWith(2, 'Full description for Medium Low Prio Rule\n\n3 - medium low prio rule (Priority: 4, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleMediumLowPrio', { title: 'Medium Low Prio Rule', file: 'file4.txt', startLine: 7 });
58+
expect(core.notice).toHaveBeenCalledTimes(2);
59+
expect(core.notice).toHaveBeenNthCalledWith(1, 'Full description for Low Prio Rule\n\n4 - low prio rule (Priority: 5, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleLowPrio', { title: 'Low Prio Rule', file: 'file5.txt', startLine: 8 });
60+
expect(core.notice).toHaveBeenNthCalledWith(2, 'Full description for Low Prio Rule\n\n4 - low prio rule (Priority: 5, Ruleset: sample ruleset)\nhttps://pmd.github.io/latest/ruleLowPrio', { title: 'Low Prio Rule', file: 'file6.txt', startLine: 9 });
61+
});
62+
});

‎tests/data/pmd-report-priorities.sarif‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
{
2+
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
3+
"version": "2.1.0",
4+
"runs": [
5+
{
6+
"tool": {
7+
"driver": {
8+
"rules": [
9+
{
10+
"id": "0 - high prio rule",
11+
"fullDescription": {
12+
"text": "Full description for High Prio Rule"
13+
},
14+
"helpUri": "https://pmd.github.io/latest/ruleHighPrio",
15+
"properties": {
16+
"priority": 1,
17+
"ruleset": "sample ruleset"
18+
}
19+
},
20+
{
21+
"id": "1 - medium high prio rule",
22+
"fullDescription": {
23+
"text": "Full description for Medium High Prio Rule"
24+
},
25+
"helpUri": "https://pmd.github.io/latest/ruleMediumHighPrio",
26+
"properties": {
27+
"priority": 2,
28+
"ruleset": "sample ruleset"
29+
}
30+
},
31+
{
32+
"id": "2 - medium prio rule",
33+
"fullDescription": {
34+
"text": "Full description for Medium Prio Rule"
35+
},
36+
"helpUri": "https://pmd.github.io/latest/ruleMediumPrio",
37+
"properties": {
38+
"priority": 3,
39+
"ruleset": "sample ruleset"
40+
}
41+
},
42+
{
43+
"id": "3 - medium low prio rule",
44+
"fullDescription": {
45+
"text": "Full description for Medium Low Prio Rule"
46+
},
47+
"helpUri": "https://pmd.github.io/latest/ruleMediumLowPrio",
48+
"properties": {
49+
"priority": 4,
50+
"ruleset": "sample ruleset"
51+
}
52+
},
53+
{
54+
"id": "4 - low prio rule",
55+
"fullDescription": {
56+
"text": "Full description for Low Prio Rule"
57+
},
58+
"helpUri": "https://pmd.github.io/latest/ruleLowPrio",
59+
"properties": {
60+
"priority": 5,
61+
"ruleset": "sample ruleset"
62+
}
63+
}
64+
]
65+
}
66+
},
67+
"results": [
68+
{
69+
"ruleIndex": 0,
70+
"message": {
71+
"text": "High Prio Rule"
72+
},
73+
"locations": [
74+
{
75+
"physicalLocation": {
76+
"artifactLocation": {
77+
"uri": "/folder/file1.txt"
78+
},
79+
"region": {
80+
"startLine": 4,
81+
"endLine": 5
82+
}
83+
}
84+
}
85+
]
86+
},
87+
{
88+
"ruleIndex": 1,
89+
"message": {
90+
"text": "Medium High Prio Rule"
91+
},
92+
"locations": [
93+
{
94+
"physicalLocation": {
95+
"artifactLocation": {
96+
"uri": "/folder/dir/file2.txt"
97+
},
98+
"region": {
99+
"startLine": 5
100+
}
101+
}
102+
}
103+
]
104+
},
105+
{
106+
"ruleIndex": 2,
107+
"message": {
108+
"text": "Medium Prio Rule"
109+
},
110+
"locations": [
111+
{
112+
"physicalLocation": {
113+
"artifactLocation": {
114+
"uri": "/folder/file3.txt"
115+
},
116+
"region": {
117+
"startLine": 6
118+
}
119+
}
120+
}
121+
]
122+
},
123+
{
124+
"ruleIndex": 3,
125+
"message": {
126+
"text": "Medium Low Prio Rule"
127+
},
128+
"locations": [
129+
{
130+
"physicalLocation": {
131+
"artifactLocation": {
132+
"uri": "/folder/file4.txt"
133+
},
134+
"region": {
135+
"startLine": 7
136+
}
137+
}
138+
}
139+
]
140+
},
141+
{
142+
"ruleIndex": 4,
143+
"message": {
144+
"text": "Low Prio Rule"
145+
},
146+
"locations": [
147+
{
148+
"physicalLocation": {
149+
"artifactLocation": {
150+
"uri": "/folder/file5.txt"
151+
},
152+
"region": {
153+
"startLine": 8
154+
}
155+
}
156+
},
157+
{
158+
"physicalLocation": {
159+
"artifactLocation": {
160+
"uri": "/folder/file6.txt"
161+
},
162+
"region": {
163+
"startLine": 9
164+
}
165+
}
166+
}
167+
]
168+
}
169+
]
170+
}
171+
]
172+
}

‎tests/data/pmd-report.sarif‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"text": "Variable 'x' defined but not used"
1616
},
1717
"fullDescription": {
18-
"text": "\nDetects when a local variable is declared and/or assigned but not used.\n "
18+
"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 "
1919
},
2020
"helpUri": "https://pmd.github.io/pmd-6.40.0/pmd_rules_apex_bestpractices.html#unusedlocalvariable",
2121
"help": {

0 commit comments

Comments
 (0)