Re: [pmd-devel] Custom Apex rule questions

[Juan M. S. D. <jua...@gm...>]

Hi Caleb,

I’m glad to see you are making progress. Are you planning to contribute
these rules back to PMD? If so, it will be easier / we will be far more
willing to help you out. Just submit a PR for each in such case and we will
comment directly on your code.

In the meantime, let me go in order:
ApexOptimizeSoqlRule:

I was planning on looking at ASTForEachStatement nodes, checking the
children for an ASTSoqlExpression node

I’m not sure I understand this… as far as I can tell, this is actually the
desired code, so there is no point ins searching for it (you only want to
add warnings for violations).

looking at ASTSoqlExpression nodes and checking their parents for a List<>
declaration

This is actually good. Once you get that List<> declaration, you need to
check if it is being used in a foreach and if so flag it.
ApexSingleSoqlResultRule

I am checking ASTSoqlExpression nodes. If their parent nodes include a
try/catch, no violation. Else, add violation.

2 things here. I believe you already thought about this, but the try may
not be a direct parent but some other ancestor of the node, and you have to
actually check if the exception being caught is a QueryException

I also check the ASTSoqlExpression for a LIMIT 1 since that’s how people
avoid the QueryException.

This is a must to avoid flagging all SOQL. Given ASTSoqlExpression node you
can do node.getNode().getCanonicalQuery() to obtain the contents of the
expression being used.
ApexCommentRule

can I just use a Java rule, like CommentRequiredRule?

No you can’t. The Java rules use the Java Lexer and Parser, that only
recognize Java code. You need Apex specific rules to do this.

Comments are not part of the AST, so doing this will not be trivial. I’m
not sure the Jorje parser provides access to comments at all, I’d have to
check the APIs and do some tests…
ApexReadabilityRule


   - Lines of code in a method exceed 50 => add to violation count

This is already implemented as apex-complexity/NcssMethodCount
<https://pmd.github.io/pmd-5.8.1/pmd-apex/rules/apex/complexity.html#NcssMethodCount>.
The default is 40 lines (without comments), but can be configured


   - Nested if statements exceed more than 3 => add to violation count

This is already implemented as apex-complexity/AvoidDeeplyNestedIfStmts
<https://pmd.github.io/pmd-5.8.1/pmd-apex/rules/apex/complexity.html#AvoidDeeplyNestedIfStmts>.
The default limit is exactly 3, you would have to set it to 4 to allow up
to 3.


   - Non trigger framework code exists in triggers => add to violation count

I’m not sure what this would be. I’m not an Apex developer. Given proper
samples I may assist you non the lest given I do know about it’s AST.

Regards,
​

On Thu, Jul 13, 2017 at 6:38 PM, Caleb Knox <cal...@en...>
wrote:

> Hi there folks,
>
> I am trying to implement a few more rules for the project I've emailed
> about before, before we send it off to our customers. This is working with
> Salesforce/Apex classes.
>
> Here they are:
>
>
>
> *ApexOptimizeSoqlRule*
> Example:
>
> List<Account> acct = […];
> for (Account a : acct ) {
>
> }
>
> // add violation
>
> -------------------------------------------
>
> // optimized version
>
> for (Account acct : [Select … ] ) {
>
> }
>
> // all good
>
> For this rule, I was planning on looking at ASTForEachStatement nodes,
> checking the children for an ASTSoqlExpression node, as well as looking at
> ASTSoqlExpression nodes and checking their parents for a List<>
> declaration. However, what I have is not working, so I am wondering if my
> basic way of thinking about this rule is wrong, and if I should do
> something else.
>
>
>
> *ApexSingleSoqlResultRule*
> Example:
>
> Account acct = [Select Id From Account Where Name = ‘abc’ ];
>
> // Get QueryException back if 0 or duplicate
>
> // People try to avoid by doing:
>
> List<Account> acct = [Select … LIMIT 1]; // bad practice to avoid dupes
> etc --> add violation
>
> if (acct.size() > 0 )
>
> [...]
>
>
> --------------------------------------------
>
> // Best practice is:
>
> try {
> Account acct = […];
> } catch (QueryException q){
>
> }
>
> For this rule, I am checking ASTSoqlExpression nodes. If their parent
> nodes include a try/catch, no violation. Else, add violation.
>
> I also check the ASTSoqlExpression for a LIMIT 1 since that's how people
> avoid the QueryException. But, I don't quite know how to do that.
>
> Maybe something like this?
>
> if(node.getNode().getQuery().toLowerCase().contains("list") &&
>                 node.getNode().getQuery().toLowerCase().contains("limit
> 1"))
>
>
> *ApexCommentRule*
>
> The last two rules are related to readability. The first one looks at the
> following:
>
>    - No class or method comments => add to violation count
>
>
> For this, can I just use a Java rule, like CommentRequiredRule?
>
> *ApexReadabilityRule*
>
> The last one follows these criteria:
>
>    - Lines of code in a method exceed 50 => add to violation count
>    - Nested if statements exceed more than 3  => add to violation count
>    - Non trigger framework code  exists in triggers  => add to violation
>    count
>
>
> I'm not sure how to do this last one.
>
> If you have any insight or feedback, let me know!
>
> Thank you.
>
> --
> Caleb Knox
>
> Endeveran
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Pmd-devel mailing list
> Pmd...@li...
> https://lists.sourceforge.net/lists/listinfo/pmd-devel
>
>