forcedotcom / sfdx-scanner

MIT License
212 stars 49 forks source link

Allow plug-in to leverage xml metadata rules #571

Closed rsoesemann closed 2 years ago

rsoesemann commented 2 years ago

In the recent PMD Versions, there are some great additions for SF Devs that I am unsure if the CLI Scanner fully supports them.

The biggest one is Support for Custom XPath rules that do not check Apex but arbitrary XML metadata like this.image

Feel free to use a clone of this repo to test it

https://github.com/rsoesemann/unhappy-soup

rsoesemann commented 2 years ago

FYI @rmohan20 @jbartolotta-sfdc

rmohan20 commented 2 years ago

Hi @rsoesemann - thanks for notifying us about this feature. We'll try it out and update you here.

jfeingold35 commented 2 years ago

Hi @rsoesemann . Thanks for bringing this issue to our attention, and submitting test code for us. We've tested our custom rule support, and determined that the XPath rules you submitted do work properly. As per our documentation, you can define the new rules in a category file, put that xml in a directory structure matching PMD's standards (/category/<language>/<file>.xml), then you can turn that directory into a JAR. Then, you can use scanner:rule:add --language xml --path path/to/your/jar to add the rules. Finally, if you want the rules to run against the -meta.xml files, you'll need to modify the ~/.sfdx-scanner/Config.json, removing the !**/*-meta.xml entry in PMD's targetPatterns array.

rsoesemann commented 2 years ago

@jfeingold35 thanks for your response.

What I don't undertstand or what I found improvable is why I need to compile custom rules into a jar before I can use them. The whole point of custom rules in the ruleset.xml is that team members can modify them, commit them and then use them without any admin of the CI process to compile something. I mean make it work like it would work with PMD on the command line. Otherwise

Is this technically feasable? I believe many PMD users are currently wanting to leverage custom rules and need tool support for them.

jfeingold35 commented 2 years ago

Hi @rsoesemann . Thanks for your feedback. This feature was originally designed with the assumption that Java-based custom rules and their XML descriptors would be contained in JAR files. However, it turns out that the scanner will accommodate standalone XML files consisting exclusively of XPath-based rules.

To make your file compatible with the scanner, you could move the rule definitions into a /category/xml/metadata-rules.xml file, and the rule references into a /rulesets/apex/mega-ruleset.xml file.

After registering the files through scanner:rule:add once, users should be able to safely make modifications to the contents of the XML files, as long as their locations do not change. We’ll continue to explore ways to make this more flexible in future releases. Please try this solution and let us know if it meets your needs.

jfeingold35 commented 2 years ago

For future readers of this thread, the criteria for standalone XML files are as follows:

  1. Standalone XML files must be either a ruleset or a category, and cannot be both. We use PMD’s internal convention for differentiating categories and rulesets, namely checking the path for either /category/ or /rulesets/ to indicate how the file should be classified.
  2. Categories should be used to define new rules, and rulesets should consist of references to existing rules.
  3. Rules defined within standalone XML files must be XPath rules. If custom Java is used, then a JAR is required to ensure that the custom classes are properly added to the classpath.
  4. Rules defined in standalone XML category files cannot be referenced by custom rulesets, because bundling the category in a JAR is what allows the ruleset’s ref tags to work properly.

Also, please be aware that, since PMD has deprecated rulesets and plans to remove them in v7, we have deprecated rulesets as well, and have long-term plans to remove support for them.

Our documentation will be updated to reflect this information, but we’re also leaving it here for future reference.

rsoesemann commented 2 years ago

@jfeingold35 thanks for those detail. Do you have an example repo showing such a split between Custom Rules XML and the actual ruleset. Maybe I should wait for this documentation but I am just so keen on getting this to work.

Maybe you can just describe how to split https://github.com/rsoesemann/unhappy-soup/blob/master/ruleset.xml and where to put the second xml.

jfeingold35 commented 2 years ago

@rsoesemann , no problem.

For the custom rules you defined in lines 26-188, we recommend the following:

For the standard Apex rules referenced in lines 8-21, we recommend the following:

At this point, you can verify that everything worked properly by running sfdx scanner:rule:list --language xml,apex and inspecting the results.

jfeingold35 commented 2 years ago

@rsoesemann , we released a new version of the plugin yesterday, and it includes the aforementioned changes to the documentation. I'm going to mark this issue closed. Please feel free to open a new one if needed. Thanks!

rsoesemann commented 2 years ago

Sorry for my ignorance but where is this updated documentation @jfeingold35? Can't find it.