pmd icon indicating copy to clipboard operation
pmd copied to clipboard

[apex] Type declaration/casts/constructors have correct capitalization

Open aranwe opened this issue 1 year ago • 5 comments

Proposed Rule Name: Type has correct capitalization

Proposed Category: [Code Style]

Description: Check if Type declaration and constructors have proper capitalization according to SFDX class cache. Alternatively if not possible to implement at least that the Type conforms to the class naming regexp.

Also it might be good idea to split this into 2 rules -> 1 for constructors and 1 for Type declarations. (or maybe to 3 -> 1 for type casts).

Code Sample:

string s1; // gets flagged
String s2; // ok

account a1; // gets flagged
Account a2; // ok

Account a3 = new account(); // gets flagged
Account a4 = new Account(); // ok

list<Account> accounts1; //gets flagged
List<account> accounts2; // gets flagged
List<Account> accounts3; // OK

List<Account> accounts4 = (List<account>) new List<SObject>(); // gets flagged
List<Account> accounts5 = (List<Account>) new List<SObject>(); // gets flagged

Possible Properties:

  • can PMD check other files in the hierarchy? (as the list of classes with proper names is generated with SFDX)
  • it could be useful for some teams to be able to pass the list of sObjects & classes (with proper capitalization) or just regex

aranwe avatar Sep 06 '22 10:09 aranwe

@aranwe Totally love that Rule idea. There is so much chaos out there even in the official Salesforce sample code. Do you want me to show you how to write and contribute rules? I don't have the time to do it myself but I can get you started. Please let me know.

rsoesemann avatar Sep 21 '22 17:09 rsoesemann

@rsoesemann Yup, I can implement it, I actually already peeked into the source code and saw that I'd have to study too much code in order to do it solely by myself. But if you can give me a hand and point me in the right direction it would be great :)

(I have also checked if there is a pmd-java or pmd-java8 implementation but sadly no).

aranwe avatar Sep 22 '22 16:09 aranwe

Don't get me wrong it's been a while since I contribute rules. Here is a few things you need to do:

  • Set up a proper PMD build in your IDE using the guideline in https://pmd.sourceforge.io/pmd-6.49.0/pmd_devdocs_development.html
  • Use the Rule Designer to understand the current AST so you know which node types you need to visit in your rule.
  • I would start from the NamingConvention rules. Eighter the new Java Implementations or the older Apex version. Its a rule which basically does the same. Visit nearly all AST node types and check if they obey the rules.https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/ClassNamingConventionsRule.java
  • Keep those rules in mind https://pmd.sourceforge.io/pmd-6.49.0/pmd_devdocs_major_rule_guidelines.html

rsoesemann avatar Sep 22 '22 18:09 rsoesemann

A good tip is to look at another professional rule contribution like the Cognitive Complexity from Gearset. By looking a the thread of actions, the discussion with the PMD core maintainers and the code changes you see whats on your list ;-)

https://github.com/pmd/pmd/pull/2297

rsoesemann avatar Sep 22 '22 18:09 rsoesemann

Let me also pull in one of the smartest developer I know. @SCWells72 of the Illuminated Cloud. He has written and improved some PMD rules but also has written a full IDE for Salesforce.

Scott, sorry for putting you on the spot. But I thought you might be able to say:

  • Wether a rules that enforces official Naming besides Apex being Case-agnostic is valuable
  • If there is a simple, smart way to do this. I know in your IC IDE you have something similar.

rsoesemann avatar Sep 22 '22 18:09 rsoesemann

Hi. Yes, I implemented this in Illuminated Cloud (IC2). See "Usage Case Does Not Match Declaration Case" here.

Based on my understanding of PMD, what might make this difficult is knowing the correct case of the referenced identifier. IC2 adds live references to all identifier usages pointing at the respective declaration, and then my code inspection (JetBrains term for what PMD calls a rule) just needs to compare the usage case to the declaration case. Without that, you're going to be using heuristics to determine the correct case for any given identifier. Yes, if you constrain this to type names only you'll probably have an easier time because type names often use distinct AST node types vs. generic identifiers in reference expressions.

You're also going to need to take into account name scoping. For example, when you see Site as a type name, does that mean the SObject in the Schema namespace, the class in the System namespace, the inner class in System.Label, or the pseudo-inner Visualforce support class Component.Site? And that's one example of many.

The other difficulty will be creating and maintaining an authoritative set of known type names across both the system types, the local project types, and even types from installed managed packages that might be referenced by project code.

So there are simplifying assumptions you could make to have an implementation fall under the 80/20 rule, but to make this comprehensive (for type names only), you really need the authoritative list of type names available for use in the project and a reliable way to know for sure which actual type is referenced in any given situation.

It's worth pointing out that Java doesn't have this issue because it's a case-sensitive language. The existing naming convention rules are more prescriptive based on conventions and whether the existing code conforms to those conventions, but (I think) it only inspects names in declarations and not the usages of those names.

I hope that helps. Let me know if you have other questions.

SCWells72 avatar Sep 22 '22 19:09 SCWells72

Thanks guys! I will be offline for a week -> I am gonna look into this after I am back :)

Yeah at first I had in mind just custom classes & primitives, completely missed the platform classes & the inner classes. I guess scraping the Apex reference guide menu for Classes & Exceptions might do the trick and catch 80%, if there are some badly documented classes (which I am sure there are), lets just add those once reported (+ have property for manually adding classes in the meantime).

However for example System.Label is out of scope for this as its not instantiatable/assignable -> labels itself are Strings (but definitely good idea for another PMD check - however in order for that one to work, it would have to be able to parse SFDX xmls).

Btw: Didn't know that the cognitive load is from gearset - I love those guys :)

aranwe avatar Sep 23 '22 08:09 aranwe