From 4d07d80c93a35add73e0538963bac0cd01d884ce Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 23 May 2024 16:13:45 +0200 Subject: [PATCH 1/3] [apex] UnusedLocalVariable - support concat strings for binding vars Fixes #5000 --- docs/pages/release_notes.md | 2 + .../pmd/lang/apex/ast/ASTSoslExpression.java | 7 + .../UnusedLocalVariableRule.java | 54 ++++-- .../bestpractices/xml/UnusedLocalVariable.xml | 161 ++++++++++++++++++ 4 files changed, 207 insertions(+), 17 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a753816b29..9ec4111d61 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -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 diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTSoslExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTSoslExpression.java index b1af4a56f4..fde63721b8 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTSoslExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTSoslExpression.java @@ -17,4 +17,11 @@ public final class ASTSoslExpression extends AbstractApexNode.Single R acceptApexVisitor(ApexVisitor 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(); + } } 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..7f68ab8b40 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 + } +} ]]> From 3f5dc971e00b9998b104885d6536892547356a9b Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 23 May 2024 16:18:40 +0200 Subject: [PATCH 2/3] Add @pablogomez2197 as a contributor --- .all-contributorsrc | 9 +++++++++ docs/pages/pmd/projectdocs/credits.md | 23 +++++++++++++---------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index cb5a734560..d56b857e70 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -7581,6 +7581,15 @@ "bug", "code" ] + }, + { + "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 bf7ccbd23f..643c911dc5 100644 --- a/docs/pages/pmd/projectdocs/credits.md +++ b/docs/pages/pmd/projectdocs/credits.md @@ -981,99 +981,102 @@ 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

๐Ÿ› - patpatpat123
patpatpat123

๐Ÿ› + patpatpat123
patpatpat123

๐Ÿ› patriksevallius
patriksevallius

๐Ÿ› pbrajesh1
pbrajesh1

๐Ÿ› phoenix384
phoenix384

๐Ÿ› piotrszymanski-sc
piotrszymanski-sc

๐Ÿ’ป plan3d
plan3d

๐Ÿ› poojasix
poojasix

๐Ÿ› - prabhushrikant
prabhushrikant

๐Ÿ› + prabhushrikant
prabhushrikant

๐Ÿ› pujitha8783
pujitha8783

๐Ÿ› r-r-a-j
r-r-a-j

๐Ÿ› raghujayjunk
raghujayjunk

๐Ÿ› rajeshveera
rajeshveera

๐Ÿ› rajeswarreddy88
rajeswarreddy88

๐Ÿ› recdevs
recdevs

๐Ÿ› - reudismam
reudismam

๐Ÿ’ป ๐Ÿ› + reudismam
reudismam

๐Ÿ’ป ๐Ÿ› rijkt
rijkt

๐Ÿ› rillig-tk
rillig-tk

๐Ÿ› rmohan20
rmohan20

๐Ÿ’ป ๐Ÿ› rnveach
rnveach

๐Ÿ› rxmicro
rxmicro

๐Ÿ› ryan-gustafson
ryan-gustafson

๐Ÿ’ป ๐Ÿ› - sabi0
sabi0

๐Ÿ› + sabi0
sabi0

๐Ÿ› scais
scais

๐Ÿ› screamingfrog
screamingfrog

๐Ÿ’ต sebbASF
sebbASF

๐Ÿ› sergeygorbaty
sergeygorbaty

๐Ÿ’ป shilko2013
shilko2013

๐Ÿ› shiomiyan
shiomiyan

๐Ÿ“– - simeonKondr
simeonKondr

๐Ÿ› + simeonKondr
simeonKondr

๐Ÿ› snajberk
snajberk

๐Ÿ› sniperrifle2004
sniperrifle2004

๐Ÿ› snuyanzin
snuyanzin

๐Ÿ› ๐Ÿ’ป soyodream
soyodream

๐Ÿ› sratz
sratz

๐Ÿ› stonio
stonio

๐Ÿ› - sturton
sturton

๐Ÿ’ป ๐Ÿ› + sturton
sturton

๐Ÿ’ป ๐Ÿ› sudharmohan
sudharmohan

๐Ÿ› suruchidawar
suruchidawar

๐Ÿ› svenfinitiv
svenfinitiv

๐Ÿ› tashiscool
tashiscool

๐Ÿ› test-git-hook
test-git-hook

๐Ÿ› testation21
testation21

๐Ÿ’ป ๐Ÿ› - thanosa
thanosa

๐Ÿ› + thanosa
thanosa

๐Ÿ› tiandiyixian
tiandiyixian

๐Ÿ› tobwoerk
tobwoerk

๐Ÿ› tprouvot
tprouvot

๐Ÿ› ๐Ÿ’ป trentchilders
trentchilders

๐Ÿ› triandicAnt
triandicAnt

๐Ÿ› trishul14
trishul14

๐Ÿ› - tsui
tsui

๐Ÿ› + tsui
tsui

๐Ÿ› wangzitom12306
wangzitom12306

๐Ÿ› winhkey
winhkey

๐Ÿ› witherspore
witherspore

๐Ÿ› wjljack
wjljack

๐Ÿ› wuchiuwong
wuchiuwong

๐Ÿ› xingsong
xingsong

๐Ÿ› - xioayuge
xioayuge

๐Ÿ› + xioayuge
xioayuge

๐Ÿ› xnYi9wRezm
xnYi9wRezm

๐Ÿ’ป ๐Ÿ› xuanuy
xuanuy

๐Ÿ› xyf0921
xyf0921

๐Ÿ› yalechen-cyw3
yalechen-cyw3

๐Ÿ› yasuharu-sato
yasuharu-sato

๐Ÿ› zenglian
zenglian

๐Ÿ› - zgrzyt93
zgrzyt93

๐Ÿ’ป ๐Ÿ› + zgrzyt93
zgrzyt93

๐Ÿ’ป ๐Ÿ› zh3ng
zh3ng

๐Ÿ› zt_soft
zt_soft

๐Ÿ› ztt79
ztt79

๐Ÿ› zzzzfeng
zzzzfeng

๐Ÿ› รrpรกd Magosรกnyi
รrpรกd Magosรกnyi

๐Ÿ› ไปป่ดตๆฐ
ไปป่ดตๆฐ

๐Ÿ› + + ่Œ…ๅปถๅฎ‰
่Œ…ๅปถๅฎ‰

๐Ÿ’ป From 6517dff840570da92ecfbebc0c7c3ffff963c407 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 27 Jun 2024 11:48:26 +0200 Subject: [PATCH 3/3] [apex] Mention apex-parser issue refs apex-dev-tools/apex-parser#44 --- .../lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7f68ab8b40..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 @@ -307,7 +307,7 @@ public class Foo { if (B != null) {} // usage of B } -/* not supported by apex-parser yet +/* 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