Re: [pmd-devel] Custom Apex rule questions
A source code analyzer
Brought to you by:
adangel,
juansotuyo
Menu
▾
▴
Re: [pmd-devel] Custom Apex rule questions
|
From: Caleb K. <cal...@en...> - 2017-07-14 15:15:24
|
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). > So, what I mean is that I'd like to check the ForEach to see if there is a Soql expression within its declaration, because that's a direct child node. Is that wrong? ApexReadabilityRule: > - 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. > Triggers in Salesforce should not have any logic. The trigger should call a handler. Thanks. On Fri, Jul 14, 2017 at 9:14 AM, Caleb Knox <cal...@en...> wrote: > Thanks for the insight. I'll get back to you about the ApexOptimizeSoql if > I don't get it right soon. > > I can definitely add these rules to PMD, assuming my boss okays it. > > On Thu, Jul 13, 2017 at 6:48 PM, Juan Martín Sotuyo Dodero < > jua...@gm...> wrote: > >> 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 >>> >>> >> > > > -- > Caleb Knox > > Endeveran > -- Caleb Knox Endeveran |
