[apex] AvoidNonRestrictiveQueriesRule - support SOSL

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<ASTAnnotation> 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);
         }
     }
 

pmd-apex/src/main/resources/category/apex/performance.xml

@@ -56,11 +56,13 @@ public class Foo {
     <rule name="AvoidNonRestrictiveQueries"
           language="apex"
           since="7.4.0"
-          message="Avoid SOQL queries without a where or limit statement"
+          message="Avoid {0} queries without a where or limit statement"
           class="net.sourceforge.pmd.lang.apex.rule.performance.AvoidNonRestrictiveQueriesRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_performance.html#avoidnonrestrictivequeries">
         <description>
-            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.
         </description>
         <priority>3</priority>
         <example>
@@ -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<List<SObject>> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; // bad
     }
 }
 ]]>

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
     }
 }
+]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Test case with SOSL query - missing where</description>
+        <expected-problems>1</expected-problems>
+        <expected-messages>
+            <message>Avoid SOSL queries without a where or limit statement</message>
+        </expected-messages>
+        <code><![CDATA[
+public class Something {
+    public static void main( String[] as ) {
+        List<List<SObject>> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; // bad
+    }
+}
+]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Test case with SOSL query - with limit is OK</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Something {
+    public static void main( String[] as ) {
+        List<List<SObject>> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead LIMIT 1];
+    }
+}
+]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Test case with SOSL query - with where is OK</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Something {
+    public static void main( String[] as ) {
+        List<List<SObject>> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name WHERE Name like 'test'), Contact, Opportunity, Lead];
+    }
+}
 ]]></code>
     </test-code>
 </test-data>