diff --git a/.all-contributorsrc b/.all-contributorsrc index 2fdad95a30..898c8d6350 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -7590,6 +7590,14 @@ "contributions": [ "bug" ] + }, + "login": "pablogomez2197", + "name": "pablogomez2197", + "avatar_url": "https://avatars.githubusercontent.com/u/110610165?v=4", + "profile": "https://github.com/pablogomez2197", + "contributions": [ + "bug" + ] } ], "contributorsPerLine": 7, diff --git a/docs/pages/pmd/projectdocs/credits.md b/docs/pages/pmd/projectdocs/credits.md index 92b9bac09c..0f97c47bec 100644 --- a/docs/pages/pmd/projectdocs/credits.md +++ b/docs/pages/pmd/projectdocs/credits.md @@ -982,6 +982,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d oggboy
oggboy
🐛 oinume
oinume
🐛 orimarko
orimarko
💻 🐛 + pablogomez2197
pablogomez2197
🐛 pacvz
pacvz
💻 pallavi agarwal
pallavi agarwal
🐛 parksungrin
parksungrin
🐛 diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8eb3f60fb3..bbe210bcce 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -21,6 +21,8 @@ This is a {{ site.pmd.release_type }} release. * [#4922](https://github.com/pmd/pmd/issues/4922): \[apex] SOQL syntax error with TYPEOF in sub-query * [#5053](https://github.com/pmd/pmd/issues/5053): \[apex] CPD fails to parse string literals with escaped characters * [#5055](https://github.com/pmd/pmd/issues/5055): \[apex] SOSL syntax error with WITH USER_MODE or WITH SYSTEM_MODE +* apex-bestpractices + * [#5000](https://github.com/pmd/pmd/issues/5000): \[apex] UnusedLocalVariable FP with binds in SOSL / SOQL * java-bestpractices * [#5047](https://github.com/pmd/pmd/issues/5047): \[java] UnusedPrivateMethod FP for Generics & Overloads * plsql diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java index f25090ee86..f22a14eeb4 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.apex.rule.bestpractices; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -13,6 +14,7 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.checkerframework.checker.nullness.qual.NonNull; @@ -21,6 +23,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression; import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration; import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression; import net.sourceforge.pmd.lang.apex.ast.ApexNode; @@ -53,9 +56,9 @@ public class UnusedLocalVariableRule extends AbstractApexRule { List> potentialUsages = new ArrayList<>(); - // Variable expression catch things like the `a` in `a + b` + // Variable expression catch things like the `a` in `a + b` or in `:a` (BindingExpression) potentialUsages.addAll(variableContext.descendants(ASTVariableExpression.class).toList()); - // Reference expressions catch things like the `a` in `a.foo()` + // Reference expressions catch things like the `a` in `a.foo()` or in `:a.Id` (Binding Expression) potentialUsages.addAll(variableContext.descendants(ASTReferenceExpression.class).toList()); for (ApexNode usage : potentialUsages) { @@ -68,8 +71,9 @@ public class UnusedLocalVariableRule extends AbstractApexRule { } } - List soqlBindingVariables = findBindingsInSOQLStringLiterals(variableContext); - if (soqlBindingVariables.contains(variableName.toLowerCase(Locale.ROOT))) { + List bindingVariables = new ArrayList<>(findBindingsInSOQLStringLiterals(variableContext)); + bindingVariables.addAll(findBindingsInSOSLQueries(variableContext)); + if (bindingVariables.contains(variableName.toLowerCase(Locale.ROOT))) { return data; } @@ -77,15 +81,27 @@ public class UnusedLocalVariableRule extends AbstractApexRule { return data; } - private List findBindingsInSOQLStringLiterals(ASTBlockStatement variableContext) { - List bindingVariables = new ArrayList<>(); + /** + * Manually parses the sosl query, as not all binding vars are in the AST yet + * (as {@link net.sourceforge.pmd.lang.apex.ast.ASTBindExpressions}). + * This is not needed anymore, once summit ast is fixed. + */ + private Collection findBindingsInSOSLQueries(ASTBlockStatement variableContext) { + return variableContext.descendants(ASTSoslExpression.class) + .toStream() + .map(ASTSoslExpression::getQuery) + .flatMap(UnusedLocalVariableRule::extractBindindVars) + .collect(Collectors.toList()); + } + private List findBindingsInSOQLStringLiterals(ASTBlockStatement variableContext) { List methodCalls = variableContext.descendants(ASTMethodCallExpression.class) .filter(m -> DATABASE_QUERY_METHODS.contains(m.getFullMethodName().toLowerCase(Locale.ROOT))) .collect(Collectors.toList()); - methodCalls.forEach(databaseMethodCall -> { - List stringLiterals = new ArrayList<>(); + List stringLiterals = new ArrayList<>(); + + for (ASTMethodCallExpression databaseMethodCall : methodCalls) { stringLiterals.addAll(databaseMethodCall.descendants(ASTLiteralExpression.class) .filter(ASTLiteralExpression::isString) .toStream() @@ -100,22 +116,26 @@ public class UnusedLocalVariableRule extends AbstractApexRule { .filter(usage -> referencedVariable.equalsIgnoreCase(usage.getImage())) .forEach(usage -> { stringLiterals.addAll(usage.getParent() - .children(ASTLiteralExpression.class) + .descendants(ASTLiteralExpression.class) .filter(ASTLiteralExpression::isString) .toStream() .map(ASTLiteralExpression::getImage) .collect(Collectors.toList())); }); }); + } - stringLiterals.forEach(s -> { - Matcher matcher = BINDING_VARIABLE.matcher(s); - while (matcher.find()) { - bindingVariables.add(matcher.group(1).toLowerCase(Locale.ROOT)); - } - }); - }); + return stringLiterals.stream() + .flatMap(UnusedLocalVariableRule::extractBindindVars) + .collect(Collectors.toList()); + } - return bindingVariables; + private static Stream extractBindindVars(String query) { + List vars = new ArrayList<>(); + Matcher matcher = BINDING_VARIABLE.matcher(query); + while (matcher.find()) { + vars.add(matcher.group(1).toLowerCase(Locale.ROOT)); + } + return vars.stream(); } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml index 25a529036f..02ef459183 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml @@ -240,6 +240,167 @@ public class Foo { Database.query(query3d, System.AccessLevel.USER_MODE); } } +]]> + + + + [apex] UnusedLocalVariable FP with binds in SOSL / SOQL #5000 + 0 + objectsToQuery = this.getOldRecords(); // used in bind + + SObjectType sObjectType = record.getSObjectType(); + + String[] relatedObjectData = RELATED_RECORD_DATA_BY_OBJECT.get(sObjectType).split('::'); + + String relatedObjectName = relatedObjectData[0]; + String relatedFieldName = relatedObjectData[1]; + + String query = 'SELECT ' + String.escapeSingleQuotes(relatedFieldName) + + ' FROM ' + String.escapeSingleQuotes(relatedObjectName) + + ' WHERE ' + String.escapeSingleQuotes(relatedFieldName) + + ' IN :objectsToQuery'; // bind + + for (sObject relatedRecord : (Database.query(query))) { + this.withRelatedRecords.add((String)relatedRecord.get(relatedFieldName)); + } + } +} +]]> + + + + Using Apex Variables in SOQL and SOSL Queries (#5000) + 0 + offsetList = [SELECT Id FROM Account OFFSET :offsetVal]; // usage of offsetVal + if (offsetList != null) {} // usage of offsetList + } + + public void inBindIdList() { + // An IN-bind with an Id list. Note that a list of sObjects + // can also be used--the Ids of the objects are used for + // the bind + Contact[] cc = [SELECT Id FROM Contact LIMIT 2]; + Task[] tt = [SELECT Id FROM Task WHERE WhoId IN :cc]; // usage of cc + if (tt != null) {} // usage of tt + } + + public void inBindStringList() { + // An IN-bind with a String list + String[] ss = new String[]{'a', 'b'}; + Account[] aa = [SELECT Id FROM Account + WHERE AccountNumber IN :ss]; // usage of ss + if (aa != null) {} // usage of aa + } + + public void soslQuery() { + // A SOSL query with binds in all possible clauses + + String myString1 = 'aaa'; + String myString2 = 'bbb'; + Integer myInt3 = 11; + //String myString4 = 'ccc'; + Integer myInt5 = 22; + + List> searchList = [FIND :myString1 IN ALL FIELDS + RETURNING + Account (Id, Name WHERE Name LIKE :myString2 + LIMIT :myInt3), + Contact, + Opportunity, + Lead + // WITH DIVISION =:myString4 -- that's not supported by apex-parser yet + WITH DIVISION = 'ccc' + LIMIT :myInt5]; + if (searchList != null) {} // usage of searchList + } +} ]]>