hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

[apex] UnusedLocalVariable - support concat strings for binding vars

Browse Source 
Fixes #5000
This commit is contained in:
Andreas Dangel
2024-05-23 16:13:45 +02:00
parent b284ca6ec8
commit 4d07d80c93
4 changed files with 207 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/pages/release_notes.md
View File

@ -25,6 +25,8 @@ Since this release, PMD will also expose any getter returning a collection of an
```
### 🐛 Fixed Issues
* apex-bestpractices
* [#5000](https://github.com/pmd/pmd/issues/5000): \[apex] UnusedLocalVariable FP with binds in SOSL / SOQL
* core
* [#4467](https://github.com/pmd/pmd/issues/4467): \[core] Expose collections from getters as XPath sequence attributes
* [#4978](https://github.com/pmd/pmd/issues/4978): \[core] Referenced Rulesets do not emit details on validation errors

7
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTSoslExpression.java
View File

@ -17,4 +17,11 @@ public final class ASTSoslExpression extends AbstractApexNode.Single<SoslExpress
protected <P, R> R acceptApexVisitor(ApexVisitor<? super P, ? extends R> visitor, P data) {
return visitor.visit(this, data);
}
/**
* Returns the raw query as it appears in the source code.
*/
public String getQuery() {
return node.getQuery();
}
}

54
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java
View File

@ -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<ApexNode<?>> 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<String> soqlBindingVariables = findBindingsInSOQLStringLiterals(variableContext);
if (soqlBindingVariables.contains(variableName.toLowerCase(Locale.ROOT))) {
List<String> 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<String> findBindingsInSOQLStringLiterals(ASTBlockStatement variableContext) {
List<String> 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<String> findBindingsInSOSLQueries(ASTBlockStatement variableContext) {
return variableContext.descendants(ASTSoslExpression.class)
.toStream()
.map(ASTSoslExpression::getQuery)
.flatMap(UnusedLocalVariableRule::extractBindindVars)
.collect(Collectors.toList());
}
private List<String> findBindingsInSOQLStringLiterals(ASTBlockStatement variableContext) {
List<ASTMethodCallExpression> methodCalls = variableContext.descendants(ASTMethodCallExpression.class)
.filter(m -> DATABASE_QUERY_METHODS.contains(m.getFullMethodName().toLowerCase(Locale.ROOT)))
.collect(Collectors.toList());
methodCalls.forEach(databaseMethodCall -> {
List<String> stringLiterals = new ArrayList<>();
List<String> 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<String> extractBindindVars(String query) {
List<String> vars = new ArrayList<>();
Matcher matcher = BINDING_VARIABLE.matcher(query);
while (matcher.find()) {
vars.add(matcher.group(1).toLowerCase(Locale.ROOT));
}
return vars.stream();
}
}

161
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml
View File

@ -240,6 +240,167 @@ public class Foo {
Database.query(query3d, System.AccessLevel.USER_MODE);
}
}
]]></code>
</test-code>
<test-code>
<description>[apex] UnusedLocalVariable FP with binds in SOSL / SOQL #5000</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void example() {
SObject record = this.getOldRecords()[0];
List<SObject> 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));
}
}
}
]]></code>
</test-code>
<test-code>
<description>Using Apex Variables in SOQL and SOSL Queries (#5000)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
// examples taken from https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/langCon_apex_SOQL_variables.htm
public class Foo {
public void simpleBind() {
Account A = new Account(Name='xxx');
//insert A; // usage
// A simple bind
Account B = [SELECT Id FROM Account WHERE Id = :A.Id]; // usage of A
if (B != null) {} // usage of B
}
public void bindWithArithmetic() {
Account A = new Account(Name='xxx');
insert A; // usage of A
// A bind with arithmetic
Account B = [SELECT Id FROM Account
WHERE Name = :('x' + 'xx')]; // that's actually not a variable ref, but an expression
if (B != null) {} // usage of B
}
public void bindWithExpressions() {
Account A = new Account(Name='xxx');
insert A; // usage of A
// A bind with expressions
Account B = [SELECT Id FROM Account
WHERE Name = :'XXXX'.substring(0,3)];
if (B != null) {} // usage of B
}
/* not supported by apex-parser yet
public void bindWithIncludes() {
Account A = new Account(Name='xxx');
//insert A; // usage of A
// A bind with INCLUDES clause
Account B = [SELECT Id FROM Account WHERE :A.TYPE INCLUDES ('Customer - Direct; Customer - Channel')]; // usage of A
if (B != null) {} // usage of B
}
*/
public void bindWithQueryResult() {
Account A = new Account(Name='xxx');
//insert A; // usage of A
// A bind with an expression that is itself a query result
Account B = [SELECT Id FROM Account
WHERE Name = :[SELECT Name FROM Account
WHERE Id = :A.Id].Name]; // usage of A
if (B != null) {} // usage of B
}
public void bindInParentAndAggregateQuery() {
Account A = new Account(Name='xxx');
//insert A; // usage of A
Contact C = new Contact(LastName='xxx', AccountId=A.Id);
//insert new Contact[]{C, new Contact(LastName='yyy', // usage of C
// accountId=A.id)}; // usage of A
// Binds in both the parent and aggregate queries
Account B = [SELECT Id, (SELECT Id FROM Contacts
WHERE Id = :C.Id) // usage of C
FROM Account
WHERE Id = :A.Id]; // usage of A
// One contact returned
Contact D = B.Contacts; // usage of B
update D; // usage of D
}
public void limitBind() {
// A limit bind
Integer i = 1;
Account B = [SELECT Id FROM Account LIMIT :i]; // usage of i
if (B != null) {} // usage of B
}
public void offsetBind() {
// An OFFSET bind
Integer offsetVal = 10;
List<Account> 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<List<SObject>> 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
}
}
]]></code>
</test-code>
</test-data>