Skip to content

Commit 0601cf7

Browse files
adangeladangel
committed
Merge pull request #24 from adangel:analyze-modified-files-only
Determine modified files for pull requests and pushes #24 * pr-24: Log warning when MAX_PAGE is reached Analyze all files if modified files cannot be determined Determine modified files for pushes Fix unit tests under windows Fix unit tests under windows Fix unit tests under windows Fix unit tests under windows Fix path handling under windows Revert "REVERT ME - Enable debug logging" REVERT ME - Enable debug logging Improved logging Add debug logging Don't execute PMD when no modified files are found Fix input parameter "analyzeModifiedFilesOnly" Determine modified files for pull requests
2 parents 97c92b8 + 22ebdb5 commit 0601cf7

File tree

19 files changed

+457
-22
lines changed

19 files changed

+457
-22
lines changed

‎.github/workflows/test.yml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,16 @@ on:
1212
jobs:
1313
# unit tests
1414
unit:
15-
runs-on: ubuntu-latest
15+
runs-on: ${{ matrix.os }}
16+
strategy:
17+
matrix:
18+
os: [ ubuntu-latest, windows-latest, macos-latest ]
19+
fail-fast: false
1620
steps:
1721
- uses: actions/checkout@v2
22+
- name: Set Node.js 12.x
23+
uses: actions/[email protected]
24+
with:
25+
node-version: 12.x
1826
- run: npm ci
1927
- run: npm test

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ See also [Uploading a SARIF file to GitHub](https://docs.github.com/en/code-secu
7474
|`version` |no |"latest"|PMD version to use. Using "latest" automatically downloads the latest version.<br>Available versions: https://github.com/pmd/pmd/releases|
7575
|`sourcePath`|no |"." |Root directory for sources. Uses by default the current directory|
7676
|`rulesets` |yes| |Comma separated list of ruleset names to use.|
77+
|`analyzeModifiedFilesOnly`|no|"true"|Instead of analyze all files under "sourcePath", only the files that have been touched in a pull request or push will be analyzed. This makes the analysis faster and helps especially bigger projects which gradually want to introduce PMD. This helps in enforcing that no new code violation is introduced.<br>Depending on the analyzed language, the results might be less accurate results. At the moment, this is not a problem, as PMD mostly analyzes each file individually, but that might change in the future.<br>If the change is very big, not all files might be analyzed. Currently the maximum number of modified files is 300.|
7778

7879
## Outputs
7980

‎action.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,22 @@ inputs:
2929
rulesets:
3030
description: Comma separated list of ruleset names to use
3131
required: true
32+
analyzeModifiedFilesOnly:
33+
description: >-
34+
Instead of analyze all files under "sourcePath", only the files that have
35+
been touched in a pull request or push will be analyzed. This makes the
36+
analysis faster and helps especially bigger projects which gradually want
37+
to introduce PMD. This helps in enforcing that no new code violation is
38+
introduced.
39+
40+
Depending on the analyzed language, the results might be less accurate
41+
results. At the moment, this is not a problem, as PMD mostly analyzes each
42+
file individually, but that might change in the future.
43+
44+
If the change is very big, not all files might be analyzed. Currently the
45+
maximum number of modified files is 300.
46+
required: false
47+
default: true
3248
outputs:
3349
violations:
3450
description: Number of violations found

‎dist/index.js‎

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

‎lib/index.js‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,27 @@ const reportFormat = 'sarif';
99
const reportFile = 'pmd-report.sarif'
1010

1111
async function main() {
12-
let pmdInfo, execOutput, violations;
12+
let pmdInfo, modifiedFiles, execOutput, violations;
13+
let token = core.getInput('token', { required: true });
14+
let sourcePath = validator.validateSourcePath(core.getInput('sourcePath', { required: true }));
1315
try {
1416
pmdInfo = await util.downloadPmd(
1517
validator.validateVersion(core.getInput('version'), { required: true }),
16-
core.getInput('token', { required: true })
18+
token
1719
);
20+
21+
if (core.getInput('analyzeModifiedFilesOnly', { required: true }) === 'true') {
22+
core.info(`Determining modified files in ${sourcePath}...`);
23+
modifiedFiles = await util.determineModifiedFiles(token, sourcePath);
24+
if (modifiedFiles !== undefined && modifiedFiles.length === 0) {
25+
core.info(`No modified files have been found in ${sourcePath} - exiting`);
26+
core.setOutput('violations', 0);
27+
return;
28+
}
29+
}
30+
1831
execOutput = await util.executePmd(pmdInfo,
19-
validator.validateSourcePath(core.getInput('sourcePath', { required: true })),
32+
modifiedFiles || sourcePath,
2033
validator.validateRulesets(core.getInput('rulesets', { required: true })),
2134
reportFormat, reportFile)
2235

‎lib/util.js‎

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ const tc = require('@actions/tool-cache');
44
const exec = require('@actions/exec');
55
const semver = require('semver');
66
const os = require('os');
7+
const fs = require('fs').promises;
8+
const path = require('path');
9+
10+
// Load at most MAX_PAGE pages when determining modified files.
11+
// This is used both for pull/{pull_number}/files as well as for
12+
// repos/compareCommits API calls.
13+
const MAX_PAGE = 10;
714

815
const downloadPmd = async function(version, token) {
916
let pmdVersion = version;
@@ -20,19 +27,29 @@ const downloadPmd = async function(version, token) {
2027
core.info(`Using PMD ${pmdVersion} from cached path ${cachedPmdPath}`);
2128
return {
2229
version: pmdVersion,
23-
path: `${cachedPmdPath}/pmd-bin-${pmdVersion}`
30+
path: path.join(cachedPmdPath, `pmd-bin-${pmdVersion}`)
2431
}
2532
}
2633

27-
const executePmd = async function(pmdInfo, sourcePath, ruleset, reportFormat, reportFile) {
34+
const executePmd = async function(pmdInfo, fileListOrSourcePath, ruleset, reportFormat, reportFile) {
2835
let pmdExecutable = '/bin/run.sh pmd';
2936
if (os.platform() === 'win32') {
3037
pmdExecutable = '\\bin\\pmd.bat';
3138
}
39+
40+
let sourceParameter = ['-d', fileListOrSourcePath];
41+
if (Array.isArray(fileListOrSourcePath)) {
42+
await writeFileList(fileListOrSourcePath);
43+
sourceParameter = [useNewArgsFormat(pmdInfo.version) ? '--file-list' : '-filelist', 'pmd.filelist'];
44+
core.info(`Running PMD ${pmdInfo.version} on ${fileListOrSourcePath.length} modified files...`);
45+
} else {
46+
core.info(`Running PMD ${pmdInfo.version} on all files in path ${fileListOrSourcePath}...`);
47+
}
48+
3249
const execOutput = await exec.getExecOutput(`${pmdInfo.path}${pmdExecutable}`,
3350
[
3451
useNewArgsFormat(pmdInfo.version) ? '--no-cache' : '-no-cache',
35-
'-d', sourcePath,
52+
...sourceParameter,
3653
'-f', reportFormat,
3754
'-R', ruleset,
3855
'-r', reportFile,
@@ -80,5 +97,92 @@ async function getDownloadURL(release) {
8097
return asset.browser_download_url;
8198
}
8299

100+
async function writeFileList(fileList) {
101+
await fs.writeFile(path.join('.', 'pmd.filelist'), fileList.join(','), 'utf8');
102+
}
103+
104+
const determineModifiedFiles = async function(token, sourcePath) {
105+
// creating new context instead of using "github.context" to reinitialize for unit testing
106+
const context = new github.context.constructor();
107+
const eventData = context.payload;
108+
const octokit = github.getOctokit(token);
109+
if (context.eventName === 'pull_request') {
110+
core.debug(`Pull request ${eventData.number}: ${eventData.pull_request.html_url}`);
111+
112+
let modifiedFilenames = new Set();
113+
114+
// maximum of 300 files are loaded (page size is 30, max 10 pages)
115+
let page;
116+
for(page = 1; page <= MAX_PAGE; page++) {
117+
const listFilesResponse = await octokit.rest.pulls.listFiles({
118+
...context.repo,
119+
pull_number: eventData.number,
120+
per_page: 30,
121+
page: page
122+
});
123+
const allFiles = listFilesResponse.data;
124+
if (allFiles.length == 0) {
125+
break;
126+
}
127+
const filenames = extractFilenames(allFiles, page, sourcePath);
128+
filenames.forEach(f => modifiedFilenames.add(f));
129+
}
130+
if (page >= MAX_PAGE) {
131+
core.warning(`The pull request ${eventData.number} is too big - not all changed files will be analyzed!`);
132+
}
133+
134+
return [...modifiedFilenames];
135+
} else if (context.eventName === 'push') {
136+
core.debug(`Push on ${eventData.ref}: ${eventData.before}...${eventData.after}`);
137+
138+
let modifiedFilenames = new Set();
139+
140+
// maximum of 300 files are loaded (page size is 30, max 10 pages)
141+
let page;
142+
for(page = 1; page <= MAX_PAGE; page++) {
143+
const compareResponse = await octokit.rest.repos.compareCommitsWithBasehead({
144+
...context.repo,
145+
basehead: `${eventData.before}...${eventData.after}`,
146+
per_page: 30,
147+
page: page
148+
});
149+
const allFiles = compareResponse.data.files;
150+
if (allFiles === undefined || allFiles.length == 0) {
151+
break;
152+
}
153+
const filenames = extractFilenames(allFiles, page, sourcePath);
154+
filenames.forEach(f => modifiedFilenames.add(f));
155+
}
156+
if (page >= MAX_PAGE) {
157+
core.warning(`The push on ${eventData.ref} is too big - not all changed files will be analyzed!`);
158+
}
159+
160+
return [...modifiedFilenames];
161+
} else {
162+
core.warning(`Unsupported github action event '${context.eventName}' - cannot determine modified files. All files will be analyzed.`);
163+
return undefined;
164+
}
165+
}
166+
167+
function extractFilenames(allFiles, page, sourcePath) {
168+
core.debug(` got ${allFiles.length} entries from page ${page} to check...`);
169+
if (core.isDebug()) {
170+
// output can be enabled by adding repository secret "ACTIONS_STEP_DEBUG" with value "true".
171+
for (let i = 0; i < allFiles.length; i++) {
172+
core.debug(` ${i}: ${allFiles[i].status} ${allFiles[i].filename}`);
173+
}
174+
}
175+
const filenames = allFiles
176+
.filter(f => f.status === 'added' || f.status === 'changed' || f.status === 'modified')
177+
.map(f => path.normalize(f.filename))
178+
.filter(f => sourcePath === '.' || f.startsWith(sourcePath));
179+
if (core.isDebug()) {
180+
core.debug(` after filtering by status and with '${sourcePath}' ${filenames.length} files remain:`);
181+
core.debug(` ${filenames.join(', ')}`);
182+
}
183+
return filenames;
184+
}
185+
83186
module.exports.downloadPmd = downloadPmd;
84187
module.exports.executePmd = executePmd;
188+
module.exports.determineModifiedFiles = determineModifiedFiles;

‎tests/data/compare-files-page1.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
3+
"status": "ahead",
4+
"ahead_by": 2,
5+
"behind_by": 0,
6+
"total_commits": 2,
7+
"commits": [],
8+
"files": [
9+
{
10+
"sha": "879c25e370bb62b991c95adf0e4becf220d394fb",
11+
"filename": "src/main/java/AvoidCatchingThrowableSample.java",
12+
"status": "modified",
13+
"additions": 2,
14+
"deletions": 1,
15+
"changes": 3,
16+
"blob_url": "https://github.com/pmd/pmd-github-action-tests/blob/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
17+
"raw_url": "https://github.com/pmd/pmd-github-action-tests/raw/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
18+
"contents_url": "https://api.github.com/repos/pmd/pmd-github-action-tests/contents/src/main/java/AvoidCatchingThrowableSample.java?ref=cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f",
19+
"patch": "@@ -1,9 +1,10 @@\n public class AvoidCatchingThrowableSample {\n+ \n public void bar() {\n try {\n // do something\n } catch (Throwable th) { // should not catch Throwable\n th.printStackTrace();\n }\n }\n-}\n\\ No newline at end of file\n+}"
20+
},
21+
{
22+
"filename": "src/main/java/DeletedFile.java",
23+
"status": "removed"
24+
},
25+
{
26+
"filename": "src/main/java/NewFile.java",
27+
"status": "added"
28+
},
29+
{
30+
"filename": "src/main/java/ChangedFile.java",
31+
"status": "changed"
32+
},
33+
{
34+
"filename": "README.md",
35+
"status": "modified"
36+
}
37+
]
38+
}

‎tests/data/compare-files-page2.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
3+
"status": "ahead",
4+
"ahead_by": 2,
5+
"behind_by": 0,
6+
"total_commits": 2,
7+
"commits": []
8+
}

‎tests/data/pmd-bin-6.39.0.zip‎

325 Bytes
Binary file not shown.

‎tests/data/pmd-bin-6.39.0/pmd.bat‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
@echo off
2+
echo Running PMD 6.39.0 with: pmd %*
3+
4+
(
5+
echo {
6+
echo "runs": [
7+
echo {
8+
echo "tool": {
9+
echo "driver": {
10+
echo "name": "PMD",
11+
echo "version": "6.39.0"
12+
echo }
13+
echo }
14+
echo }
15+
echo ]
16+
echo }
17+
)>"pmd-report.sarif"

0 commit comments

Comments
 (0)