From 19f8752a432aa1325a979916d67b58686a6fe412 Mon Sep 17 00:00:00 2001 From: orimarko Date: Sun, 11 Nov 2018 15:07:50 +0200 Subject: [PATCH 1/5] Add Test for issue #647 Add test to verify fix for issue #647: JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` --- .../xml/JUnitTestsShouldIncludeAssert.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml index b69044eff3..a44902ff99 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml @@ -431,4 +431,19 @@ class Style { } }]]> + + #647 JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` + 2 + + From bf1542f272d30c4126a039dc52ad0e5cf1a971de Mon Sep 17 00:00:00 2001 From: orimarko Date: Sun, 11 Nov 2018 15:12:04 +0200 Subject: [PATCH 2/5] Update JUnitTestsShouldIncludeAssert.xml --- .../rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml index a44902ff99..4132fd0e4e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml @@ -433,7 +433,7 @@ class Style { #647 JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` - 2 + 1 Date: Mon, 12 Nov 2018 14:06:12 +0200 Subject: [PATCH 3/5] Fix Test Fix Test add `import org.junit.*;` --- .../rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml index 4132fd0e4e..e972736cf0 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml @@ -435,6 +435,7 @@ class Style { #647 JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` 1 Date: Tue, 13 Nov 2018 08:05:40 +0200 Subject: [PATCH 4/5] Update Test to check this not creating failure --- .../bestpractices/xml/JUnitTestsShouldIncludeAssert.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml index e972736cf0..9c05a7ca41 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnitTestsShouldIncludeAssert.xml @@ -433,7 +433,7 @@ class Style { #647 JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` - 1 + 0 From 8f268c81e6576509936d9b249299275d4cd7b35d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 1 Dec 2018 19:11:12 +0100 Subject: [PATCH 5/5] [java] JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` Fixes #647 --- docs/pages/release_notes.md | 1 + .../JUnitTestsShouldIncludeAssertRule.java | 42 +++++++++---------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 689b1e48d4..ce565bf154 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -41,6 +41,7 @@ This means, you can use CPD to find duplicated code in your Kotlin projects. * java * [#1460](https://github.com/pmd/pmd/issues/1460): \[java] Intermittent PMD failure : PMD processing errors while no violations reported * java-bestpractices + * [#647](https://github.com/pmd/pmd/issues/647): \[java] JUnitTestsShouldIncludeAssertRule should support `this.exception` as well as just `exception` * [#1435](https://github.com/pmd/pmd/issues/1435): \[java] JUnitTestsShouldIncludeAssert: Support AssertJ soft assertions * java-codestyle * [#1232](https://github.com/pmd/pmd/issues/1232): \[java] Detector for large numbers not separated by _ diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java index 4cbfb154b2..f7c8e7b424 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestsShouldIncludeAssertRule.java @@ -17,6 +17,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTNormalAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTReferenceType; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; @@ -173,29 +175,25 @@ public class JUnitTestsShouldIncludeAssertRule extends AbstractJUnitRule { private boolean isExpectStatement(ASTStatementExpression expression, Map> expectables) { - - if (expression != null) { - - ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class); - if (pe != null) { - Node name = pe.getFirstDescendantOfType(ASTName.class); - // case of an AllocationExpression - if (name == null) { - return false; + ASTPrimaryExpression pe = expression.getFirstChildOfType(ASTPrimaryExpression.class); + if (pe != null) { + ASTPrimaryPrefix primaryPrefix = pe.getFirstChildOfType(ASTPrimaryPrefix.class); + Node name = pe.getFirstDescendantOfType(ASTName.class); + if (!primaryPrefix.usesThisModifier() && name != null) { + String[] parts = name.getImage().split("\\."); + if (parts.length >= 2) { + String varname = parts[0]; + String methodName = parts[1]; + if (expectables.containsKey(varname) && "expect".equals(methodName)) { + return true; + } } - - String img = name.getImage(); - if (img.indexOf(".") == -1) { - return false; - } - String varname = img.split("\\.")[0]; - - if (!expectables.containsKey(varname)) { - return false; - } - - for (NameOccurrence occ : expectables.get(varname)) { - if (occ.getLocation() == name && img.startsWith(varname + ".expect")) { + } else if (primaryPrefix.usesThisModifier()) { + List primarySuffixes = pe.findChildrenOfType(ASTPrimarySuffix.class); + if (primarySuffixes.size() >= 2) { + String varname = primarySuffixes.get(0).getImage(); + String methodName = primarySuffixes.get(1).getImage(); + if (expectables.containsKey(varname) && "expect".equals(methodName)) { return true; } }