diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d336edf2d0..c38a847dfa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,6 +6,7 @@ jobs: build: runs-on: ${{ matrix.os }} continue-on-error: ${{ matrix.experimental }} + if: "!contains(github.event.head_commit.message, '[skip ci]')" strategy: matrix: os: [ ubuntu-latest , windows-latest , macos-latest ] diff --git a/docs/_config.yml b/docs/_config.yml index af4b5c2faf..252ef3bd60 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -2,7 +2,7 @@ repository: pmd/pmd pmd: version: 7.0.0-SNAPSHOT - previous_version: 6.28.0 + previous_version: 6.29.0 date: ??-?????-2020 release_type: major diff --git a/docs/pages/next_major_development.md b/docs/pages/next_major_development.md index f369be2901..31dd1316bd 100644 --- a/docs/pages/next_major_development.md +++ b/docs/pages/next_major_development.md @@ -246,6 +246,10 @@ the breaking API changes will be performed in 7.0.0. an API is tagged as `@Deprecated` or not in the latest minor release. During the development of 7.0.0, we may decide to remove some APIs that were not tagged as deprecated, though we'll try to avoid it." %} +#### 6.29.0 + +No changes. + #### 6.28.0 ##### Deprecated API diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 727f0919d1..8abb75f161 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -19,27 +19,11 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy -#### Renamed Rules - -* The Java rule [`DoNotCallSystemExit`](https://pmd.github.io/latest/pmd_rules_java_errorprone.html#donotcallsystemexit) has been renamed to - {% rule "java/errorprone/DoNotTerminateVM" %}, since it checks for all the following calls: - `System.exit(int)`, `Runtime.exit(int)`, `Runtime.halt(int)`. All these calls terminate - the Java VM, which is bad, if the VM runs an application server which many independent applications. - ### Fixed Issues -* java-errorprone - * [#2157](https://github.com/pmd/pmd/issues/2157): \[java] Improve DoNotCallSystemExit: permit call in main(), flag System.halt - ### API Changes ### External Contributions -* [#2803](https://github.com/pmd/pmd/pull/2803): \[java] Improve DoNotCallSystemExit (Fixes #2157) - [Vitaly Polonetsky](https://github.com/mvitaly) -* [#2809](https://github.com/pmd/pmd/pull/2809): \[java] Move test config from file to test class - [Stefan Birkner](https://github.com/stefanbirkner) -* [#2810](https://github.com/pmd/pmd/pull/2810): \[core] Move method "renderTempFile" to XMLRendererTest - [Stefan Birkner](https://github.com/stefanbirkner) -* [#2813](https://github.com/pmd/pmd/pull/2813): \[core] Use JUnit's TemporaryFolder rule - [Stefan Birkner](https://github.com/stefanbirkner) -* [#2829](https://github.com/pmd/pmd/pull/2829): \[doc] Small correction in pmd\_report\_formats.md - [Gustavo Krieger](https://github.com/gustavopcassol) - {% endtocmaker %} diff --git a/docs/pages/release_notes_old.md b/docs/pages/release_notes_old.md index 073c5354e4..2e90ca3079 100644 --- a/docs/pages/release_notes_old.md +++ b/docs/pages/release_notes_old.md @@ -5,6 +5,88 @@ permalink: pmd_release_notes_old.html Previous versions of PMD can be downloaded here: https://github.com/pmd/pmd/releases +## 24-October-2020 - 6.29.0 + +The PMD team is pleased to announce PMD 6.29.0. + +This is a minor release. + +### Table Of Contents + +* [New and noteworthy](#new-and-noteworthy) + * [Updated Apex Support](#updated-apex-support) + * [New Rules](#new-rules) + * [Renamed Rules](#renamed-rules) + * [Deprecated Rules](#deprecated-rules) +* [Fixed Issues](#fixed-issues) +* [External Contributions](#external-contributions) +* [Stats](#stats) + +### New and noteworthy + +#### Updated Apex Support + +* The Apex language support has been bumped to version 50 (Winter '21). All new language features are now properly + parsed and processed. Especially the [Safe Navigation Operator](https://releasenotes.docs.salesforce.com/en-us/winter21/release-notes/rn_apex_SafeNavigationOperator.htm) is now supported. + See also [Salesforce Winter '21 Release Notes](https://releasenotes.docs.salesforce.com/en-us/winter21/release-notes/rn_apex.htm) + +#### New Rules + +* The new Apex rule [`OperationWithLimitsInLoop`](https://pmd.github.io/pmd-6.29.0/pmd_rules_apex_performance.html#operationwithlimitsinloop) (`apex-performance`) + finds operations in loops that may hit governor limits such as DML operations, SOQL + queries and more. The rule replaces the three rules "AvoidDmlStatementsInLoops", "AvoidSoqlInLoops", + and "AvoidSoslInLoops". + +#### Renamed Rules + +* The Java rule [`DoNotCallSystemExit`](https://pmd.github.io/pmd-6.29.0/pmd_rules_java_errorprone.html#donotcallsystemexit) has been renamed to + [`DoNotTerminateVM`](https://pmd.github.io/pmd-6.29.0/pmd_rules_java_errorprone.html#donotterminatevm), since it checks for all the following calls: + `System.exit(int)`, `Runtime.exit(int)`, `Runtime.halt(int)`. All these calls terminate + the Java VM, which is bad, if the VM runs an application server which many independent applications. + +#### Deprecated Rules + +* The Apex rules [`AvoidDmlStatementsInLoops`](https://pmd.github.io/pmd-6.29.0/pmd_rules_apex_performance.html#avoiddmlstatementsinloops), + [`AvoidSoqlInLoops`](https://pmd.github.io/pmd-6.29.0/pmd_rules_apex_performance.html#avoidsoqlinloops) and [`AvoidSoslInLoops`](https://pmd.github.io/pmd-6.29.0/pmd_rules_apex_performance.html#avoidsoslinloops) + (`apex-performance`) are deprecated in favour of the new rule + [`OperationWithLimitsInLoop`](https://pmd.github.io/pmd-6.29.0/pmd_rules_apex_performance.html#operationwithlimitsinloop). The deprecated rules will be removed + with PMD 7.0.0. + +### Fixed Issues + +* apex + * [#2839](https://github.com/pmd/pmd/issues/2839): \[apex] Apex classes with safe navigation operator from Winter 21 (50.0) are skipped +* apex-performance + * [#1713](https://github.com/pmd/pmd/issues/1713): \[apex] Mark Database DML statements in For Loop +* core + * [#2831](https://github.com/pmd/pmd/pull/2831): \[core] Fix XMLRenderer newlines when running under IBM Java +* java-errorprone + * [#2157](https://github.com/pmd/pmd/issues/2157): \[java] Improve DoNotCallSystemExit: permit call in main(), flag System.halt + * [#2764](https://github.com/pmd/pmd/issues/2764): \[java] CloseResourceRule does not recognize multiple assignment done to resource +* miscellaneous + * [#2823](https://github.com/pmd/pmd/issues/2823): \[doc] Renamed/Moved rules are missing in documentation +* vf (Salesforce VisualForce) + * [#2765](https://github.com/pmd/pmd/issues/2765): \[vf] Attributes with dot cause a VfParseException + +### External Contributions + +* [#2803](https://github.com/pmd/pmd/pull/2803): \[java] Improve DoNotCallSystemExit (Fixes #2157) - [Vitaly Polonetsky](https://github.com/mvitaly) +* [#2809](https://github.com/pmd/pmd/pull/2809): \[java] Move test config from file to test class - [Stefan Birkner](https://github.com/stefanbirkner) +* [#2810](https://github.com/pmd/pmd/pull/2810): \[core] Move method "renderTempFile" to XMLRendererTest - [Stefan Birkner](https://github.com/stefanbirkner) +* [#2811](https://github.com/pmd/pmd/pull/2811): \[java] CloseResource - Fix #2764: False-negative when re-assigning variable - [Andi Pabst](https://github.com/andipabst) +* [#2813](https://github.com/pmd/pmd/pull/2813): \[core] Use JUnit's TemporaryFolder rule - [Stefan Birkner](https://github.com/stefanbirkner) +* [#2816](https://github.com/pmd/pmd/pull/2816): \[apex] Detect 'Database' method invocations inside loops - [Jeff Bartolotta](https://github.com/jbartolotta-sfdc) +* [#2829](https://github.com/pmd/pmd/pull/2829): \[doc] Small correction in pmd\_report\_formats.md - [Gustavo Krieger](https://github.com/gustavopcassol) +* [#2834](https://github.com/pmd/pmd/pull/2834): \[vf] Allow attributes with dot in Visualforce - [rmohan20](https://github.com/rmohan20) +* [#2842](https://github.com/pmd/pmd/pull/2842): \[core] Bump antlr4 from 4.7 to 4.7.2 - [Adrien Lecharpentier](https://github.com/alecharp) +* [#2865](https://github.com/pmd/pmd/pull/2865): \[java] (doc) Update ExcessiveImports example code for clarity - [Gustavo Krieger](https://github.com/gustavopcassol) +* [#2866](https://github.com/pmd/pmd/pull/2866): \[java] (doc) Fix example for CouplingBetweenObjects - [Gustavo Krieger](https://github.com/gustavopcassol) + +### Stats +* 50 commits +* 23 closed tickets & PRs +* Days since last release: 27 + ## 26-September-2020 - 6.28.0 The PMD team is pleased to announce PMD 6.28.0. diff --git a/pmd-apex-jorje/pom.xml b/pmd-apex-jorje/pom.xml index 248127643b..9c2da9d32c 100644 --- a/pmd-apex-jorje/pom.xml +++ b/pmd-apex-jorje/pom.xml @@ -13,7 +13,7 @@ - 2020-06-04-ba31c0 + 2020-09-10-5a5192 diff --git a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.jar b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.jar similarity index 82% rename from pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.jar rename to pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.jar index 745e7e90c7..4546f33cc9 100644 Binary files a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.jar and b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.jar differ diff --git a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.pom b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.pom similarity index 91% rename from pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.pom rename to pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.pom index a2e0b78dfc..3c74212901 100644 --- a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-06-04-ba31c0/apex-jorje-lsp-minimized-2020-06-04-ba31c0.pom +++ b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/2020-09-10-5a5192/apex-jorje-lsp-minimized-2020-09-10-5a5192.pom @@ -4,6 +4,6 @@ 4.0.0 apex apex-jorje-lsp-minimized - 2020-06-04-ba31c0 + 2020-09-10-5a5192 POM was created from install:install-file diff --git a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/maven-metadata-local.xml b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/maven-metadata-local.xml index b2e75aa47d..d3df37e6e4 100644 --- a/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/maven-metadata-local.xml +++ b/pmd-apex-jorje/repo/apex/apex-jorje-lsp-minimized/maven-metadata-local.xml @@ -3,10 +3,10 @@ apex apex-jorje-lsp-minimized - 2020-06-04-ba31c0 + 2020-09-10-5a5192 - 2020-06-04-ba31c0 + 2020-09-10-5a5192 - 20200724082242 + 20201022163714 diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTInvalidDependentCompilation.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTInvalidDependentCompilation.java new file mode 100644 index 0000000000..9e3f1b1e99 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTInvalidDependentCompilation.java @@ -0,0 +1,27 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.ast; + +import apex.jorje.semantic.ast.compilation.InvalidDependentCompilation; + +public final class ASTInvalidDependentCompilation extends AbstractApexNode { + + ASTInvalidDependentCompilation(InvalidDependentCompilation userClass) { + super(userClass); + } + + + @Override + protected R acceptApexVisitor(ApexVisitor visitor, P data) { + return visitor.visit(this, data); + } + + + @Override + public String getImage() { + String apexName = getDefiningType(); + return apexName.substring(apexName.lastIndexOf('.') + 1); + } +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java index 8632675205..147c1ca998 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java @@ -52,4 +52,8 @@ public final class ASTReferenceExpression extends AbstractApexNode extends AbstractNodeWithTextCoordinates, ApexNode> implements ApexNode { @@ -166,18 +167,28 @@ abstract class AbstractApexNode extends AbstractNodeWithTextC } } + private TypeInfo getDefiningTypeOrNull() { + try { + return node.getDefiningType(); + } catch (UnsupportedOperationException e) { + return null; + } + } + @Override public String getDefiningType() { - if (node.getDefiningType() != null) { - return node.getDefiningType().getApexName(); + TypeInfo definingType = getDefiningTypeOrNull(); + if (definingType != null) { + return definingType.getApexName(); } return null; } @Override public String getNamespace() { - if (node.getDefiningType() != null) { - return node.getDefiningType().getNamespace().toString(); + TypeInfo definingType = getDefiningTypeOrNull(); + if (definingType != null) { + return definingType.getNamespace().toString(); } return null; } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java index dc839dafe5..717c041aa4 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java @@ -24,6 +24,7 @@ import apex.jorje.parser.impl.ApexLexer; import apex.jorje.semantic.ast.AstNode; import apex.jorje.semantic.ast.compilation.AnonymousClass; import apex.jorje.semantic.ast.compilation.ConstructorPreamble; +import apex.jorje.semantic.ast.compilation.InvalidDependentCompilation; import apex.jorje.semantic.ast.compilation.UserClass; import apex.jorje.semantic.ast.compilation.UserClassMethods; import apex.jorje.semantic.ast.compilation.UserEnum; @@ -152,6 +153,8 @@ final class ApexTreeBuilder extends AstVisitor { register(DmlUpdateStatement.class, ASTDmlUpdateStatement.class); register(DmlUpsertStatement.class, ASTDmlUpsertStatement.class); register(DoLoopStatement.class, ASTDoLoopStatement.class); + register(ElseWhenBlock.class, ASTElseWhenBlock.class); + register(EmptyReferenceExpression.class, ASTEmptyReferenceExpression.class); register(Expression.class, ASTExpression.class); register(ExpressionStatement.class, ASTExpressionStatement.class); register(Field.class, ASTField.class); @@ -159,12 +162,15 @@ final class ApexTreeBuilder extends AstVisitor { register(FieldDeclarationStatements.class, ASTFieldDeclarationStatements.class); register(ForEachStatement.class, ASTForEachStatement.class); register(ForLoopStatement.class, ASTForLoopStatement.class); + register(IdentifierCase.class, ASTIdentifierCase.class); register(IfBlockStatement.class, ASTIfBlockStatement.class); register(IfElseBlockStatement.class, ASTIfElseBlockStatement.class); register(IllegalStoreExpression.class, ASTIllegalStoreExpression.class); register(InstanceOfExpression.class, ASTInstanceOfExpression.class); + register(InvalidDependentCompilation.class, ASTInvalidDependentCompilation.class); register(JavaMethodCallExpression.class, ASTJavaMethodCallExpression.class); register(JavaVariableExpression.class, ASTJavaVariableExpression.class); + register(LiteralCase.class, ASTLiteralCase.class); register(LiteralExpression.class, ASTLiteralExpression.class); register(MapEntryNode.class, ASTMapEntryNode.class); register(Method.class, ASTMethod.class); @@ -199,29 +205,25 @@ final class ApexTreeBuilder extends AstVisitor { register(StatementExecuted.class, ASTStatementExecuted.class); register(SuperMethodCallExpression.class, ASTSuperMethodCallExpression.class); register(SuperVariableExpression.class, ASTSuperVariableExpression.class); + register(SwitchStatement.class, ASTSwitchStatement.class); register(TernaryExpression.class, ASTTernaryExpression.class); register(ThisMethodCallExpression.class, ASTThisMethodCallExpression.class); register(ThisVariableExpression.class, ASTThisVariableExpression.class); register(ThrowStatement.class, ASTThrowStatement.class); register(TriggerVariableExpression.class, ASTTriggerVariableExpression.class); register(TryCatchFinallyBlockStatement.class, ASTTryCatchFinallyBlockStatement.class); + register(TypeWhenBlock.class, ASTTypeWhenBlock.class); register(UserClass.class, ASTUserClass.class); register(UserClassMethods.class, ASTUserClassMethods.class); register(UserExceptionMethods.class, ASTUserExceptionMethods.class); register(UserEnum.class, ASTUserEnum.class); register(UserInterface.class, ASTUserInterface.class); register(UserTrigger.class, ASTUserTrigger.class); + register(ValueWhenBlock.class, ASTValueWhenBlock.class); register(VariableDeclaration.class, ASTVariableDeclaration.class); register(VariableDeclarationStatements.class, ASTVariableDeclarationStatements.class); register(VariableExpression.class, ASTVariableExpression.class); register(WhileLoopStatement.class, ASTWhileLoopStatement.class); - register(SwitchStatement.class, ASTSwitchStatement.class); - register(ElseWhenBlock.class, ASTElseWhenBlock.class); - register(TypeWhenBlock.class, ASTTypeWhenBlock.class); - register(ValueWhenBlock.class, ASTValueWhenBlock.class); - register(LiteralCase.class, ASTLiteralCase.class); - register(IdentifierCase.class, ASTIdentifierCase.class); - register(EmptyReferenceExpression.class, ASTEmptyReferenceExpression.class); } private static void register(Class nodeType, Class> nodeAdapterType) { diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexVisitor.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexVisitor.java index 7b1fd0879d..2f1baca081 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexVisitor.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexVisitor.java @@ -168,6 +168,10 @@ public interface ApexVisitor extends AstVisitor { return visitApexNode(node, data); } + default R visit(ASTInvalidDependentCompilation node, P data) { + return visitApexNode(node, data); + } + default R visit(ASTJavaMethodCallExpression node, P data) { return visitApexNode(node, data); } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/TestAccessEvaluator.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/TestAccessEvaluator.java index 4ca7462bb0..50c4176d8b 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/TestAccessEvaluator.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/TestAccessEvaluator.java @@ -353,4 +353,9 @@ class TestAccessEvaluator implements AccessEvaluator { public boolean hasNamespaceGuardedAccess(Namespace namespace, String arg1) { return false; } + + @Override + public boolean isNamespaceGuardNamespace(Namespace arg0) { + return false; + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java index 1901420c19..4be3586849 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java @@ -38,6 +38,7 @@ import net.sourceforge.pmd.lang.apex.ast.ApexNode; @InternalApi public final class Helper { public static final String ANY_METHOD = "*"; + private static final String DATABASE_CLASS_NAME = "Database"; private Helper() { throw new AssertionError("Can't instantiate helper classes"); @@ -165,10 +166,10 @@ public final class Helper { public static boolean isSystemLevelClass(ASTUserClass node) { List interfaces = node.getInterfaceNames(); - return interfaces.stream().anyMatch(Helper::isWhitelisted); + return interfaces.stream().anyMatch(Helper::isAllowed); } - private static boolean isWhitelisted(String identifier) { + private static boolean isAllowed(String identifier) { switch (identifier.toLowerCase(Locale.ROOT)) { case "queueable": case "database.batchable": @@ -186,4 +187,10 @@ public final class Helper { return sb.toString(); } + /** + * @return true if {@code node} is an invocation of a {@code Database} method. + */ + public static boolean isAnyDatabaseMethodCall(ASTMethodCallExpression node) { + return isMethodName(node, DATABASE_CLASS_NAME, ANY_METHOD); + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AbstractAvoidNodeInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AbstractAvoidNodeInLoopsRule.java new file mode 100644 index 0000000000..a866a94778 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AbstractAvoidNodeInLoopsRule.java @@ -0,0 +1,62 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.performance; + +import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; +import net.sourceforge.pmd.lang.ast.Node; + +/** + * Base class for any rules that detect operations contained within a loop that could be more efficiently executed by + * refactoring the code into a batched execution. + */ +abstract class AbstractAvoidNodeInLoopsRule extends AbstractApexRule { + /** + * Adds a violation if any parent of {@code node} is a looping construct that would cause {@code node} to execute + * multiple times and {@code node} is not part of a return statement that short circuits the loop. + */ + protected Object checkForViolation(ApexNode node, Object data) { + if (insideLoop(node) && parentNotReturn(node)) { + addViolation(data, node); + } + return data; + } + + /** + * @return false if {@code node} is a direct child of a return statement. Children of return statements should not + * result in a violation because the return short circuits the loop's execution. + */ + private boolean parentNotReturn(ApexNode node) { + return !(node.getParent() instanceof ASTReturnStatement); + } + + /** + * @return true if any parent of {@code node} is a construct that would cause {@code node} to execute multiple + * times. + */ + private boolean insideLoop(Node node) { + Node n = node.getParent(); + + while (n != null) { + if (n instanceof ASTBlockStatement && n.getParent() instanceof ASTForEachStatement) { + // only consider the block of the for-each statement, not the iterator + return true; + } + if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement + || n instanceof ASTForLoopStatement) { + return true; + } + n = n.getParent(); + } + + return false; + } +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java index c0478106ee..8dfb04fed7 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java @@ -10,74 +10,40 @@ import net.sourceforge.pmd.lang.apex.ast.ASTDmlMergeStatement; import net.sourceforge.pmd.lang.apex.ast.ASTDmlUndeleteStatement; import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpdateStatement; import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpsertStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement; -import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; -import net.sourceforge.pmd.lang.ast.Node; -public class AvoidDmlStatementsInLoopsRule extends AbstractApexRule { +/** + * @deprecated use {@link OperationWithLimitsInLoopRule} + */ +@Deprecated +public class AvoidDmlStatementsInLoopsRule extends AbstractAvoidNodeInLoopsRule { @Override public Object visit(ASTDmlDeleteStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; + return checkForViolation(node, data); } @Override public Object visit(ASTDmlInsertStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; + return checkForViolation(node, data); } @Override public Object visit(ASTDmlMergeStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; + return checkForViolation(node, data); } @Override public Object visit(ASTDmlUndeleteStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; + return checkForViolation(node, data); } @Override public Object visit(ASTDmlUpdateStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; + return checkForViolation(node, data); } @Override public Object visit(ASTDmlUpsertStatement node, Object data) { - if (insideLoop(node)) { - addViolation(data, node); - } - return data; - } - - private boolean insideLoop(Node node) { - Node n = node.getParent(); - - while (n != null) { - if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement - || n instanceof ASTForLoopStatement || n instanceof ASTForEachStatement) { - return true; - } - n = n.getParent(); - } - - return false; + return checkForViolation(node, data); } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java index df945f8153..ccc32a495b 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoqlInLoopsRule.java @@ -4,45 +4,16 @@ package net.sourceforge.pmd.lang.apex.rule.performance; -import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; -import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement; -import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; -import net.sourceforge.pmd.lang.ast.Node; -public class AvoidSoqlInLoopsRule extends AbstractApexRule { +/** + * @deprecated use {@link OperationWithLimitsInLoopRule} + */ +@Deprecated +public class AvoidSoqlInLoopsRule extends AbstractAvoidNodeInLoopsRule { @Override public Object visit(ASTSoqlExpression node, Object data) { - if (insideLoop(node) && parentNotReturn(node)) { - addViolation(data, node); - } - return data; - } - - private boolean parentNotReturn(ASTSoqlExpression node) { - return !(node.getParent() instanceof ASTReturnStatement); - } - - private boolean insideLoop(ASTSoqlExpression node) { - Node n = node.getParent(); - - while (n != null) { - if (n instanceof ASTBlockStatement && n.getParent() instanceof ASTForEachStatement) { - // only consider the block of the for-each statement, not the iterator - return true; - } - if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement - || n instanceof ASTForLoopStatement) { - return true; - } - n = n.getParent(); - } - - return false; + return checkForViolation(node, data); } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java index 5985fae0f3..d100b7787f 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidSoslInLoopsRule.java @@ -4,44 +4,16 @@ package net.sourceforge.pmd.lang.apex.rule.performance; -import net.sourceforge.pmd.lang.apex.ast.ASTDoLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement; -import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; -import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement; -import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; -import net.sourceforge.pmd.lang.ast.Node; -public class AvoidSoslInLoopsRule extends AbstractApexRule { +/** + * @deprecated use {@link OperationWithLimitsInLoopRule} + */ +@Deprecated +public class AvoidSoslInLoopsRule extends AbstractAvoidNodeInLoopsRule { @Override public Object visit(ASTSoslExpression node, Object data) { - if (insideLoop(node) && parentNotReturn(node) && parentNotForEach(node)) { - addViolation(data, node); - } - return data; - } - - private boolean parentNotReturn(ASTSoslExpression node) { - return !(node.getParent() instanceof ASTReturnStatement); - } - - private boolean parentNotForEach(ASTSoslExpression node) { - return !(node.getParent() instanceof ASTForEachStatement); - } - - private boolean insideLoop(ASTSoslExpression node) { - Node n = node.getParent(); - - while (n != null) { - if (n instanceof ASTDoLoopStatement || n instanceof ASTWhileLoopStatement - || n instanceof ASTForLoopStatement || n instanceof ASTForEachStatement) { - return true; - } - n = n.getParent(); - } - - return false; + return checkForViolation(node, data); } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopRule.java new file mode 100644 index 0000000000..5c52e7ea45 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopRule.java @@ -0,0 +1,101 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.performance; + +import org.checkerframework.checker.nullness.qual.NonNull; + +import net.sourceforge.pmd.lang.apex.ast.ASTDmlDeleteStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlInsertStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlMergeStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUndeleteStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpdateStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpsertStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; +import net.sourceforge.pmd.lang.apex.rule.internal.Helper; +import net.sourceforge.pmd.lang.rule.RuleTargetSelector; + +/** + * Warn users when code that could trigger governor limits is executing within a looping construct. + */ +public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule { + + @Override + protected @NonNull RuleTargetSelector buildTargetSelector() { + return RuleTargetSelector.forTypes( + // DML + ASTDmlDeleteStatement.class, + ASTDmlInsertStatement.class, + ASTDmlMergeStatement.class, + ASTDmlUndeleteStatement.class, + ASTDmlUpdateStatement.class, + ASTDmlUpsertStatement.class, + // Database methods + ASTMethodCallExpression.class, + // SOQL + ASTSoqlExpression.class, + // SOSL + ASTSoslExpression.class + ); + } + + // Begin DML Statements + @Override + public Object visit(ASTDmlDeleteStatement node, Object data) { + return checkForViolation(node, data); + } + + @Override + public Object visit(ASTDmlInsertStatement node, Object data) { + return checkForViolation(node, data); + } + + @Override + public Object visit(ASTDmlMergeStatement node, Object data) { + return checkForViolation(node, data); + } + + @Override + public Object visit(ASTDmlUndeleteStatement node, Object data) { + return checkForViolation(node, data); + } + + @Override + public Object visit(ASTDmlUpdateStatement node, Object data) { + return checkForViolation(node, data); + } + + @Override + public Object visit(ASTDmlUpsertStatement node, Object data) { + return checkForViolation(node, data); + } + // End DML Statements + + // Begin Database method invocations + @Override + public Object visit(ASTMethodCallExpression node, Object data) { + if (Helper.isAnyDatabaseMethodCall(node)) { + return checkForViolation(node, data); + } else { + return data; + } + } + // End Database method invocations + + // Begin SOQL method invocations + @Override + public Object visit(ASTSoqlExpression node, Object data) { + return checkForViolation(node, data); + } + // End SOQL method invocations + + // Begin SOSL method invocations + @Override + public Object visit(ASTSoslExpression node, Object data) { + return checkForViolation(node, data); + } + // End SOSL method invocations +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java index 55a0668cc6..e6baf2de71 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java @@ -103,7 +103,7 @@ public class ApexSharingViolationsRule extends AbstractApexRule { @Override public Object visit(ASTMethodCallExpression node, Object data) { - if (Helper.isMethodName(node, "Database", Helper.ANY_METHOD)) { + if (Helper.isAnyDatabaseMethodCall(node)) { checkForViolation(node, data); } diff --git a/pmd-apex/src/main/resources/category/apex/performance.xml b/pmd-apex/src/main/resources/category/apex/performance.xml index 1d9bd4c821..0ed0f78e73 100644 --- a/pmd-apex/src/main/resources/category/apex/performance.xml +++ b/pmd-apex/src/main/resources/category/apex/performance.xml @@ -12,11 +12,15 @@ Rules that flag suboptimal code. Avoid DML statements inside loops to avoid hitting the DML governor limit. Instead, try to batch up the data into a list and invoke your DML once on that list of data outside the loop. + +This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 @@ -37,11 +41,15 @@ public class Something { New objects created within loops should be checked to see if they can created outside them and reused. + +This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 @@ -60,11 +68,15 @@ public class Something { Sosl calls within loops can cause governor limit exceptions. + +This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 @@ -80,4 +92,46 @@ public class Something { + + + Database class methods, DML operations, SOQL queries, or SOSL queries within loops can cause governor limit exceptions. Instead, try to batch up the data into a list and invoke the operation once on that list of data outside the loop. + + 3 + + accounts) { + for (Account a : accounts) { + Database.insert(a); + } + } + + public void dmlInsideOfLoop() { + for (Integer i = 0; i < 151; i++) { + Account account; + // ... + insert account; + } + } + + public void soqlInsideOfLoop() { + for (Integer i = 0; i < 10; i++) { + List accounts = [SELECT Id FROM Account]; + } + } + + public void soslInsideOfLoop() { + for (Integer i = 0; i < 10; i++) { + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} +]]> + + diff --git a/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml b/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml index ecf513fddc..1b5e312091 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/quickstart.xml @@ -66,13 +66,7 @@ - - 3 - - - 3 - - + 3 diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml index 9f1302647e..78c1864f93 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml @@ -1,6 +1,10 @@ - - Default ruleset for Salesforce.com Apex + + +Default ruleset used by the Code Climate Engine for Salesforce.com Apex + +Note: This ruleset is deprecated. Use "rulesets/apex/quickstart.xml" instead. + @@ -12,9 +16,9 @@ - - - + + + diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/DefaultRulesetTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/DefaultRulesetTest.java index 2f5eeda3a4..11ef33e0d1 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/DefaultRulesetTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/DefaultRulesetTest.java @@ -17,13 +17,13 @@ import org.junit.contrib.java.lang.system.SystemErrRule; import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSetFactory; -import net.sourceforge.pmd.util.ResourceLoader; +import net.sourceforge.pmd.RulesetsFactoryUtils; public class DefaultRulesetTest { @Rule public final SystemErrRule systemErrRule = new SystemErrRule().enableLog().muteForSuccessfulTests(); - private RuleSetFactory factory = new RuleSetFactory(new ResourceLoader(), RulePriority.LOW, true, false); + private RuleSetFactory factory = RulesetsFactoryUtils.createFactory(RulePriority.LOW, true, false); @Test public void loadDefaultRuleset() throws Exception { diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java new file mode 100644 index 0000000000..dc7c66b6f1 --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java @@ -0,0 +1,29 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.apex.ast; + +import org.junit.Test; + +import net.sourceforge.pmd.lang.ast.test.BaseParsingHelper; +import net.sourceforge.pmd.lang.ast.test.BaseTreeDumpTest; +import net.sourceforge.pmd.lang.ast.test.RelevantAttributePrinter; + +public class ApexTreeDumpTest extends BaseTreeDumpTest { + + public ApexTreeDumpTest() { + super(new RelevantAttributePrinter(), ".cls"); + } + + @Override + public BaseParsingHelper getParser() { + return ApexParsingHelper.DEFAULT; + } + + @Test + public void safeNavigationOperator() throws Exception { + doTest("SafeNavigationOperator"); + } +} diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopTest.java new file mode 100644 index 0000000000..56a609bad8 --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopTest.java @@ -0,0 +1,11 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.performance; + +import net.sourceforge.pmd.testframework.PmdRuleTst; + +public class OperationWithLimitsInLoopTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.cls b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.cls new file mode 100644 index 0000000000..6dedbd277b --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.cls @@ -0,0 +1,25 @@ +// See https://github.com/pmd/pmd/issues/2839 +// and https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/langCon_apex_SafeNavigationOperator.htm + +public class Foo { + Integer x = anObject?.anIntegerField; // The expression is of type Integer because the field is of type Integer + + // New code using the safe navigation operator + String profileUrl = user.getProfileUrl()?.toExternalForm(); + + public void bar1(Object a) { + a?.b; // Evaluates to: a == null ? null : a.b + ((T)a1?.b1)?.c1(); + } + + public void bar2(Object[] a, int x) { + a[x]?.aMethod().aField; // Evaluates to null if a[x] == null + a[x].aMethod()?.aField; + } + + public String getName(int accId) { + String s = contact.Account?.BillingCity; + // New code using the safe navigation operator + return [SELECT Name FROM Account WHERE Id = :accId]?.Name; + } +} \ No newline at end of file diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt new file mode 100644 index 0000000000..749922bf6b --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt @@ -0,0 +1,99 @@ ++- ApexFile[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(4, 14, 180, 183)", @Namespace = "", @RealLoc = "true"] + +- UserClass[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "Foo", @Location = "(4, 14, 180, 183)", @Namespace = "", @RealLoc = "true", @SuperClassName = "", @TypeKind = "CLASS"] + +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(4, 14, 180, 183)", @Modifiers = "1", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "true", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- Field[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Name = "x", @Namespace = "", @RealLoc = "true", @Type = "Integer", @Value = null] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(5, 13, 198, 199)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- Field[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = "true", @Type = "String", @Value = null] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(8, 12, 365, 375)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- FieldDeclarationStatements[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(5, 5, 190, 199)", @Namespace = "", @RealLoc = "true", @TypeName = "Integer"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "no location", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "false", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- FieldDeclaration[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 13, 198, 199)", @Name = "anIntegerField", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 27, 212, 226)", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "anObject", @Location = "(5, 17, 202, 210)", @Namespace = "", @RealLoc = "true"] + | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Namespace = "", @RealLoc = "true"] + | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + +- FieldDeclarationStatements[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(8, 5, 358, 375)", @Namespace = "", @RealLoc = "true", @TypeName = "String"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "no location", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "false", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- FieldDeclaration[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = "true"] + | +- MethodCallExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @FullMethodName = "toExternalForm", @InputParametersSize = "0", @Location = "(8, 47, 400, 414)", @MethodName = "toExternalForm", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "METHOD", @SafeNav = "true"] + | | +- MethodCallExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @FullMethodName = "user.getProfileUrl", @InputParametersSize = "0", @Location = "(8, 30, 383, 396)", @MethodName = "getProfileUrl", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Image = "user", @Location = "(8, 25, 378, 382)", @Namespace = "", @RealLoc = "true", @ReferenceType = "METHOD", @SafeNav = "false"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Namespace = "", @RealLoc = "true"] + | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + +- Method[@ApexVersion = "51.0", @Arity = "1", @CanonicalName = "bar1", @Constructor = "false", @DefiningType = "Foo", @Image = "bar1", @Location = "(10, 17, 435, 439)", @Namespace = "", @RealLoc = "true", @ReturnType = "void", @Synthetic = "false"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(10, 17, 435, 439)", @Modifiers = "1", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "true", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- Parameter[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a", @Location = "(10, 29, 447, 448)", @Namespace = "", @RealLoc = "true", @Type = "Object"] + | | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(10, 29, 447, 448)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- BlockStatement[@ApexVersion = "51.0", @CurlyBrace = "true", @DefiningType = "Foo", @Location = "(10, 32, 450, 538)", @Namespace = "", @RealLoc = "true"] + | +- ExpressionStatement[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(11, 12, 463, 465)", @Namespace = "", @RealLoc = "true"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "b", @Location = "(11, 12, 463, 464)", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a", @Location = "(11, 9, 460, 461)", @Namespace = "", @RealLoc = "true"] + | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | +- ExpressionStatement[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(12, 22, 527, 532)", @Namespace = "", @RealLoc = "true"] + | +- MethodCallExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @FullMethodName = "c1", @InputParametersSize = "0", @Location = "(12, 22, 527, 529)", @MethodName = "c1", @Namespace = "", @RealLoc = "true"] + | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "METHOD", @SafeNav = "true"] + | +- CastExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(12, 10, 515, 518)", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "b1", @Location = "(12, 17, 522, 524)", @Namespace = "", @RealLoc = "true"] + | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a1", @Location = "(12, 13, 518, 520)", @Namespace = "", @RealLoc = "true"] + | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + +- Method[@ApexVersion = "51.0", @Arity = "2", @CanonicalName = "bar2", @Constructor = "false", @DefiningType = "Foo", @Image = "bar2", @Location = "(15, 17, 556, 560)", @Namespace = "", @RealLoc = "true", @ReturnType = "void", @Synthetic = "false"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(15, 17, 556, 560)", @Modifiers = "1", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "true", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- Parameter[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a", @Location = "(15, 31, 570, 571)", @Namespace = "", @RealLoc = "true", @Type = "List"] + | | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(15, 31, 570, 571)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- Parameter[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "x", @Location = "(15, 38, 577, 578)", @Namespace = "", @RealLoc = "true", @Type = "int"] + | | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(15, 38, 577, 578)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- BlockStatement[@ApexVersion = "51.0", @CurlyBrace = "true", @DefiningType = "Foo", @Location = "(15, 41, 580, 688)", @Namespace = "", @RealLoc = "true"] + | +- ExpressionStatement[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(16, 25, 606, 613)", @Namespace = "", @RealLoc = "true"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "aField", @Location = "(16, 25, 606, 612)", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "false"] + | | +- MethodCallExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @FullMethodName = "aMethod", @InputParametersSize = "0", @Location = "(16, 15, 596, 603)", @MethodName = "aMethod", @Namespace = "", @RealLoc = "true"] + | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "METHOD", @SafeNav = "true"] + | | +- ArrayLoadExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(16, 9, 590, 591)", @Namespace = "", @RealLoc = "true"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a", @Location = "(16, 9, 590, 591)", @Namespace = "", @RealLoc = "true"] + | | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "x", @Location = "(16, 11, 592, 593)", @Namespace = "", @RealLoc = "true"] + | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | +- ExpressionStatement[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(17, 25, 675, 682)", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "aField", @Location = "(17, 25, 675, 681)", @Namespace = "", @RealLoc = "true"] + | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | +- MethodCallExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @FullMethodName = "aMethod", @InputParametersSize = "0", @Location = "(17, 14, 664, 671)", @MethodName = "aMethod", @Namespace = "", @RealLoc = "true"] + | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "METHOD", @SafeNav = "false"] + | +- ArrayLoadExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(17, 9, 659, 660)", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "a", @Location = "(17, 9, 659, 660)", @Namespace = "", @RealLoc = "true"] + | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "x", @Location = "(17, 11, 661, 662)", @Namespace = "", @RealLoc = "true"] + | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + +- Method[@ApexVersion = "51.0", @Arity = "1", @CanonicalName = "getName", @Constructor = "false", @DefiningType = "Foo", @Image = "getName", @Location = "(20, 19, 708, 715)", @Namespace = "", @RealLoc = "true", @ReturnType = "String", @Synthetic = "false"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(20, 19, 708, 715)", @Modifiers = "1", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "true", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- Parameter[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "accId", @Location = "(20, 31, 720, 725)", @Namespace = "", @RealLoc = "true", @Type = "int"] + | | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(20, 31, 720, 725)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | +- BlockStatement[@ApexVersion = "51.0", @CurlyBrace = "true", @DefiningType = "Foo", @Location = "(20, 38, 727, 905)", @Namespace = "", @RealLoc = "true"] + | +- VariableDeclarationStatements[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(21, 9, 737, 745)", @Namespace = "", @RealLoc = "true"] + | | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "no location", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "false", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + | | +- VariableDeclaration[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "s", @Location = "(21, 16, 744, 745)", @Namespace = "", @RealLoc = "true", @Type = "String"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "BillingCity", @Location = "(21, 37, 765, 776)", @Namespace = "", @RealLoc = "true"] + | | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "Account", @Location = "(21, 28, 756, 763)", @Namespace = "", @RealLoc = "true"] + | | | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Image = "contact", @Location = "(21, 20, 748, 755)", @Namespace = "", @RealLoc = "true", @ReferenceType = "LOAD", @SafeNav = "false"] + | | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "s", @Location = "(21, 16, 744, 745)", @Namespace = "", @RealLoc = "true"] + | | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + | +- ReturnStatement[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(23, 9, 841, 899)", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "Name", @Location = "(23, 62, 894, 898)", @Namespace = "", @RealLoc = "true"] + | +- ReferenceExpression[@ApexVersion = "51.0", @Context = null, @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReferenceType = "LOAD", @SafeNav = "true"] + | +- SoqlExpression[@ApexVersion = "51.0", @CanonicalQuery = "SELECT Name FROM Account WHERE Id = :tmpVar1", @DefiningType = "Foo", @Location = "(23, 16, 848, 892)", @Namespace = "", @Query = "SELECT Name FROM Account WHERE Id = :accId", @RealLoc = "true"] + | +- BindExpressions[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "(23, 16, 848, 892)", @Namespace = "", @RealLoc = "true"] + | +- VariableExpression[@ApexVersion = "51.0", @DefiningType = "Foo", @Image = "accId", @Location = "(23, 54, 886, 891)", @Namespace = "", @RealLoc = "true"] + | +- EmptyReferenceExpression[@ApexVersion = "51.0", @DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = "false"] + +- Method[@ApexVersion = "51.0", @Arity = "0", @CanonicalName = "", @Constructor = "false", @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReturnType = "void", @Synthetic = "true"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "false", @InheritedSharing = "false", @Location = "(4, 14, 180, 183)", @Modifiers = "8", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "true", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- Method[@ApexVersion = "51.0", @Arity = "0", @CanonicalName = "clone", @Constructor = "false", @DefiningType = "Foo", @Image = "clone", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReturnType = "Object", @Synthetic = "true"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "true", @InheritedSharing = "false", @Location = "no location", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "false", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- UserClassMethods[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false"] + | +- Method[@ApexVersion = "51.0", @Arity = "0", @CanonicalName = "", @Constructor = "true", @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = "false", @ReturnType = "void", @Synthetic = "true"] + | +- ModifierNode[@Abstract = "false", @ApexVersion = "51.0", @DefiningType = "Foo", @Final = "false", @Global = "true", @InheritedSharing = "false", @Location = "(4, 14, 180, 183)", @Modifiers = "0", @Namespace = "", @Override = "false", @Private = "false", @Protected = "false", @Public = "false", @RealLoc = "true", @Static = "false", @Test = "false", @TestOrTestSetup = "false", @Transient = "false", @WebService = "false", @WithSharing = "false", @WithoutSharing = "false"] + +- BridgeMethodCreator[@ApexVersion = "51.0", @DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = "false"] diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithLimitsInLoop.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithLimitsInLoop.xml new file mode 100644 index 0000000000..6c26bfb6c0 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithLimitsInLoop.xml @@ -0,0 +1,413 @@ + + + + + + Problematic Dml Statement in for each + 1 + {1,2}) { + Account account = new Account(); + insert account; + } + } +} + ]]> + + + + Problematic Dml Statement in for loop + 1 + + + + + Problematic Dml Statement in While loop + 1 + + + + + Problematic Dml Statement in do loop + 1 + + + + + Best Practice: Batch up data into a list and invoke your DML once on that list of data. + 0 + accounts = new List(); + + while(true) { + Account account = new Account(); + accounts.add(account); + } + + insert accounts; + } +} + ]]> + + + + + + Problematic Database Statement in for each + 1 + {1,2}) { + Account account = new Account(); + Database.insert(new Account[]{account}, true); + } + } +} + ]]> + + + + Problematic Database Statement in for loop + 1 + + + + + Problematic Database Statement in While loop + 1 + + + + + Problematic Database Statement in do loop + 1 + + + + + Best Practice: Batch up data into a list and invoke your Database method once on that list of data. + 0 + accounts = new List(); + + while(true) { + Account account = new Account(); + accounts.add(account); + } + + Database.insert(accounts, true); + } +} + ]]> + + + + Return database method call is even ok in loop. + 0 + accounts = new List(); + + while(true) { + Account account = new Account(); + return Database.insert(accounts, true); + } + + } +} + ]]> + + + + Database statement as foreach iterator is allowed. + 0 + + + + + + Problematic Soql in for each + 1 + {1,2}) { + List accounts = [SELECT Id FROM Account]; + } + } +} + ]]> + + + + Problematic Soql in for loop + 1 + accounts = [SELECT Id FROM Account]; + } + } +} + ]]> + + + + Problematic Soql in While loop + 1 + accounts = [SELECT Id FROM Account]; + } + } +} + ]]> + + + + Problematic Soql in do loop + 1 + accounts = [SELECT Id FROM Account]; + } while(true); + } +} + ]]> + + + + Multiple problematic Soql expressions + 2 + accounts = [SELECT Id FROM Account]; + List accounts = [SELECT Id FROM Contact]; + } while(true); + } +} + ]]> + + + + Return Soql is even ok in loop + 0 + test1() { + for(;;) { + return [SELECT Id FROM Account]; + } + } +} + ]]> + + + + #29 SOQL For Loops should not throw an Avoid Soql queries inside loops issue + 0 + + + + + [apex] AvoidSoqlInLoops false positive for SOQL with in For-Loop #2598 + 0 + + + + + + Problematic Sosl in for each + 1 + {1,2}) { + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in for loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in While loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + Problematic Sosl in do loop + 1 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } while(true); + } +} + ]]> + + + + Multiple problematic Sosl expressions + 2 + > searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + List> searchList = [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name)]; + } while(true); + } +} + ]]> + + + + Return Sosl is even ok in loop + 0 + test1() { + for(;;) { + return [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity, Lead]; + } + } +} + ]]> + + + + #29 SOSL For Loops should not throw an Avoid Sosl queries inside loops issue + 0 + a : [FIND 'map*' IN ALL FIELDS RETURNING Account (Id, Name), Contact, Opportunity]) { + } + } +} + ]]> + + + diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index 17ba4e1940..f934f33b58 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -44,9 +44,10 @@ import net.sourceforge.pmd.util.ResourceLoader; /** * RuleSetFactory is responsible for creating RuleSet instances from XML * content. By default Rules will be loaded using the {@link RulePriority#LOW} priority, - * with Rule deprecation warnings off. - * By default, the ruleset compatibility filter is active, too. - * See {@link RuleSetFactoryCompatibility}. + * with Rule deprecation warnings off; + * the ruleset compatibility filter is active, too (see {@link RuleSetFactoryCompatibility}); + * if the ruleset contains rule references (e.g. for renamed or moved rules), these + * are ignored by default. */ public class RuleSetFactory { @@ -60,6 +61,7 @@ public class RuleSetFactory { private final RulePriority minimumPriority; private final boolean warnDeprecated; private final RuleSetFactoryCompatibility compatibilityFilter; + private final boolean includeDeprecatedRuleReferences; private final Map parsedRulesets = new HashMap<>(); @@ -88,9 +90,15 @@ public class RuleSetFactory { @Deprecated // to be hidden with PMD 7.0.0. public RuleSetFactory(final ResourceLoader resourceLoader, final RulePriority minimumPriority, final boolean warnDeprecated, final boolean enableCompatibility) { + this(resourceLoader, minimumPriority, warnDeprecated, enableCompatibility, false); + } + + RuleSetFactory(final ResourceLoader resourceLoader, final RulePriority minimumPriority, + final boolean warnDeprecated, final boolean enableCompatibility, boolean includeDeprecatedRuleReferences) { this.resourceLoader = resourceLoader; this.minimumPriority = minimumPriority; this.warnDeprecated = warnDeprecated; + this.includeDeprecatedRuleReferences = includeDeprecatedRuleReferences; if (enableCompatibility) { this.compatibilityFilter = new RuleSetFactoryCompatibility(); @@ -132,27 +140,25 @@ public class RuleSetFactory { */ public Iterator getRegisteredRuleSets() throws RuleSetNotFoundException { String rulesetsProperties = null; - try { - List ruleSetReferenceIds = new ArrayList<>(); - for (Language language : LanguageRegistry.getLanguages()) { - Properties props = new Properties(); - rulesetsProperties = "category/" + language.getTerseName() + "/categories.properties"; - try (InputStream inputStream = resourceLoader.loadClassPathResourceAsStreamOrThrow(rulesetsProperties)) { - props.load(inputStream); - String rulesetFilenames = props.getProperty("rulesets.filenames"); - if (rulesetFilenames != null) { - ruleSetReferenceIds.addAll(RuleSetReferenceId.parse(rulesetFilenames)); - } - } catch (RuleSetNotFoundException e) { - LOG.warning("The language " + language.getTerseName() + " provides no " + rulesetsProperties + "."); + List ruleSetReferenceIds = new ArrayList<>(); + for (Language language : LanguageRegistry.getLanguages()) { + Properties props = new Properties(); + rulesetsProperties = "category/" + language.getTerseName() + "/categories.properties"; + try (InputStream inputStream = resourceLoader.loadClassPathResourceAsStreamOrThrow(rulesetsProperties)) { + props.load(inputStream); + String rulesetFilenames = props.getProperty("rulesets.filenames"); + if (rulesetFilenames != null) { + ruleSetReferenceIds.addAll(RuleSetReferenceId.parse(rulesetFilenames)); } + } catch (RuleSetNotFoundException e) { + LOG.warning("The language " + language.getTerseName() + " provides no " + rulesetsProperties + "."); + } catch (IOException ioe) { + throw new RuntimeException("Couldn't find " + rulesetsProperties + + "; please ensure that the directory is on the classpath. The current classpath is: " + + System.getProperty("java.class.path")); } - return createRuleSets(ruleSetReferenceIds).getRuleSetsIterator(); - } catch (IOException ioe) { - throw new RuntimeException("Couldn't find " + rulesetsProperties - + "; please ensure that the directory is on the classpath. The current classpath is: " - + System.getProperty("java.class.path")); } + return createRuleSets(ruleSetReferenceIds).getRuleSetsIterator(); } /** @@ -224,7 +230,7 @@ public class RuleSetFactory { * if unable to find a resource. */ public RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId) throws RuleSetNotFoundException { - return createRuleSet(ruleSetReferenceId, false); + return createRuleSet(ruleSetReferenceId, includeDeprecatedRuleReferences); } private RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId, boolean withDeprecatedRuleReferences) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java b/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java index e68c6e44ba..69a1ff066c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java @@ -174,6 +174,32 @@ public final class RulesetsFactoryUtils { return new RuleSetFactory(new ResourceLoader(), minimumPriority, warnDeprecated, enableCompatibility); } + /** + * Returns a ruleset factory which uses the classloader for PMD + * classes to resolve resource references. + * + * @param minimumPriority Minimum priority for rules to be included + * @param warnDeprecated If true, print warnings when deprecated rules are included + * @param enableCompatibility If true, rule references to moved rules are mapped to their + * new location if they are known + * @param includeDeprecatedRuleReferences If true, deprecated rule references are retained. Usually, these + * references are ignored, since they indicate renamed/moved rules, and the referenced + * rule is often included in the same ruleset. Enabling this might result in + * duplicated rules. + * + * @return A ruleset factory + * + * @see #createFactory(PMDConfiguration) + */ + public static RuleSetFactory createFactory(RulePriority minimumPriority, + boolean warnDeprecated, + boolean enableCompatibility, + boolean includeDeprecatedRuleReferences) { + + return new RuleSetFactory(new ResourceLoader(), minimumPriority, warnDeprecated, enableCompatibility, + includeDeprecatedRuleReferences); + } + /** * If in debug modus, print the names of the rules. * diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ant/Formatter.java b/pmd-core/src/main/java/net/sourceforge/pmd/ant/Formatter.java index 2ef0afd459..275fb97d0e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ant/Formatter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ant/Formatter.java @@ -182,8 +182,7 @@ public class Formatter { boolean isOnError = true; try { output = Files.newOutputStream(file.toPath()); - writer = new OutputStreamWriter(output, charset); - writer = new BufferedWriter(writer); + writer = new BufferedWriter(new OutputStreamWriter(output, charset)); isOnError = false; } finally { if (isOnError) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java index b96fa57d86..88fc97bcc2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/XMLRenderer.java @@ -48,6 +48,7 @@ public class XMLRenderer extends AbstractIncrementingRenderer { private XMLStreamWriter xmlWriter; private OutputStream stream; + private byte[] lineSeparator; public XMLRenderer() { super(NAME, "XML format."); @@ -67,10 +68,11 @@ public class XMLRenderer extends AbstractIncrementingRenderer { @Override public void start() throws IOException { String encoding = getProperty(ENCODING); + lineSeparator = PMD.EOL.getBytes(encoding); try { xmlWriter.writeStartDocument(encoding, "1.0"); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.setDefaultNamespace(PMD_REPORT_NS_URI); xmlWriter.writeStartElement(PMD_REPORT_NS_URI, "pmd"); xmlWriter.writeDefaultNamespace(PMD_REPORT_NS_URI); @@ -86,6 +88,33 @@ public class XMLRenderer extends AbstractIncrementingRenderer { } } + /** + * Outputs a platform dependent line separator. + * + * @throws XMLStreamException if XMLStreamWriter couldn't be flushed. + * @throws IOException if an I/O error occurs. + */ + private void writeNewLine() throws XMLStreamException, IOException { + /* + * Note: we are not using xmlWriter.writeCharacters(PMD.EOL), because some + * XMLStreamWriter implementations might do extra encoding for \r and/or \n. + * Notably IBM's Java 8 will escape "\r" with " " which will render an + * invalid XML document. IBM's Java 8 would also output a platform dependent + * line separator when writing "\n" which results under Windows, that "\r" + * actually is written twice (once escaped, once raw). + * + * Note2: Before writing the raw bytes to the underlying stream, we need + * to flush XMLStreamWriter. Notably IBM's Java 8 might still need to output + * data. + * + * Note3: Before writing the raw bytes, we issue a empty writeCharacters, + * so that any open tags are closed and we are ready for writing raw bytes. + */ + xmlWriter.writeCharacters(""); + xmlWriter.flush(); + stream.write(lineSeparator); + } + @Override public void renderFileViolations(Iterator violations) throws IOException { String filename = null; @@ -102,10 +131,10 @@ public class XMLRenderer extends AbstractIncrementingRenderer { xmlWriter.writeEndElement(); } filename = nextFilename; - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeStartElement("file"); xmlWriter.writeAttribute("name", filename); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); } xmlWriter.writeStartElement("violation"); @@ -121,11 +150,11 @@ public class XMLRenderer extends AbstractIncrementingRenderer { maybeAdd("variable", rv.getVariableName()); maybeAdd("externalInfoUrl", rv.getRule().getExternalInfoUrl()); xmlWriter.writeAttribute("priority", String.valueOf(rv.getRule().getPriority().getPriority())); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeCharacters(StringUtil.removedInvalidXml10Characters(rv.getDescription())); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeEndElement(); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); } if (filename != null) { // Not first file ? xmlWriter.writeEndElement(); @@ -140,20 +169,20 @@ public class XMLRenderer extends AbstractIncrementingRenderer { try { // errors for (Report.ProcessingError pe : errors) { - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeStartElement("error"); xmlWriter.writeAttribute("filename", determineFileName(pe.getFile())); xmlWriter.writeAttribute("msg", pe.getMsg()); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeCData(pe.getDetail()); - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeEndElement(); } // suppressed violations if (showSuppressedViolations) { for (Report.SuppressedViolation s : suppressed) { - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeStartElement("suppressedviolation"); xmlWriter.writeAttribute("filename", determineFileName(s.getRuleViolation().getFilename())); xmlWriter.writeAttribute("suppressiontype", s.getSuppressor().getId().toLowerCase(Locale.ROOT)); @@ -165,14 +194,15 @@ public class XMLRenderer extends AbstractIncrementingRenderer { // config errors for (final Report.ConfigurationError ce : configErrors) { - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeEmptyElement("configerror"); xmlWriter.writeAttribute("rule", ce.rule().getName()); xmlWriter.writeAttribute("msg", ce.issue()); } - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); xmlWriter.writeEndElement(); // - xmlWriter.writeCharacters(PMD.EOL); + writeNewLine(); + xmlWriter.writeEndDocument(); xmlWriter.flush(); } catch (XMLStreamException e) { throw new IOException(e); diff --git a/pmd-core/src/main/resources/rulesets/releases/6290.xml b/pmd-core/src/main/resources/rulesets/releases/6290.xml new file mode 100644 index 0000000000..59e64c0e46 --- /dev/null +++ b/pmd-core/src/main/resources/rulesets/releases/6290.xml @@ -0,0 +1,13 @@ + + + + +This ruleset contains links to rules that are new in PMD v6.29.0 + + + + + diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java index 1a3aa47c96..209dea08f5 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java @@ -246,6 +246,30 @@ public class RuleSetFactoryTest { assertTrue(logging.getLog().isEmpty()); } + /** + * This is an example of a category (built-in) ruleset, which contains a rule, that has been renamed. + * This means: a rule definition for "NewName" and a rule reference "OldName", that is deprecated + * and exists for backwards compatibility. + * + *

When loading this ruleset at a whole for generating the documentation, we should still + * include the deprecated rule reference, so that we can create a nice documentation. + * + * @throws Exception + */ + @Test + public void testRuleSetWithDeprecatedRenamedRuleForDoc() throws Exception { + RuleSetFactory rsf = RulesetsFactoryUtils.createFactory(RulePriority.LOW, false, false, true); + RuleSet rs = rsf.createRuleSet(createRuleSetReferenceId("\n" + "\n" + + " ruleset desc\n" + + " " + + " " + + " d\n" + " 2\n" + " " + + "")); + assertEquals(2, rs.getRules().size()); + assertNotNull(rs.getRuleByName("NewName")); + assertNotNull(rs.getRuleByName("OldName")); + } + /** * This is an example of a custom user ruleset, that references a rule, that has been renamed. * The user should get a deprecation warning. diff --git a/pmd-doc/src/main/java/net/sourceforge/pmd/docs/GenerateRuleDocsCmd.java b/pmd-doc/src/main/java/net/sourceforge/pmd/docs/GenerateRuleDocsCmd.java index 75c56f7f49..2256a21d18 100644 --- a/pmd-doc/src/main/java/net/sourceforge/pmd/docs/GenerateRuleDocsCmd.java +++ b/pmd-doc/src/main/java/net/sourceforge/pmd/docs/GenerateRuleDocsCmd.java @@ -19,6 +19,7 @@ import java.util.regex.Pattern; import org.apache.commons.io.FilenameUtils; +import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSetFactory; import net.sourceforge.pmd.RuleSetNotFoundException; @@ -39,7 +40,8 @@ public final class GenerateRuleDocsCmd { Path output = FileSystems.getDefault().getPath(args[0]).resolve("..").toAbsolutePath().normalize(); System.out.println("Generating docs into " + output); - RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.defaultFactory(); + // important: use a RuleSetFactory that includes all rules, e.g. deprecated rule references + RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.createFactory(RulePriority.LOW, false, false, true); Iterator registeredRuleSets = ruleSetFactory.getRegisteredRuleSets(); List additionalRulesets = findAdditionalRulesets(output); diff --git a/pmd-doc/src/test/java/net/sourceforge/pmd/docs/RuleDocGeneratorTest.java b/pmd-doc/src/test/java/net/sourceforge/pmd/docs/RuleDocGeneratorTest.java index 00a84abf6e..5e7561cdf3 100644 --- a/pmd-doc/src/test/java/net/sourceforge/pmd/docs/RuleDocGeneratorTest.java +++ b/pmd-doc/src/test/java/net/sourceforge/pmd/docs/RuleDocGeneratorTest.java @@ -21,6 +21,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSetFactory; import net.sourceforge.pmd.RuleSetNotFoundException; @@ -61,7 +62,7 @@ public class RuleDocGeneratorTest { public void testSingleRuleset() throws RuleSetNotFoundException, IOException { RuleDocGenerator generator = new RuleDocGenerator(writer, root); - RuleSetFactory rsf = RulesetsFactoryUtils.defaultFactory(); + RuleSetFactory rsf = RulesetsFactoryUtils.createFactory(RulePriority.LOW, false, false, true); RuleSet ruleset = rsf.createRuleSet("rulesets/ruledoctest/sample.xml"); generator.generate(Arrays.asList(ruleset).iterator(), diff --git a/pmd-doc/src/test/resources/expected/java.md b/pmd-doc/src/test/resources/expected/java.md index a20c0aa803..0ba141b68c 100644 --- a/pmd-doc/src/test/resources/expected/java.md +++ b/pmd-doc/src/test/resources/expected/java.md @@ -15,7 +15,10 @@ folder: pmd/rules * [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer): Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional. * [MovedRule](pmd_rules_java_sample.html#movedrule): Deprecated The rule has been moved to another ruleset. Use instead [JumbledIncrementer](pmd_rules_java_sample2.html#jumbledincrementer). * [OverrideBothEqualsAndHashcode](pmd_rules_java_sample.html#overridebothequalsandhashcode): Override both 'public boolean Object.equals(Object other)', and 'public int Object.hashCode()', o... -* [RenamedRule](pmd_rules_java_sample.html#renamedrule): Deprecated The rule has been renamed. Use instead [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer). +* [RenamedRule1](pmd_rules_java_sample.html#renamedrule1): Deprecated The rule has been renamed. Use instead [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer). +* [RenamedRule2](pmd_rules_java_sample.html#renamedrule2): Deprecated The rule has been renamed. Use instead [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer). +* [RenamedRule3](pmd_rules_java_sample.html#renamedrule3): Deprecated The rule has been renamed. Use instead [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer). +* [RenamedRule4](pmd_rules_java_sample.html#renamedrule4): Deprecated The rule has been renamed. Use instead [JumbledIncrementer](pmd_rules_java_sample.html#jumbledincrementer). * [XSSInDocumentation](pmd_rules_java_sample.html#xssindocumentation): <script>alert('XSS at the beginning');</script> HTML tags might appear at various places. ... ## Additional rulesets diff --git a/pmd-doc/src/test/resources/expected/sample.md b/pmd-doc/src/test/resources/expected/sample.md index ff3c6fc81d..c1f20199a4 100644 --- a/pmd-doc/src/test/resources/expected/sample.md +++ b/pmd-doc/src/test/resources/expected/sample.md @@ -5,7 +5,7 @@ permalink: pmd_rules_java_sample.html folder: pmd/rules/java sidebaractiveurl: /pmd_rules_java.html editmepath: ../rulesets/ruledoctest/sample.xml -keywords: Sample, XSSInDocumentation, OverrideBothEqualsAndHashcode, JumbledIncrementer, DeprecatedSample, RenamedRule, MovedRule +keywords: Sample, XSSInDocumentation, OverrideBothEqualsAndHashcode, JumbledIncrementer, DeprecatedSample, RenamedRule1, RenamedRule2, RenamedRule3, RenamedRule4, MovedRule language: Java --- @@ -204,7 +204,7 @@ public class Foo { ``` -## RenamedRule +## RenamedRule1 Deprecated @@ -257,12 +257,235 @@ Avoid jumbled loop incrementers - its usually a mistake, and is confusing even i **Use this rule with the default properties by just referencing it:** ``` xml - + ``` **Use this rule and customize it:** ``` xml - + + + + + + + + + + + + + +``` + +## RenamedRule2 + +Deprecated + +This rule has been renamed. Use instead: [JumbledIncrementer](#jumbledincrementer) + +**Since:** PMD 1.0 + +**Priority:** Medium (3) + +Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional. + +**This rule is defined by the following XPath expression:** +``` xpath +//ForStatement + [ + ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image + = + ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image + ] +``` + +**Example(s):** + +``` java +{%raw%}public class JumbledIncrementerRule1 { + public void foo() { + for (int i = 0; i < 10; i++) { // only references 'i' + for (int k = 0; k < 20; i++) { // references both 'i' and 'k' + System.out.println("Hello"); + } + } + } +}{%endraw%} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description|Multivalued| +|----|-------------|-----------|-----------| +|sampleAdditionalProperty|the value|This is a additional property for tests|no| +|sampleMultiStringProperty|Value1 \| Value2|Test property with multiple strings|yes. Delimiter is '\|'.| +|sampleDeprecatedProperty|test|Deprecated This is a sample deprecated property for tests|no| +|sampleRegexProperty1|\\/\\\*\\s+(default\|package)\\s+\\\*\\/|The property is of type regex|no| +|sampleRegexProperty2|\[a-z\]\*|The property is of type regex|no| +|sampleRegexProperty3|\\s+|The property is of type regex|no| +|sampleRegexProperty4|\_dd\_|The property is of type regex|no| +|sampleRegexProperty5|\[0-9\]{1,3}|The property is of type regex|no| +|sampleRegexProperty6|\\b|The property is of type regex|no| +|sampleRegexProperty7|\\n|The property is of type regex|no| + +**Use this rule with the default properties by just referencing it:** +``` xml + +``` + +**Use this rule and customize it:** +``` xml + + + + + + + + + + + + + +``` + +## RenamedRule3 + +Deprecated + +This rule has been renamed. Use instead: [JumbledIncrementer](#jumbledincrementer) + +Deprecated + +**Since:** PMD 1.0 + +**Priority:** Medium (3) + +Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional. + +**This rule is defined by the following XPath expression:** +``` xpath +//ForStatement + [ + ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image + = + ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image + ] +``` + +**Example(s):** + +``` java +{%raw%}public class JumbledIncrementerRule1 { + public void foo() { + for (int i = 0; i < 10; i++) { // only references 'i' + for (int k = 0; k < 20; i++) { // references both 'i' and 'k' + System.out.println("Hello"); + } + } + } +}{%endraw%} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description|Multivalued| +|----|-------------|-----------|-----------| +|sampleAdditionalProperty|the value|This is a additional property for tests|no| +|sampleMultiStringProperty|Value1 \| Value2|Test property with multiple strings|yes. Delimiter is '\|'.| +|sampleDeprecatedProperty|test|Deprecated This is a sample deprecated property for tests|no| +|sampleRegexProperty1|\\/\\\*\\s+(default\|package)\\s+\\\*\\/|The property is of type regex|no| +|sampleRegexProperty2|\[a-z\]\*|The property is of type regex|no| +|sampleRegexProperty3|\\s+|The property is of type regex|no| +|sampleRegexProperty4|\_dd\_|The property is of type regex|no| +|sampleRegexProperty5|\[0-9\]{1,3}|The property is of type regex|no| +|sampleRegexProperty6|\\b|The property is of type regex|no| +|sampleRegexProperty7|\\n|The property is of type regex|no| + +**Use this rule with the default properties by just referencing it:** +``` xml + +``` + +**Use this rule and customize it:** +``` xml + + + + + + + + + + + + + +``` + +## RenamedRule4 + +Deprecated + +This rule has been renamed. Use instead: [JumbledIncrementer](#jumbledincrementer) + +Deprecated + +**Since:** PMD 1.0 + +**Priority:** Medium (3) + +Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional. + +**This rule is defined by the following XPath expression:** +``` xpath +//ForStatement + [ + ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image + = + ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image + ] +``` + +**Example(s):** + +``` java +{%raw%}public class JumbledIncrementerRule1 { + public void foo() { + for (int i = 0; i < 10; i++) { // only references 'i' + for (int k = 0; k < 20; i++) { // references both 'i' and 'k' + System.out.println("Hello"); + } + } + } +}{%endraw%} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description|Multivalued| +|----|-------------|-----------|-----------| +|sampleAdditionalProperty|the value|This is a additional property for tests|no| +|sampleMultiStringProperty|Value1 \| Value2|Test property with multiple strings|yes. Delimiter is '\|'.| +|sampleDeprecatedProperty|test|Deprecated This is a sample deprecated property for tests|no| +|sampleRegexProperty1|\\/\\\*\\s+(default\|package)\\s+\\\*\\/|The property is of type regex|no| +|sampleRegexProperty2|\[a-z\]\*|The property is of type regex|no| +|sampleRegexProperty3|\\s+|The property is of type regex|no| +|sampleRegexProperty4|\_dd\_|The property is of type regex|no| +|sampleRegexProperty5|\[0-9\]{1,3}|The property is of type regex|no| +|sampleRegexProperty6|\\b|The property is of type regex|no| +|sampleRegexProperty7|\\n|The property is of type regex|no| + +**Use this rule with the default properties by just referencing it:** +``` xml + +``` + +**Use this rule and customize it:** +``` xml + diff --git a/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml b/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml index da81d702d1..4037c8ffb8 100644 --- a/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml +++ b/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml @@ -216,8 +216,18 @@ RuleTag with full category and without quotes: Use {% rule java/sample/RenamedRu - + - + + + + + + + + + + diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java index 34d9e6d794..3f2058eb54 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java @@ -37,6 +37,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTResourceList; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; @@ -44,6 +45,7 @@ import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.types.TypeTestUtil; +import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.properties.PropertyDescriptor; /** @@ -65,6 +67,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor; public class CloseResourceRule extends AbstractJavaRule { private static final String WRAPPING_TRY_WITH_RES_VAR_MESSAGE = "it is recommended to wrap resource in try-with-resource declaration directly"; + private static final String REASSIGN_BEFORE_CLOSED_MESSAGE = "'' is reassigned, but the original instance is not closed"; private static final String CLOSE_IN_FINALLY_BLOCK_MESSAGE = "'' is not closed within a finally block, thus might not be closed at all in case of exceptions"; private static final PropertyDescriptor> CLOSE_TARGETS_DESCRIPTOR = @@ -164,6 +167,12 @@ public class CloseResourceRule extends AbstractJavaRule { } else if (shouldVarOfTypeBeClosedInMethod(resVar, resVarType, methodOrConstructor)) { reportedVarNames.add(resVar.getVarId().getName()); addCloseResourceViolation(resVar.getVarId(), resVarType, data); + } else { + ASTStatementExpression reassigningStatement = getFirstReassigningStatementBeforeBeingClosed(resVar, methodOrConstructor); + if (reassigningStatement != null) { + reportedVarNames.add(resVar.getVarId().getName()); + addViolationWithMessage(data, reassigningStatement, reassignBeforeClosedMessageForVar(resVar.getName())); + } } } } @@ -660,4 +669,77 @@ public class CloseResourceRule extends AbstractJavaRule { private String closeInFinallyBlockMessageForVar(String var) { return "''" + var + CLOSE_IN_FINALLY_BLOCK_MESSAGE; } + + private String reassignBeforeClosedMessageForVar(String var) { + return "''" + var + REASSIGN_BEFORE_CLOSED_MESSAGE; + } + + private ASTStatementExpression getFirstReassigningStatementBeforeBeingClosed(ASTVariableDeclarator variable, ASTMethodOrConstructorDeclaration methodOrConstructor) { + List statements = methodOrConstructor.findDescendantsOfType(ASTStatementExpression.class); + boolean variableClosed = false; + boolean isInitialized = !hasNullInitializer(variable); + ASTExpression initializingExpression = initializerExpressionOf(variable); + for (ASTStatementExpression statement : statements) { + if (isClosingVariableStatement(statement, variable)) { + variableClosed = true; + } + + if (isAssignmentForVariable(statement, variable)) { + if (isInitialized && !variableClosed) { + if (initializingExpression != null && !inSameIfBlock(statement, initializingExpression)) { + return statement; + } + } + + if (variableClosed) { + variableClosed = false; + } + if (!isInitialized) { + isInitialized = true; + initializingExpression = statement.getFirstDescendantOfType(ASTExpression.class); + } + } + } + return null; + } + + private boolean inSameIfBlock(ASTStatementExpression statement1, ASTExpression statement2) { + List parents1 = statement1.getParentsOfType(ASTIfStatement.class); + List parents2 = statement2.getParentsOfType(ASTIfStatement.class); + parents1.retainAll(parents2); + return !parents1.isEmpty(); + } + + private boolean isClosingVariableStatement(ASTStatementExpression statement, ASTVariableDeclarator variable) { + List expressions = statement.findDescendantsOfType(ASTPrimaryExpression.class); + for (ASTPrimaryExpression expression : expressions) { + if (isMethodCallClosingResourceVariable(expression, variable.getName())) { + return true; + } + } + List names = statement.findDescendantsOfType(ASTName.class); + for (ASTName name : names) { + if (isCloseCallOnVariable(name, variable.getName())) { + return true; + } + } + return false; + } + + private boolean isAssignmentForVariable(ASTStatementExpression statement, ASTVariableDeclarator variable) { + if (statement == null || variable == null) { + return false; + } + ASTName name = statement.getFirstDescendantOfType(ASTName.class); + if (name == null) { + return false; + } + NameDeclaration statementVariable = name.getNameDeclaration(); + if (statementVariable == null) { + return false; + } + + return statement.hasDescendantOfType(ASTAssignmentOperator.class) + && statementVariable.equals(variable.getVarId().getNameDeclaration()); + } } diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 7968b6ec57..8b743427ae 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -411,11 +411,11 @@ public class Foo { private Bar var2; //followed by many imports of unique objects - void ObjectC doWork() { + ObjectC doWork() { Bardo var55; ObjectA var44; ObjectZ var93; - return something; + return something(); } } ]]> @@ -616,7 +616,7 @@ user-specified threshold. + + + + #2764 false-negative when re-assigning connection + java.sql.Connection,java.sql.Statement,java.sql.ResultSet + 1 + 7 + + 'c' is reassigned, but the original instance is not closed + + + + + + #2764 false-negative when re-assigning connection - no problem when closed before + java.sql.Connection,java.sql.Statement,java.sql.ResultSet + 0 + diff --git a/pmd-visualforce/etc/grammar/Vf.jjt b/pmd-visualforce/etc/grammar/Vf.jjt index 8edfcc2612..2325155a9b 100644 --- a/pmd-visualforce/etc/grammar/Vf.jjt +++ b/pmd-visualforce/etc/grammar/Vf.jjt @@ -66,6 +66,7 @@ PARSER_END(VfParserImpl) | <#ALPHANUM_CHAR: ( | ) > | <#IDENTIFIER_CHAR: ( | [ "_", "-", ":" ] ) > | <#IDENTIFIER: ()* > +| <#IDENTIFIER_DOTTED: ( )+ > | <#XMLNAME: ( | "_" | ":") ()* > | <#QUOTED_STRING_NO_BREAKS: ( "'" ( ~["'", "\r", "\n"] )* "'" ) | ( "\"" ( ~["\"", "\r", "\n"] )* "\"" ) > @@ -215,7 +216,7 @@ PARSER_END(VfParserImpl) TOKEN : { - > + | > | " > : AfterTagState | " | "!>") > : AfterTagState | " | "/ >") > : AfterTagState diff --git a/pmd-visualforce/src/test/java/net/sourceforge/pmd/lang/vf/ast/VfParserTest.java b/pmd-visualforce/src/test/java/net/sourceforge/pmd/lang/vf/ast/VfParserTest.java index f425470314..6ce22de09c 100644 --- a/pmd-visualforce/src/test/java/net/sourceforge/pmd/lang/vf/ast/VfParserTest.java +++ b/pmd-visualforce/src/test/java/net/sourceforge/pmd/lang/vf/ast/VfParserTest.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.vf.ast; import org.junit.Test; +import net.sourceforge.pmd.lang.ast.ParseException; + /** * @author sergey.gorbaty */ @@ -26,4 +28,39 @@ public class VfParserTest extends AbstractVfNodesTest { vf.parse("${\"yes\"}"); } + @Test + public void testAttributeNameWithDot() { + vf.parse(""); + } + + @Test + public void testAttributeNameWithUnderscore() { + vf.parse(""); + } + + @Test + public void testAttributeNameWithColon() { + vf.parse(""); + } + + @Test(expected = ParseException.class) + public void testAttributeNameWithInvalidSymbol() { + vf.parse(""); + } + + @Test(expected = ParseException.class) + public void testAttributeNameWithInvalidDot() { + vf.parse(""); + } + + @Test(expected = ParseException.class) + public void testAttributeNameWithInvalidDotV2() { + vf.parse(""); + } + + @Test(expected = ParseException.class) + public void testAttributeNameWithInvalidDotV3() { + vf.parse(""); + } + } diff --git a/pom.xml b/pom.xml index fb5838c4e6..96c3774d56 100644 --- a/pom.xml +++ b/pom.xml @@ -76,7 +76,7 @@ - 2020-09-26T08:25:16Z + 2020-10-24T08:17:24Z 8 @@ -97,7 +97,7 @@ 3.13.0 1.10.8 3.2.0 - 4.7 + 4.7.2 UTF-8 UTF-8 @@ -284,7 +284,7 @@ org.junit.vintage junit-vintage-engine - 5.6.2 + 5.7.0 @@ -415,12 +415,12 @@ net.sourceforge.pmd pmd-core - 6.28.0 + 6.29.0 net.sourceforge.pmd pmd-java - 6.28.0 + 6.29.0 @@ -768,7 +768,7 @@ junit junit - 4.13 + 4.13.1 test