sfdx-scan-pull-request icon indicating copy to clipboard operation
sfdx-scan-pull-request copied to clipboard

Not able to comment violation on First Line when modified

Open gyadav4 opened this issue 1 year ago • 8 comments

Steps to reproduce:

  1. Update any exisiting Apex class name, which is already in master eg May be from Example to example
  2. Run npx sfdx scanner:run --target "force-app/main/default/classes/Example2.cls" --json from local which will give violations

{ "line": 1, "column": 27, "endLine": 6, "endColumn": 1, "severity": 1, "ruleName": "ClassNamingConventions", "category": "Code Style", "url": "https://pmd.github.io/pmd-6.55.0/pmd_rules_apex_codestyle.html#classnamingconventions", "message": "\nThe class name 'example2' doesn't match '[A-Z][a-zA-Z0-9_]*'\n" }

  1. Check-in code, raise PR - PR is passed without violation

PR Action config used uses: mitchspano/[email protected] with: report-mode: comments severity-threshold: 3

But it is catching up in newly created apex class

gyadav4 avatar Mar 22 '24 11:03 gyadav4

This is to be expected. The way that the scan works is by filtering the findings to only lines which are modified in the pull request. For a simple rename, the scan should not result in publication of findings.

The GitHub REST API actually restricts our ability to create a comment on a line which is not modified.

mitchspano avatar Mar 22 '24 13:03 mitchspano

But rename happened in same Pull request

gyadav4 avatar Mar 22 '24 13:03 gyadav4

If line 1 was not modified, then the scan will not produce a comment on that line

mitchspano avatar Mar 22 '24 13:03 mitchspano

Yes it was modified. Initially force-app/main/default/classes/Example2.cls

public with sharing class Example2 { public Example2() { Integer i = 0; i+=1; } }


Developer Updated to force-app/main/default/classes/Example2.cls

public with sharing class example2 { --> Updated class name, so line 1 modified public Example2() { Integer i = 0; i+=1; } }

Updated Comment: Nothing else was modified. If modifying other lines, then it is making comment for line1 as well

gyadav4 avatar Mar 22 '24 13:03 gyadav4

Ahh okay I understand now. I thought you were just modifying the file name. I'll take a look at this over the weekend

mitchspano avatar Mar 22 '24 13:03 mitchspano

Perfect ! Let me know any other info needed, in b/w great library upgrading CI/CD for my org, using it for better code quality, and first scan even before review :)

gyadav4 avatar Mar 22 '24 13:03 gyadav4

Oh I see what's going on. The only line which was modified is line 1, but if you check the finding, it is from line 1-6.

In this action, we check for complete coverage before reporting a finding. This means that every single line that is within the finding must be modified for it to be reported as a comment. We do this because this guarantees that the GitHub rest API will not fail should a comment attempt to be reported, but because lines 2 through 6 are not part of the pull request, this will result in a rest API call out failure should we attempt to make a comment on it.

mitchspano avatar Mar 22 '24 13:03 mitchspano

We can give a try, I can rerun that same usecase to check !

gyadav4 avatar Mar 22 '24 14:03 gyadav4