search

forcedotcom / sfdx-scanner

MIT License
215 stars 49 forks source link

[BUG] SFGE reports false positives for constructors only called within a given class #1001

Closed jamessimone closed 1 year ago

jamessimone commented 1 year ago

Describe the bug

Running the command:

sfdx scanner:run -e sfge -p rollup,plugins/RollupCallback --target 'rollup,plugins,extra-tests,!plugins/ExtraCodeCoverage/**'

In my repo leads to false positives being reported where private/protected constructors are being called:

{
  "result": [
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupCalculator.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupCalculator is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 100,
          "column": 13
        },
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupCalculator.GroupByCalculator is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 1286,
          "column": 13
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupAsyncProcessor.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupAsyncProcessor.QueueableProcessor is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 366,
          "column": 13
        },
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupAsyncProcessor.QueueableProcessor is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 398,
          "column": 13
        },
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupAsyncProcessor.QueueableProcessor is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 370,
          "column": 13
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupMetaPicklists.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupMetaPicklists is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 39,
          "column": 11
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupRelationshipFieldFinder.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupRelationshipFieldFinder.Traversal is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 85,
          "column": 13
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/Rollup.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class Rollup.FlowInputWrapper is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 702,
          "column": 13
        },
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class Rollup.QueryWrapper is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 2453,
          "column": 13
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupLogger.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupLogger is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 10,
          "column": 13
        }
      ]
    },
    {
      "engine": "sfge",
      "fileName": "./rollup/rollup/core/classes/RollupParentResetProcessor.cls",
      "violations": [
        {
          "ruleName": "UnusedMethodRule",
          "message": "Method <init> in class RollupParentResetProcessor.QueueableResetProcessor is never invoked",
          "severity": 3,
          "category": "Performance",
          "url": "https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#UnusedMethodRule",
          "line": 11,
          "column": 13
        }
      ]
    }
  ]
}

To Reproduce

Run the command above.

Expected behavior

These should not be flagged as scan violations because in each and every case the constructor is actually being called.

Desktop (please complete the following information):

Additional context

The graph engine also chokes if you have multiple files with the same name; why do we need to specific the --target and a --project if the ignore globs in the target arg are going to be ignored? For example, I have a git ignored directory, plugins/ExtraCodeCoverage which I use to generate an unlocked package for orgs that need additional code coverage, but because the classes in this plugin stem from a directory I also want to scan (extra-tests), I can't use the following command:

sfdx scanner:run -e sfge -p rollup,plugins,extra-tests --target 'rollup,plugins,extra-tests,!plugins/ExtraCodeCoverage/**'

And instead have to explicitly list out only the directories in my plugins folder that I do want to scan, which seems duplicative (or, at least, I would expect the ignore glob patterns in the --target arg to be applied properly):

sfdx scanner:run -e sfge -p rollup,plugins/CustomObjectRollupLogger,plugins/NebulaLogger,plugins/RollupCallback,extra-tests --target rollup,plugins,extra-tests
jfeingold35 commented 1 year ago

@jamessimone , thanks for bringing this to our attention. In our upcoming April release, we plan to enhance the functionality of this rule. We'll do our best to address these false positives as well. Regarding your additional question, the --projectdir flag is necessary because GraphEngine needs to construct a graph of the entire project in order to properly run its rules, not merely the specific files in the project that are actually being scanned.

jamessimone commented 1 year ago

@jfeingold35 I'm of the mind that .gitignore paths should be ignored when --projectdir is used, if that's the case. In other words - if I'm using . as my project dir, I don't expect to be getting scan violations from within the scanner's own test files located in node_modules.

jfeingold35 commented 1 year ago

@jamessimone , this is definitely a useful discussion to have, but since it's entering the realm of a feature request, could you please log a separate issue for that and we can discuss it there?

jamessimone commented 1 year ago

I'm happy to log another bug; I want to use that word explicitly though because it's not a feature request. If the scanner is reporting false positives - which are explicitly there to aid in its own tests - it's a bug for those violations to be included in a scan when targeting the root project directory.

jfeingold35 commented 1 year ago

To clarify, the reason I think that the discussion about --projectdir should be a separate Issue is because it's unrelated to the false positives that this issue covers. Those are definitely false positives, but their presence is unrelated to which files are or aren't covered by --projectdir.

jamessimone commented 1 year ago

Absolutely agree, and I apologize - I was just being a bit pedantic on the feature request/bug thing. I've created #1007 to document this issue.

git2gus[bot] commented 1 year ago

This issue has been linked to a new work item: W-12673043

rmohan20 commented 1 year ago

Changes are ready to ship in 3.11.0. Marking issue as closed.

jamessimone commented 1 year ago

Awesome, looking forward to it!