mitchspano / sfdx-scan-pull-request

Runs sfdx-scanner on a pull request and generates in-line comments with the findings.
Apache License 2.0
73 stars 25 forks source link

Scanner breaks when invoked from branch commit/push #63

Closed leojok-fluido closed 1 year ago

leojok-fluido commented 1 year ago
Run mitchspano/sfdx-scan-pull-request@v0.1.12
Beginning sfdx-scan-pull-request run...
Validating that this action was invoked from an acceptable context...
Getting difference within the pull request ... { baseRef: undefined, headRef: undefined }
fatal: ambiguous argument 'destination/undefined...origin/undefined': unknown revision or path not in the working tree.

Code index.ts sets pullRequest: context?.payload?.pull_request in line 70, then line 223 breaks due to pullRequest being null/undefined: const filePathToChangedLines = await getDiffInPullRequest( pullRequest?.base?.ref, pullRequest?.head?.ref, pullRequest?.base?.repo?.clone_url );

GHA yml

- name: Run SFDX Scanner
  uses: mitchspano/sfdx-scan-pull-request@v0.1.12
  with:
    pmdconfig: config/pmd-rules.xml
    target: $GITHUB_WORKSPACE/force-app/main/default/classes/*.cls
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Tried different variations of target such as:

mitchspano commented 1 year ago

Thanks for checking out the action. Can you try updating to the latest version (0.1.14) and let me know if the issue persists?

leojok-fluido commented 1 year ago

@mitchspano : I have now switched to latest release version, getting the same result since code expects to found pullrequest.base/head.ref regardless is the target parameter passed or not when starting from branch commit/push:

Run mitchspano/sfdx-scan-pull-request@v0.1.14
Beginning sfdx-scan-pull-request run...
Validating that this action was invoked from an acceptable context...
Getting difference within the pull request ... { baseRef: undefined, headRef: undefined }
fatal: ambiguous argument 'destination/undefined...origin/undefined': unknown revision or path not in the working tree.
mitchspano commented 1 year ago

I was able to track down the source of the issue and will be raising a PR and creating a new version soon.

I tested the fix by making the following class:

global without sharing class MyClass {
    public MyClass() {
    }

    global void foo(){
        String unused = 'I think this class has some issues...';
        for (Integer i=0; i<5; i++) {
            insert new Account(name = 'Acme '+ i);
        }
    }
}

And I executed the action with the following configuration:

- name: Run SFDX Scanner - Report findings as comments
  uses: mitchspano/sfdx-scan-pull-request@fixTargetDefinition
  with:
    severity-threshold: 4
    target: force-app/main/default/classes/*.cls
    strictly-enforced-rules: '[{ "engine": "pmd", "category": "Documentation", "rule": "ApexDoc" }]'
    custom-pmd-rules: '[{ "path": "target/pmd-rules-1.0.0.jar", "language": "apex"}]'
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
image

I also tested with report mode of comments and that is working as expected:

- name: Run SFDX Scanner - Report findings as comments
  uses: mitchspano/sfdx-scan-pull-request@fixTargetDefinition
  with:
    report-mode: comments
    severity-threshold: 4
    target: force-app/main/default/classes/*.cls
    strictly-enforced-rules: '[{ "engine": "pmd", "category": "Documentation", "rule": "ApexDoc" }]'
    custom-pmd-rules: '[{ "path": "target/pmd-rules-1.0.0.jar", "language": "apex"}]'
  env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
image
leojok-fluido commented 1 year ago

Thanks @mitchspano . Latest version was able to go further in case of commit/push to branch. However now I faced another limitation/feature that PMD found 1899 issues in current codebase and GitHub rejected the API call due to "Invalid request.\n\nNo more than 50 items are allowed; 1899 were supplied."

Error when creating check run: {
  "name": "HttpError",
  "status": 422,
  "response": {
    "url": "https://api.github.com/repos/xxx/yyy/check-runs",
    "data": {
      "message": "Invalid request.\n\nNo more than 50 items are allowed; 1899 were supplied.",
      "documentation_url": "https://docs.github.com/rest/checks/runs#create-a-check-run"
    }
  }

One enhancement would be to post first 50 PMD issues to check-runs endpoint rather than the GHA step showing "false positive"?

mitchspano commented 1 year ago

The potential to exhaust the GitHub REST API limit is a known issue documented in #6.

As a side note - the action was never really intended to scan the entire codebase; it was really meant to scan the contents of a pull request to assist with the code review process. Support for running on a commit with a specified target came later, but only exacerbated the REST API limit issue :/

At this point in time if you are looking to scan the entire codebase, I would recommend just running the sf scanner tool locally:

npm i @salesforce/cli -g
sf plugins:install @salesforce/sfdx-scanner
sf scanner:run --target "force-app/main/default/classes" > scannerFindings.txt
leojok-fluido commented 1 year ago

Thanks Mitch. I will then gonna use my own GHA plugin from the past, just release new version of it to have PMD 6.55: https://github.com/marketplace/actions/pmd-salesforce-apex-code-analyzer-action Benefit of scanning the whole project is to validate both uat and main branches being 100% valid as per PMD rules.

Will keep using your plugin only for pull requests.

leojok-fluido commented 1 year ago

@mitchspano : I have now upgraded my own GHA plugin into PMD 7.0.0-rc3. Updated also readme with example to promote your action for pull-request apex validations.

https://github.com/marketplace/actions/pmd-salesforce-apex-code-analyzer-action