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

[apex] UnusedLocalVariable - support concat strings for binding vars (#5037)

Browse Source 
Merge pull request #5037 from adangel:issue-5000-apex-unused-local-variable
This commit is contained in:
Andreas Dangel
2024-06-27 11:51:57 +02:00
parent 1680e94eea 6517dff840
commit cd41d70546
5 changed files with 209 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
.all-contributorsrc
View File

@ -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,

1
docs/pages/pmd/projectdocs/credits.md
View File

@ -982,6 +982,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
<td align="center" valign="top" width="14.28%"><a href="https://github.com/oggboy"><img src="https://avatars.githubusercontent.com/u/4798818?v=4?s=100" width="100px;" alt="oggboy"/><br /><sub><b>oggboy</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Aoggboy" title="Bug reports">🐛</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://journal.lampetty.net/archive/category/in%20English"><img src="https://avatars.githubusercontent.com/u/78990?v=4?s=100" width="100px;" alt="oinume"/><br /><sub><b>oinume</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Aoinume" title="Bug reports">🐛</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/orimarko"><img src="https://avatars.githubusercontent.com/u/17137249?v=4?s=100" width="100px;" alt="orimarko"/><br /><sub><b>orimarko</b></sub></a><br /><a href="https://github.com/pmd/pmd/commits?author=orimarko" title="Code">💻</a> <a href="https://github.com/pmd/pmd/issues?q=author%3Aorimarko" title="Bug reports">🐛</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/pablogomez2197"><img src="https://avatars.githubusercontent.com/u/110610165?v=4?s=100" width="100px;" alt="pablogomez2197"/><br /><sub><b>pablogomez2197</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Apablogomez2197" title="Bug reports">🐛</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/pacvz"><img src="https://avatars.githubusercontent.com/u/35453365?v=4?s=100" width="100px;" alt="pacvz"/><br /><sub><b>pacvz</b></sub></a><br /><a href="https://github.com/pmd/pmd/commits?author=pacvz" title="Code">💻</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/pagarwal-ignitetech"><img src="https://avatars.githubusercontent.com/u/30888430?v=4?s=100" width="100px;" alt="pallavi agarwal"/><br /><sub><b>pallavi agarwal</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Apagarwal-ignitetech" title="Bug reports">🐛</a></td>
<td align="center" valign="top" width="14.28%"><a href="https://github.com/parksungrin"><img src="https://avatars.githubusercontent.com/u/29750262?v=4?s=100" width="100px;" alt="parksungrin"/><br /><sub><b>parksungrin</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Aparksungrin" title="Bug reports">🐛</a></td>

2
docs/pages/release_notes.md
View File

@ -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

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 - see https://github.com/apex-dev-tools/apex-parser/issues/44
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>