Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[apex] Rule priorities don't conform to guidelines #3926

Open
jfeingold35 opened this issue Apr 19, 2022 Discussed in #3911 · 4 comments
Open

[apex] Rule priorities don't conform to guidelines #3926

jfeingold35 opened this issue Apr 19, 2022 Discussed in #3911 · 4 comments

Comments

@jfeingold35
Copy link
Contributor

Discussed in #3911

Originally posted by jfeingold35 April 14, 2022
The priorities assigned to Apex rules don't seem correct to me.
E.g., ClassNamingConventions is a code style rule whose priority is 1 (the highest value), as are FieldNamingConventions, and FormalParameterNamingConventions, while security rules like ApexCRUDViolation, ApexInsecureEndpoint, and ApexSharingViolations all have priority 3, which is the lowest value.
Is this intentional? If so, what's the historical reason for this being the case?

To provide more clarity here: The security rules all have priority 3, while code style rules such as those around naming conventions have priorities of 1.
Based on the guidelines around rule priorities, it seems like security rules should be priority 1 or 2 ("Change absolutely required"/"Change highly recommended"), and code style rules should be either 3, 4, or 5 ("Change recommended"/"Change optional"/"Change highly optional").
At the very least, it seems like reducing the priority of code style rules so it's below that of security rules would provide the most helpful output to users.

@adangel adangel changed the title Apex rule priorities don't conform to guidelines? [apex] Rule priorities don't conform to guidelines? Apr 21, 2022
@adangel
Copy link
Member

adangel commented Apr 21, 2022

To be honest, I think also for the java rules, the priorities there are not always "correct". I guess we mostly use 3 - which is the golden mean - and don't think too much about it.
The use case I know for rule priorities is this: You have some rules, which will break the build - and some rules which don't and are only reported. It usually depends on the project, how you see the rules. For one project, code style rules could be considered absolutely strict and no code is allowed to go into production, which doesn't adhere to the code styles. In another project, the same rule could be tolerable. One could also argue, that if a build doesn't break, then the violations are anyway not considered, so you could disable the rule completely. This means, one would not need any priority - all rules are equally important.

I agree though, that the priorities could have a much "saner" default value.

@oowekyala
Copy link
Member

Maybe we should use 3 consistently in all category rulesets? I think the end user should be in charge here

@rsoesemann
Copy link
Member

@oowekyala very pragmatic approach which I like. Also tool-wise a good idea as IDEs like Illuminated Cloud or VS Code ApexPMD show yellow lines for prior 3 and nothing for below numbers. Red for 1.

I think yellow for PMD issues is good and people can then customize security category to be red and 1.

@adangel adangel changed the title [apex] Rule priorities don't conform to guidelines? [apex] Rule priorities don't conform to guidelines Apr 29, 2022
@rsoesemann
Copy link
Member

@jfeingold35 @oowekyala @adangel any decision made here? Why not really make everything a 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants