Determine modified files for pull requests and pushes

.github/workflows/test.yml

@@ -12,8 +12,16 @@ on:
 jobs:
   # unit tests
   unit:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ ubuntu-latest, windows-latest, macos-latest ]
+      fail-fast: false
     steps:
     - uses: actions/checkout@v2
+    - name: Set Node.js 12.x
+      uses: actions/setup-node@v2.5.0
+      with:
+        node-version: 12.x
     - run: npm ci
     - run: npm test

README.md

@@ -74,6 +74,7 @@ See also [Uploading a SARIF file to GitHub](https://docs.github.com/en/code-secu
 |`version`   |no |"latest"|PMD version to use. Using "latest" automatically downloads the latest version.<br>Available versions: https://github.com/pmd/pmd/releases|
 |`sourcePath`|no |"."     |Root directory for sources. Uses by default the current directory|
 |`rulesets`  |yes|        |Comma separated list of ruleset names to use.|
+|`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.|
 
 ## Outputs
 

action.yml

@@ -29,6 +29,22 @@ inputs:
   rulesets:
     description: Comma separated list of ruleset names to use
     required: true
+  analyzeModifiedFilesOnly:
+    description: >-
+      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.
+
+      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.
+
+      If the change is very big, not all files might be analyzed. Currently the
+      maximum number of modified files is 300.
+    required: false
+    default: true
 outputs:
   violations:
     description: Number of violations found

lib/index.js

@@ -9,14 +9,27 @@ const reportFormat = 'sarif';
 const reportFile = 'pmd-report.sarif'
 
 async function main() {
-  let pmdInfo, execOutput, violations;
+  let pmdInfo, modifiedFiles, execOutput, violations;
+  let token = core.getInput('token', { required: true });
+  let sourcePath = validator.validateSourcePath(core.getInput('sourcePath', { required: true }));
   try {
     pmdInfo = await util.downloadPmd(
       validator.validateVersion(core.getInput('version'), { required: true }),
-      core.getInput('token', { required: true })
+      token
     );
+
+    if (core.getInput('analyzeModifiedFilesOnly', { required: true }) === 'true') {
+      core.info(`Determining modified files in ${sourcePath}...`);
+      modifiedFiles = await util.determineModifiedFiles(token, sourcePath);
+      if (modifiedFiles !== undefined && modifiedFiles.length === 0) {
+        core.info(`No modified files have been found in ${sourcePath} - exiting`);
+        core.setOutput('violations', 0);
+        return;
+      }
+    }
+
     execOutput = await util.executePmd(pmdInfo,
-      validator.validateSourcePath(core.getInput('sourcePath', { required: true })),
+      modifiedFiles || sourcePath,
       validator.validateRulesets(core.getInput('rulesets', { required: true })),
       reportFormat, reportFile)
 

lib/util.js

@@ -4,6 +4,13 @@ const tc = require('@actions/tool-cache');
 const exec = require('@actions/exec');
 const semver = require('semver');
 const os = require('os');
+const fs = require('fs').promises;
+const path = require('path');
+
+// Load at most MAX_PAGE pages when determining modified files.
+// This is used both for pull/{pull_number}/files as well as for
+// repos/compareCommits API calls.
+const MAX_PAGE = 10;
 
 const downloadPmd = async function(version, token) {
     let pmdVersion = version;
@@ -20,19 +27,29 @@ const downloadPmd = async function(version, token) {
     core.info(`Using PMD ${pmdVersion} from cached path ${cachedPmdPath}`);
     return {
         version: pmdVersion,
-        path: `${cachedPmdPath}/pmd-bin-${pmdVersion}`
+        path: path.join(cachedPmdPath, `pmd-bin-${pmdVersion}`)
     }
 }
 
-const executePmd = async function(pmdInfo, sourcePath, ruleset, reportFormat, reportFile) {
+const executePmd = async function(pmdInfo, fileListOrSourcePath, ruleset, reportFormat, reportFile) {
     let pmdExecutable = '/bin/run.sh pmd';
     if (os.platform() === 'win32') {
         pmdExecutable = '\\bin\\pmd.bat';
     }
+
+    let sourceParameter = ['-d', fileListOrSourcePath];
+    if (Array.isArray(fileListOrSourcePath)) {
+        await writeFileList(fileListOrSourcePath);
+        sourceParameter = [useNewArgsFormat(pmdInfo.version) ? '--file-list' : '-filelist', 'pmd.filelist'];
+        core.info(`Running PMD ${pmdInfo.version} on ${fileListOrSourcePath.length} modified files...`);
+    } else {
+        core.info(`Running PMD ${pmdInfo.version} on all files in path ${fileListOrSourcePath}...`);
+    }
+
     const execOutput = await exec.getExecOutput(`${pmdInfo.path}${pmdExecutable}`,
         [
             useNewArgsFormat(pmdInfo.version) ? '--no-cache' : '-no-cache',
-            '-d', sourcePath,
+            ...sourceParameter,
             '-f', reportFormat,
             '-R', ruleset,
             '-r', reportFile,
@@ -80,5 +97,92 @@ async function getDownloadURL(release) {
     return asset.browser_download_url;
 }
 
+async function writeFileList(fileList) {
+    await fs.writeFile(path.join('.', 'pmd.filelist'), fileList.join(','), 'utf8');
+}
+
+const determineModifiedFiles = async function(token, sourcePath) {
+    // creating new context instead of using "github.context" to reinitialize for unit testing
+    const context = new github.context.constructor();
+    const eventData = context.payload;
+    const octokit = github.getOctokit(token);
+    if (context.eventName === 'pull_request') {
+        core.debug(`Pull request ${eventData.number}: ${eventData.pull_request.html_url}`);
+
+        let modifiedFilenames = new Set();
+
+        // maximum of 300 files are loaded (page size is 30, max 10 pages)
+        let page;
+        for(page = 1; page <= MAX_PAGE; page++) {
+            const listFilesResponse = await octokit.rest.pulls.listFiles({
+                ...context.repo,
+                pull_number: eventData.number,
+                per_page: 30,
+                page: page
+            });
+            const allFiles = listFilesResponse.data;
+            if (allFiles.length == 0) {
+                break;
+            }
+            const filenames = extractFilenames(allFiles, page, sourcePath);
+            filenames.forEach(f => modifiedFilenames.add(f));
+        }
+        if (page >= MAX_PAGE) {
+            core.warning(`The pull request ${eventData.number} is too big - not all changed files will be analyzed!`);
+        }
+
+        return [...modifiedFilenames];
+    } else if (context.eventName === 'push') {
+        core.debug(`Push on ${eventData.ref}: ${eventData.before}...${eventData.after}`);
+
+        let modifiedFilenames = new Set();
+
+        // maximum of 300 files are loaded (page size is 30, max 10 pages)
+        let page;
+        for(page = 1; page <= MAX_PAGE; page++) {
+            const compareResponse = await octokit.rest.repos.compareCommitsWithBasehead({
+                ...context.repo,
+                basehead: `${eventData.before}...${eventData.after}`,
+                per_page: 30,
+                page: page
+            });
+            const allFiles = compareResponse.data.files;
+            if (allFiles === undefined || allFiles.length == 0) {
+                break;
+            }
+            const filenames = extractFilenames(allFiles, page, sourcePath);
+            filenames.forEach(f => modifiedFilenames.add(f));
+        }
+        if (page >= MAX_PAGE) {
+            core.warning(`The push on ${eventData.ref} is too big - not all changed files will be analyzed!`);
+        }
+
+        return [...modifiedFilenames];
+    } else {
+        core.warning(`Unsupported github action event '${context.eventName}' - cannot determine modified files. All files will be analyzed.`);
+        return undefined;
+    }
+}
+
+function extractFilenames(allFiles, page, sourcePath) {
+    core.debug(` got ${allFiles.length} entries from page ${page} to check...`);
+    if (core.isDebug()) {
+        // output can be enabled by adding repository secret "ACTIONS_STEP_DEBUG" with value "true".
+        for (let i = 0; i < allFiles.length; i++) {
+            core.debug(`   ${i}: ${allFiles[i].status} ${allFiles[i].filename}`);
+        }
+    }
+    const filenames = allFiles
+        .filter(f => f.status === 'added' || f.status === 'changed' || f.status === 'modified')
+        .map(f => path.normalize(f.filename))
+        .filter(f => sourcePath === '.' || f.startsWith(sourcePath));
+    if (core.isDebug()) {
+        core.debug(`   after filtering by status and with '${sourcePath}' ${filenames.length} files remain:`);
+        core.debug(`   ${filenames.join(', ')}`);
+    }
+    return filenames;
+}
+
 module.exports.downloadPmd = downloadPmd;
 module.exports.executePmd = executePmd;
+module.exports.determineModifiedFiles = determineModifiedFiles;

tests/data/compare-files-page1.json

@@ -0,0 +1,38 @@
+{
+    "url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
+    "status": "ahead",
+    "ahead_by": 2,
+    "behind_by": 0,
+    "total_commits": 2,
+    "commits": [],
+    "files": [
+        {
+            "sha": "879c25e370bb62b991c95adf0e4becf220d394fb",
+            "filename": "src/main/java/AvoidCatchingThrowableSample.java",
+            "status": "modified",
+            "additions": 2,
+            "deletions": 1,
+            "changes": 3,
+            "blob_url": "https://github.com/pmd/pmd-github-action-tests/blob/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
+            "raw_url": "https://github.com/pmd/pmd-github-action-tests/raw/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
+            "contents_url": "https://api.github.com/repos/pmd/pmd-github-action-tests/contents/src/main/java/AvoidCatchingThrowableSample.java?ref=cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f",
+            "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+}"
+        },
+        {
+            "filename": "src/main/java/DeletedFile.java",
+            "status": "removed"
+        },
+        {
+            "filename": "src/main/java/NewFile.java",
+            "status": "added"
+        },
+        {
+            "filename": "src/main/java/ChangedFile.java",
+            "status": "changed"
+        },
+        {
+            "filename": "README.md",
+            "status": "modified"
+        }
+    ]
+}

tests/data/compare-files-page2.json

@@ -0,0 +1,8 @@
+{
+    "url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
+    "status": "ahead",
+    "ahead_by": 2,
+    "behind_by": 0,
+    "total_commits": 2,
+    "commits": []
+}

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

@@ -0,0 +1,17 @@
+@echo off
+echo Running PMD 6.39.0 with: pmd %*
+
+(
+    echo {
+    echo   "runs": [
+    echo     {
+    echo       "tool": {
+    echo         "driver": {
+    echo           "name": "PMD",
+    echo           "version": "6.39.0"
+    echo         }
+    echo       }
+    echo     }
+    echo   ]
+    echo }
+)>"pmd-report.sarif"

tests/data/pmd-bin-6.40.0/bin/pmd.bat

@@ -0,0 +1,17 @@
+@echo off
+echo Running PMD 6.40.0 with: pmd %*
+
+(
+    echo {
+    echo   "runs": [
+    echo     {
+    echo       "tool": {
+    echo         "driver": {
+    echo           "name": "PMD",
+    echo           "version": "6.40.0"
+    echo         }
+    echo       }
+    echo     }
+    echo   ]
+    echo }
+)>"pmd-report.sarif"

tests/data/pmd-bin-6.41.0/bin/pmd.bat

@@ -0,0 +1,17 @@
+@echo off
+echo Running PMD 6.41.0 with: pmd %*
+
+(
+    echo {
+    echo   "runs": [
+    echo     {
+    echo       "tool": {
+    echo         "driver": {
+    echo           "name": "PMD",
+    echo           "version": "6.41.0"
+    echo         }
+    echo       }
+    echo     }
+    echo   ]
+    echo }
+)>"pmd-report.sarif"

tests/data/pull-request-event-data.json

@@ -0,0 +1,11 @@
+{
+    "action": "opened",
+    "number": 1,
+    "pull_request": {
+        "html_url": "https://github.com/pmd/pmd-github-action-tests/pull/1",
+        "diff_url": "https://github.com/pmd/pmd-github-action-tests/pull/1.diff",
+        "number": 1,
+        "state": "open",
+        "title": "Test"
+    }
+}

tests/data/pull-request-files.json

@@ -0,0 +1,30 @@
+[
+    {
+        "sha": "879c25e370bb62b991c95adf0e4becf220d394fb",
+        "filename": "src/main/java/AvoidCatchingThrowableSample.java",
+        "status": "modified",
+        "additions": 2,
+        "deletions": 1,
+        "changes": 3,
+        "blob_url": "https://github.com/pmd/pmd-github-action-tests/blob/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
+        "raw_url": "https://github.com/pmd/pmd-github-action-tests/raw/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
+        "contents_url": "https://api.github.com/repos/pmd/pmd-github-action-tests/contents/src/main/java/AvoidCatchingThrowableSample.java?ref=cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f",
+        "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+}"
+    },
+    {
+        "filename": "src/main/java/DeletedFile.java",
+        "status": "removed"
+    },
+    {
+        "filename": "src/main/java/NewFile.java",
+        "status": "added"
+    },
+    {
+        "filename": "src/main/java/ChangedFile.java",
+        "status": "changed"
+    },
+    {
+        "filename": "README.md",
+        "status": "modified"
+    }
+]

tests/data/push-event-data.json

@@ -0,0 +1,5 @@
+{
+    "ref": "refs/heads/java",
+    "before": "44c1557134c7dbaf46ecdf796fb871c8df8989e4",
+    "after": "8a7a25638d8ca5207cc824dea9571325b243c6a1"
+}