From cc408d2876b6fc7e0b080f00d215aa4a2e8ef239 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 13 Jul 2020 20:00:46 +0200 Subject: [PATCH] [java] AvoidInstantiatingObjectsInLoopsRule - fix false negative --- .../AvoidInstantiatingObjectsInLoopsRule.java | 4 +- .../xml/AvoidInstantiatingObjectsInLoops.xml | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java index 247d5b8a3d..6a8be010f1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java @@ -76,7 +76,9 @@ public class AvoidInstantiatingObjectsInLoopsRule extends AbstractJavaRule { ASTBlock block = blockStatement.getFirstParentOfType(ASTBlock.class); if (block.getNumChildren() > blockStatement.getIndexInParent() + 1) { ASTBlockStatement next = (ASTBlockStatement) block.getChild(blockStatement.getIndexInParent() + 1); - return !next.hasDescendantOfType(ASTBreakStatement.class); + if (next.getNumChildren() == 1 && next.getChild(0).getNumChildren() == 1) { + return !(next.getChild(0).getChild(0) instanceof ASTBreakStatement); + } } } return true; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml index 28f7a76488..c875609d59 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml @@ -163,6 +163,45 @@ public class PMDDemo { cars.add(new Car()); } } +} + ]]> + + + + False negative with break in other for-loop + 1 + 7 + getFilteredMessages( + String fileName, FileContents fileContents, DetailAST rootAST) { + final SortedSet result = new TreeSet<>(messages); + for (LocalizedMessage element : messages) { + final TreeWalkerAuditEvent event = + new TreeWalkerAuditEvent(fileContents, fileName, element, rootAST); + for (TreeWalkerFilter filter : filters) { + if (!filter.accept(event)) { + result.remove(element); + break; + } + } + } + return result; + } +} + ]]> + + + + Instantiation in loop condition + 1 + 3 + 0) { + } + } } ]]>