From 3ba3eb4245b617c2b024c0e8faaae54106828902 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 12 Jul 2024 09:36:09 +0200 Subject: [PATCH] [apex] AvoidNonRestrictiveQueriesRule - support SOSL --- .../AvoidNonRestrictiveQueriesRule.java | 39 ++++++++++++------- .../resources/category/apex/performance.xml | 8 +++- .../xml/AvoidNonRestrictiveQueries.xml | 39 +++++++++++++++++++ 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidNonRestrictiveQueriesRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidNonRestrictiveQueriesRule.java index aa51602732..5603d8532b 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidNonRestrictiveQueriesRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidNonRestrictiveQueriesRule.java @@ -15,7 +15,9 @@ import net.sourceforge.pmd.lang.apex.ast.ASTAnnotationParameter; import net.sourceforge.pmd.lang.apex.ast.ASTMethod; import net.sourceforge.pmd.lang.apex.ast.ASTModifierNode; import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; import net.sourceforge.pmd.lang.ast.NodeStream; import net.sourceforge.pmd.lang.rule.RuleTargetSelector; @@ -23,18 +25,27 @@ import net.sourceforge.pmd.reporting.RuleContext; public class AvoidNonRestrictiveQueriesRule extends AbstractApexRule { private static final Pattern RESTRICTIVE_PATTERN = Pattern.compile("(where )|(limit )", Pattern.CASE_INSENSITIVE); - private static final Pattern SELECT_PATTERN = Pattern.compile("(select )", Pattern.CASE_INSENSITIVE); + private static final Pattern SELECT_OR_FIND_PATTERN = Pattern.compile("(select|find )", Pattern.CASE_INSENSITIVE); private static final Pattern SUB_QUERY_PATTERN = Pattern.compile("(?i)\\(\\s*select\\s+[^)]+\\)"); @Override protected @NonNull RuleTargetSelector buildTargetSelector() { - return RuleTargetSelector.forTypes(ASTSoqlExpression.class); + return RuleTargetSelector.forTypes(ASTSoqlExpression.class, ASTSoslExpression.class); } @Override public Object visit(ASTSoqlExpression node, Object data) { - String query = node.getQuery(); + visitSoqlOrSosl(node, "SOQL", node.getQuery(), asCtx(data)); + return data; + } + @Override + public Object visit(ASTSoslExpression node, Object data) { + visitSoqlOrSosl(node, "SOSL", node.getQuery(), asCtx(data)); + return data; + } + + private void visitSoqlOrSosl(ApexNode node, String type, String query, RuleContext ruleContext) { ASTMethod method = node.ancestors(ASTMethod.class).first(); if (method != null && method.getModifiers().isTest()) { Optional methodAnnotation = method @@ -55,17 +66,17 @@ public class AvoidNonRestrictiveQueriesRule extends AbstractApexRule { .firstOpt() .map(ASTAnnotationParameter::getBooleanValue)); boolean classSeeAllData = classAnnotation.flatMap(m -> m.children(ASTAnnotationParameter.class) - .filter(p -> ASTAnnotationParameter.SEE_ALL_DATA.equalsIgnoreCase(p.getName())) - .firstOpt() - .map(ASTAnnotationParameter::getBooleanValue)) + .filter(p -> ASTAnnotationParameter.SEE_ALL_DATA.equalsIgnoreCase(p.getName())) + .firstOpt() + .map(ASTAnnotationParameter::getBooleanValue)) .orElse(false); if (methodSeeAllData.isPresent()) { if (!methodSeeAllData.get()) { - return null; + return; } } else if (!classSeeAllData) { - return null; + return; } } @@ -76,17 +87,15 @@ public class AvoidNonRestrictiveQueriesRule extends AbstractApexRule { } subQueryMatcher.appendTail(queryWithoutSubQueries); - verifyQuery(asCtx(data), node, queryWithoutSubQueries.toString()); - - return data; + verifyQuery(ruleContext, node, type, queryWithoutSubQueries.toString()); } - private void verifyQuery(RuleContext ctx, ASTSoqlExpression node, String query) { - int occurrencesSelect = countOccurrences(SELECT_PATTERN, query); + private void verifyQuery(RuleContext ctx, ApexNode node, String type, String query) { + int occurrencesSelectOrFind = countOccurrences(SELECT_OR_FIND_PATTERN, query); int occurrencesWhereOrLimit = countOccurrences(RESTRICTIVE_PATTERN, query); - if (occurrencesSelect > 0 && occurrencesWhereOrLimit == 0) { - ctx.addViolation(node); + if (occurrencesSelectOrFind > 0 && occurrencesWhereOrLimit == 0) { + ctx.addViolation(node, type); } } diff --git a/pmd-apex/src/main/resources/category/apex/performance.xml b/pmd-apex/src/main/resources/category/apex/performance.xml index 41afacdbed..7aad71de6e 100644 --- a/pmd-apex/src/main/resources/category/apex/performance.xml +++ b/pmd-apex/src/main/resources/category/apex/performance.xml @@ -56,11 +56,13 @@ public class Foo { - When working with very large amounts of data, unfiltered SOQL queries can quickly cause governor limit exceptions. + When working with very large amounts of data, unfiltered SOQL or SOSL queries can quickly cause + [governor limit](https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_gov_limits.htm) + exceptions. 3 @@ -69,6 +71,8 @@ public class Something { public static void main( String[] as ) { Account[] accs1 = [ select id from account ]; // Bad Account[] accs2 = [ select id from account limit 10 ]; // better + + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; // bad } } ]]> diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidNonRestrictiveQueries.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidNonRestrictiveQueries.xml index 650e0b8ff9..5856641b9f 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidNonRestrictiveQueries.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/AvoidNonRestrictiveQueries.xml @@ -198,6 +198,45 @@ public class TestDataAccessClass { Account[] accounts = [SELECT Id, Name FROM Account]; // not good, inherits SeeAllData=true from class } } +]]> + + + + Test case with SOSL query - missing where + 1 + + Avoid SOSL queries without a where or limit statement + + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; // bad + } +} +]]> + + + + Test case with SOSL query - with limit is OK + 0 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead LIMIT 1]; + } +} +]]> + + + + Test case with SOSL query - with where is OK + 0 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name WHERE Name like 'test'), Contact, Opportunity, Lead]; + } +} ]]>