diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 7097969edc..39aef42caa 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,6 +15,8 @@ This is a {{ site.pmd.release_type }} release. ### 🚀 New and noteworthy ### 🐛 Fixed Issues +* apex-performance + * [#5139](https://github.com/pmd/pmd/issues/5139): \[apex] OperationWithHighCostInLoop not firing in triggers * plsql-bestpractices * [#5132](https://github.com/pmd/pmd/issues/5132): \[plsql] TomKytesDespair - exception for more complex exception handler diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTMethod.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTMethod.java index 69367768be..3f44ef5430 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTMethod.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTMethod.java @@ -10,6 +10,7 @@ import java.util.stream.Collectors; import net.sourceforge.pmd.lang.document.TextDocument; import net.sourceforge.pmd.lang.document.TextPos2d; import net.sourceforge.pmd.lang.document.TextRegion; +import net.sourceforge.pmd.lang.rule.xpath.NoAttribute; import com.google.summit.ast.SourceLocation; import com.google.summit.ast.declaration.MethodDeclaration; @@ -27,6 +28,12 @@ public final class ASTMethod extends AbstractApexNode implements ApexQualifiable */ private static final String STATIC_INIT_ID = ""; + /** + * Internal name used by the synthetic trigger method. + * @see #isTriggerBlock() + */ + private static final String TRIGGER_INVOKE_ID = ""; + // Store the details instead of wrapping a com.google.summit.ast.Node. // This is to allow synthetic ASTMethod nodes. // An example is the trigger `invoke` method. @@ -150,4 +157,14 @@ public final class ASTMethod extends AbstractApexNode implements ApexQualifiable public int getArity() { return parameterTypes.size(); } + + /** + * Checks whether this method is the synthetic trigger method. + * @return true if this method is the synthetic trigger method + * @since 7.5.0 + */ + @NoAttribute + public boolean isTriggerBlock() { + return TRIGGER_INVOKE_ID.equals(internalName); + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.kt b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.kt index a41745ce93..20dd5621aa 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.kt +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.kt @@ -229,7 +229,7 @@ class ApexTreeBuilder(private val task: ParserTask, private val proc: ApexLangua // 2. Add the expected ASTModifier child node buildModifiers(emptyList()).also { it.setParent(invokeMethod) } // 3. Elide the body CompoundStatement->ASTBlockStatement - node.body.forEach { buildChildren(it, parent = invokeMethod as AbstractApexNode) } + node.body.forEach { buildAndSetParent(it, parent = invokeMethod as AbstractApexNode) } } else { buildChildren(node, parent = this, exclude = { it in node.modifiers }) } @@ -737,18 +737,18 @@ class ApexTreeBuilder(private val task: ParserTask, private val proc: ApexLangua findDescendants(root, nodeType = ASTProperty::class).forEach { node -> generateFields(node) } // Sort resulting nodes - findDescendants(root, nodeType = ASTUserClass::class).forEach { node -> + findDescendants(root, nodeType = BaseApexClass::class).forEach { node -> sortUserClassChildren(node) } } /** - * Sort children of [ASTUserClass] in historical order. + * Sort children of [BaseApexClass] (ASTUserClass, ASTUserTrigger, ...) in historical order. * * This sorts [ASTField] nodes immediately after [ASTModifierNode] nodes at * the start of the ordered children. */ - private fun sortUserClassChildren(node: ASTUserClass) { + private fun sortUserClassChildren(node: BaseApexClass<*>) { val children = ArrayList(node.children().toList()) children.sortBy{ when (it) { @@ -772,7 +772,13 @@ class ApexTreeBuilder(private val task: ParserTask, private val proc: ApexLangua /** Generates [ASTField] nodes for the [ASTFieldDeclarationStatements]. */ private fun generateFields(node: ASTFieldDeclarationStatements) { - val parent = node.parent as BaseApexClass<*> + val parent = if (node.parent is BaseApexClass<*>) { + node.parent as BaseApexClass<*> + } else if (node.parent is ASTMethod && (node.parent as ASTMethod).isTriggerBlock) { + node.parent.parent as BaseApexClass<*> + } else { + throw IllegalStateException("Unexpected apex tree - field declaration $node cannot appear hear") + } node.node.declarations .map { decl -> 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 index 0bba0ee4f8..997d76819e 100644 --- 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 @@ -65,4 +65,9 @@ class ApexTreeDumpTest extends BaseTreeDumpTest { void switchStatements() { doTest("SwitchStatements"); } + + @Test + void trigger() { + doTest("AccountTrigger"); + } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.cls b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.cls new file mode 100644 index 0000000000..39b81e2263 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.cls @@ -0,0 +1,9 @@ +// see https://github.com/pmd/pmd/issues/5139 +trigger AccountTrigger on Account (before insert, before update) { + integer i = 0; + for (i = 0; i <15; i++) { + SObjectType token = Schema.getGlobalDescribe().get('Account'); + } + integer anotherField = 2; + System.debug('test'); +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.txt b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.txt new file mode 100644 index 0000000000..b5999de53f --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/AccountTrigger.txt @@ -0,0 +1,50 @@ ++- ApexFile[@DefiningType = "AccountTrigger", @RealLoc = true] + +- UserTrigger[@DefiningType = "AccountTrigger", @Image = "AccountTrigger", @Nested = false, @RealLoc = true, @SimpleName = "AccountTrigger", @TargetName = "Account", @Usages = (TriggerUsage.BEFORE_INSERT, TriggerUsage.BEFORE_UPDATE)] + +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- Field[@DefiningType = "AccountTrigger", @Image = "i", @Name = "i", @RealLoc = true, @Type = "Integer", @Value = "0"] + | +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- Field[@DefiningType = "AccountTrigger", @Image = "anotherField", @Name = "anotherField", @RealLoc = true, @Type = "Integer", @Value = "2"] + | +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- Method[@Arity = 0, @CanonicalName = "invoke", @Constructor = false, @DefiningType = "AccountTrigger", @Image = "invoke", @RealLoc = false, @ReturnType = "void", @StaticInitializer = false] + +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- FieldDeclarationStatements[@DefiningType = "AccountTrigger", @RealLoc = true, @TypeArguments = (), @TypeName = "Integer"] + | +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- FieldDeclaration[@DefiningType = "AccountTrigger", @Image = "i", @Name = "i", @RealLoc = true] + | +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "0", @Integer = true, @LiteralType = LiteralType.INTEGER, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = false] + | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "i", @RealLoc = true] + | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + +- ForLoopStatement[@DefiningType = "AccountTrigger", @RealLoc = true] + | +- StandardCondition[@DefiningType = "AccountTrigger", @RealLoc = true] + | | +- BooleanExpression[@DefiningType = "AccountTrigger", @Op = BooleanOperator.LESS_THAN, @RealLoc = true] + | | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "i", @RealLoc = true] + | | | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + | | +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "15", @Integer = true, @LiteralType = LiteralType.INTEGER, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = false] + | +- Expression[@DefiningType = "AccountTrigger", @RealLoc = true] + | | +- AssignmentExpression[@DefiningType = "AccountTrigger", @Op = AssignmentOperator.EQUALS, @RealLoc = true] + | | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "i", @RealLoc = true] + | | | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + | | +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "0", @Integer = true, @LiteralType = LiteralType.INTEGER, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = false] + | +- BlockStatement[@CurlyBrace = true, @DefiningType = "AccountTrigger", @RealLoc = true] + | | +- VariableDeclarationStatements[@DefiningType = "AccountTrigger", @RealLoc = true] + | | +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- VariableDeclaration[@DefiningType = "AccountTrigger", @Image = "token", @RealLoc = true, @Type = "SObjectType"] + | | +- MethodCallExpression[@DefiningType = "AccountTrigger", @FullMethodName = "get", @InputParametersSize = 1, @MethodName = "get", @RealLoc = true] + | | | +- ReferenceExpression[@DefiningType = "AccountTrigger", @Image = "", @RealLoc = false, @ReferenceType = ReferenceType.METHOD, @SObjectType = false, @SafeNav = false] + | | | | +- MethodCallExpression[@DefiningType = "AccountTrigger", @FullMethodName = "Schema.getGlobalDescribe", @InputParametersSize = 0, @MethodName = "getGlobalDescribe", @RealLoc = true] + | | | | +- ReferenceExpression[@DefiningType = "AccountTrigger", @Image = "Schema", @RealLoc = true, @ReferenceType = ReferenceType.METHOD, @SObjectType = false, @SafeNav = false] + | | | +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "Account", @Integer = false, @LiteralType = LiteralType.STRING, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = true] + | | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "token", @RealLoc = true] + | | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + | +- PostfixExpression[@DefiningType = "AccountTrigger", @Op = PostfixOperator.INCREMENT, @RealLoc = true] + | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "i", @RealLoc = true] + | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + +- FieldDeclarationStatements[@DefiningType = "AccountTrigger", @RealLoc = true, @TypeArguments = (), @TypeName = "Integer"] + | +- ModifierNode[@Abstract = false, @DefiningType = "AccountTrigger", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- FieldDeclaration[@DefiningType = "AccountTrigger", @Image = "anotherField", @Name = "anotherField", @RealLoc = true] + | +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "2", @Integer = true, @LiteralType = LiteralType.INTEGER, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = false] + | +- VariableExpression[@DefiningType = "AccountTrigger", @Image = "anotherField", @RealLoc = true] + | +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false] + +- ExpressionStatement[@DefiningType = "AccountTrigger", @RealLoc = true] + +- MethodCallExpression[@DefiningType = "AccountTrigger", @FullMethodName = "System.debug", @InputParametersSize = 1, @MethodName = "debug", @RealLoc = true] + +- ReferenceExpression[@DefiningType = "AccountTrigger", @Image = "System", @RealLoc = true, @ReferenceType = ReferenceType.METHOD, @SObjectType = false, @SafeNav = false] + +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "AccountTrigger", @Double = false, @Image = "test", @Integer = false, @LiteralType = LiteralType.STRING, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = true] diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml index fd282bac4a..8fcbd0b6d7 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/design/xml/CyclomaticComplexity.xml @@ -263,7 +263,7 @@ trigger CaseAssignLevel on CaseAssignLevel__c (after delete, after insert, after 1 1 - The trigger 'CaseAssignLevel' has a cyclomatic complexity of 9. + The trigger 'CaseAssignLevel' has a cyclomatic complexity of 12. diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithHighCostInLoop.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithHighCostInLoop.xml index 15a05db1a0..e03e0ea17b 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithHighCostInLoop.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithHighCostInLoop.xml @@ -135,4 +135,18 @@ public class Foo { ]]> + + + #5139 [apex] OperationWithHighCostInLoop not firing in triggers + 1 + 4 + +