forcedotcom / sfdx-scanner

MIT License
215 stars 49 forks source link

[BUG] Unable to run w/o eliminating level 3 errors [ sf scanner run dfa ] #1619

Closed wjanderson closed 1 month ago

wjanderson commented 1 month ago

Have you tried to resolve this issue yourself first?

Yes

Bug Description

I have 14 errors w/ level 3 . Unable to disable level 3 errors per statement: add an engine directive to skip the path

Cannot get the scanner to ignore the errors. Went back an validated.

Output / Logs

No response

Steps To Reproduce

  1. sf scanner run dfa --format csv --outfile CodeAnalyzerDFA.csv --target ./src/common/core/main/default/classes --projectdir ./src/common/core/main/default/classes/ --category Security --verbose
  2. review csv and sfge.log

Expected Behavior

Eliminate errors from output

Operating System

Windows 11 ( Version 10.0.22631)

Salesforce CLI Version

@salesforce/cli/2.37.4 win32-x64 node-v20.10.0

Code Analyzer Plugin (@salesforce/sfdx-scanner) Version

4.5.0

Java Version

penJDK 64-Bit Server VM Zulu17.50+19-CA (build 17.0.11+9-LTS, mixed mode, sharing)

Additional Context (Screenshots, Files, etc)

No response

Workaround

Have not found a way to sidestep the problem.

Urgency

Moderate

wjanderson commented 1 month ago

I did a drag and drop of the files but I do not see that they were added. I am attaching here CodeAnalyzerDFA.csv sfge.log CustomSettingController.cls.txt ApplicationACCC.cls.txt

stephen-carter-at-sf commented 1 month ago

Hmm I an unable to reproduce on my macOs machine.

@wjanderson Do you have access to another machine to see if you can reproduce this on a non-windows machine.

I just downloaded your CustomSettingController.cls.txt ApplicationACCC.cls.txt files and put them in a temporary folder called temp and renamed them so that they just have a .cls extension instead of a .cls.txt extension. Then when I run

sf scanner run dfa --target ./temp --projectdir ./temp --category Security --verbose

I see zero violations.

Maybe there are more files in your project that I'd need to reproduce?

stephen-carter-at-sf commented 1 month ago

I see in your CodeAnalyzerDFA.csv results that an internal error is what is being violated. The

/* sfge-disable-stack ApexFlsViolationRule */

directive only works to suppress a specific rule. I'm not sure if this will work but it is worth trying to do

/* sfge-disable-stack InternalExecutionError */

instead.

If that doesn't work, then I'm afraid there is most likely no engine directive for the Salesforce Graph Engine to suppress these errors. These errors that you are seeing are known issues (gaps really) in the Salesforce Graph Engine: https://github.com/forcedotcom/sfdx-scanner/issues/1497

Given the complexity of these types of issues, they may take quite a while for our team to eventually fix unfortunately. So for now you'll just need to ignore these errors.

jfeingold35 commented 1 month ago

The engine directive can only silence specific rules, and the internal execution error is being thrown during the construction of paths rather than during the execution of a specific rule. So unfortunately the directive won't be enough to silence the error here.

However, on inspecting the error message, it mentions line 234 of ApplicationACCC.cls, and the exception is associated with validateParameterSize, so it appears that return (IDBManager) getter.get(); is being incorrectly mistaken for a method that has a non-zero number of parameters. My suspicion is that it's related to the chained method call at line 232 (IApexObjectGetter getter = ApplicationACCC.creators().get(IDBManager.class);), since Graph Engine doesn't currently support this style of chaining. It's possible that splitting this into

SomeType creators = ApplicationACCC.creators();
IApexObjectGetter getter = creators.get(IDBManager.class);

could fix the problem.

If that doesn't work, then we're going to need to figure out how to reproduce the error in a simpler codebase, so we can do some debugging.

wjanderson commented 1 month ago

Thanks @stephen-carter-at-sf ,

As to the suggestion of /* sfge-disable-stack InternalExecutionError */ I had tried that as well but had to affect. Perhaps, the other files that may come into question are the the Repo classes, but , now seeing's Josh's suggestion, I will try his suggestion. Thanks.

@jfeingold35 (Josh) as to your reference I had gone back and disabled the class ( ApplicationACCC ) altogether when I saw it mentioned in the DefineType. But that had no affect. I will try your suggestion on unchaining the methods, Thanks.

wjanderson commented 1 month ago

@stephen-carter-at-sf & @jfeingold35 All good now! Thanks.

For Reference:

Django Unchained:

     Map<System.Type, blsw.IApexObjectGetter> getters = blsw.ApplicationACCC.creators();
     IApexObjectGetter getter = getters.get(IDBManager.class);

     return (IDBManager) getter.get();
wjanderson commented 1 month ago

Whoops ... Closing