diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index bc1a36ec7a..f0ff70714e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,11 +14,29 @@ This is a {{ site.pmd.release_type }} release. ### πŸš€ New and noteworthy +### 🌟 Rule Changes + +#### Renamed Rules +Several rules for unit testing have been renamed to better reflect their actual scope. Lots of them were called +after JUnit / JUnit 4, even when they applied to JUnit 5 and / or TestNG. + +* {% rule java/bestpractices/UnitTestShouldUseAfterAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseAfterAnnotation`. +* {% rule java/bestpractices/UnitTestShouldUseBeforeAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseBeforeAnnotation`. +* {% rule java/bestpractices/UnitTestShouldUseTestAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseTestAnnotation`. +* {% rule java/bestpractices/UnitTestAssertionsShouldIncludeMessage %} (Java Best Practices) has been renamed from `JUnitAssertionsShouldIncludeMessage`. +* {% rule java/bestpractices/UnitTestContainsTooManyAsserts %} (Java Best Practices) has been renamed from `JUnitTestContainsTooManyAsserts`. +* {% rule java/bestpractices/UnitTestsShouldIncludeAssert %} (Java Best Practices) has been renamed from `JUnitTestsShouldIncludeAssert`. + +The old rule names still work but are deprecated. + ### πŸ› Fixed Issues +* java + * [#4532](https://github.com/pmd/pmd/issues/4532): \[java] Rule misnomer for JUnit* rules ### 🚨 API Changes -### ✨ External Contributions +### ✨ Merged pull requests +* [#4965](https://github.com/pmd/pmd/pull/4965): \[java] Rename JUnit rules with overly restrictive names - [Juan MartΓ­n Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) {% endtocmaker %} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitAssertionsShouldIncludeMessageRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitAssertionsShouldIncludeMessageRule.java index 915415a0a3..4da48f44b0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitAssertionsShouldIncludeMessageRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitAssertionsShouldIncludeMessageRule.java @@ -4,41 +4,10 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; -import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; -import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; -import net.sourceforge.pmd.lang.java.types.InvocationMatcher; -import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher; +/** + * @deprecated The rule was renamed {@link UnitTestAssertionsShouldIncludeMessageRule} + */ +@Deprecated +public class JUnitAssertionsShouldIncludeMessageRule extends UnitTestAssertionsShouldIncludeMessageRule { -public class JUnitAssertionsShouldIncludeMessageRule extends AbstractJavaRulechainRule { - - private final CompoundInvocationMatcher checks = - InvocationMatcher.parseAll( - "_#assertEquals(_,_)", - "_#assertTrue(_)", - "_#assertFalse(_)", - "_#assertSame(_,_)", - "_#assertNotSame(_,_)", - "_#assertNull(_)", - "_#assertNotNull(_)", - "_#assertArrayEquals(_,_)", - "_#assertThat(_,_)", - "_#fail()", - "_#assertEquals(float,float,float)", - "_#assertEquals(double,double,double)" - ); - - public JUnitAssertionsShouldIncludeMessageRule() { - super(ASTMethodCall.class); - } - - @Override - public Object visit(ASTMethodCall node, Object data) { - if (TestFrameworksUtil.isCallOnAssertionContainer(node)) { - if (checks.anyMatch(node)) { - asCtx(data).addViolation(node); - } - } - return null; - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestContainsTooManyAssertsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestContainsTooManyAssertsRule.java index e68ac3e9ac..13b9d247e1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestContainsTooManyAssertsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitTestContainsTooManyAssertsRule.java @@ -4,54 +4,10 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; -import java.util.Set; -import java.util.stream.Collectors; +/** + * @deprecated The rule was renamed {@link UnitTestContainsTooManyAssertsRule} + */ +@Deprecated +public class JUnitTestContainsTooManyAssertsRule extends UnitTestContainsTooManyAssertsRule { -import net.sourceforge.pmd.lang.java.ast.ASTBlock; -import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; -import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; -import net.sourceforge.pmd.properties.NumericConstraints; -import net.sourceforge.pmd.properties.PropertyDescriptor; -import net.sourceforge.pmd.properties.PropertyFactory; - -public class JUnitTestContainsTooManyAssertsRule extends AbstractJavaRulechainRule { - - private static final PropertyDescriptor MAX_ASSERTS = - PropertyFactory.intProperty("maximumAsserts") - .desc("Maximum number of assert calls in a test method") - .require(NumericConstraints.positive()) - .defaultValue(1) - .build(); - - private static final PropertyDescriptor> EXTRA_ASSERT_METHOD_NAMES = - PropertyFactory.stringProperty("extraAssertMethodNames") - .desc("Extra valid assertion methods names") - .map(Collectors.toSet()) - .emptyDefaultValue() - .build(); - - - public JUnitTestContainsTooManyAssertsRule() { - super(ASTMethodDeclaration.class); - definePropertyDescriptor(MAX_ASSERTS); - definePropertyDescriptor(EXTRA_ASSERT_METHOD_NAMES); - } - - @Override - public Object visit(ASTMethodDeclaration method, Object data) { - ASTBlock body = method.getBody(); - if (body != null && TestFrameworksUtil.isTestMethod(method)) { - Set extraAsserts = getProperty(EXTRA_ASSERT_METHOD_NAMES); - int assertCount = body.descendants(ASTMethodCall.class) - .filter(call -> TestFrameworksUtil.isProbableAssertCall(call) - || extraAsserts.contains(call.getMethodName())) - .count(); - if (assertCount > getProperty(MAX_ASSERTS)) { - asCtx(data).addViolation(method); - } - } - return data; - } } 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 b5bf29134b..ad20f3cca5 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 @@ -4,43 +4,10 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; -import java.util.Set; -import java.util.stream.Collectors; +/** + * @deprecated The rule was renamed {@link UnitTestsShouldIncludeAssertRule} + */ +@Deprecated +public class JUnitTestsShouldIncludeAssertRule extends UnitTestsShouldIncludeAssertRule { -import net.sourceforge.pmd.lang.java.ast.ASTBlock; -import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; -import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; -import net.sourceforge.pmd.properties.PropertyDescriptor; -import net.sourceforge.pmd.properties.PropertyFactory; - -public class JUnitTestsShouldIncludeAssertRule extends AbstractJavaRulechainRule { - - private static final PropertyDescriptor> EXTRA_ASSERT_METHOD_NAMES = - PropertyFactory.stringProperty("extraAssertMethodNames") - .desc("Extra valid assertion methods names") - .map(Collectors.toSet()) - .emptyDefaultValue() - .build(); - - public JUnitTestsShouldIncludeAssertRule() { - super(ASTMethodDeclaration.class); - definePropertyDescriptor(EXTRA_ASSERT_METHOD_NAMES); - } - - @Override - public Object visit(ASTMethodDeclaration method, Object data) { - ASTBlock body = method.getBody(); - Set extraAsserts = getProperty(EXTRA_ASSERT_METHOD_NAMES); - if (body != null - && TestFrameworksUtil.isTestMethod(method) - && !TestFrameworksUtil.isExpectAnnotated(method) - && body.descendants(ASTMethodCall.class) - .none(call -> TestFrameworksUtil.isProbableAssertCall(call) - || extraAsserts.contains(call.getMethodName()))) { - asCtx(data).addViolation(method); - } - return data; - } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestAssertionsShouldIncludeMessageRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestAssertionsShouldIncludeMessageRule.java new file mode 100644 index 0000000000..78be7c765e --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestAssertionsShouldIncludeMessageRule.java @@ -0,0 +1,44 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; +import net.sourceforge.pmd.lang.java.types.InvocationMatcher; +import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher; + +public class UnitTestAssertionsShouldIncludeMessageRule extends AbstractJavaRulechainRule { + + private final CompoundInvocationMatcher checks = + InvocationMatcher.parseAll( + "_#assertEquals(_,_)", + "_#assertTrue(_)", + "_#assertFalse(_)", + "_#assertSame(_,_)", + "_#assertNotSame(_,_)", + "_#assertNull(_)", + "_#assertNotNull(_)", + "_#assertArrayEquals(_,_)", + "_#assertThat(_,_)", + "_#fail()", + "_#assertEquals(float,float,float)", + "_#assertEquals(double,double,double)" + ); + + public UnitTestAssertionsShouldIncludeMessageRule() { + super(ASTMethodCall.class); + } + + @Override + public Object visit(ASTMethodCall node, Object data) { + if (TestFrameworksUtil.isCallOnAssertionContainer(node)) { + if (checks.anyMatch(node)) { + asCtx(data).addViolation(node); + } + } + return null; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestContainsTooManyAssertsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestContainsTooManyAssertsRule.java new file mode 100644 index 0000000000..42168a10b5 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestContainsTooManyAssertsRule.java @@ -0,0 +1,57 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import java.util.Set; +import java.util.stream.Collectors; + +import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; +import net.sourceforge.pmd.properties.NumericConstraints; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; + +public class UnitTestContainsTooManyAssertsRule extends AbstractJavaRulechainRule { + + private static final PropertyDescriptor MAX_ASSERTS = + PropertyFactory.intProperty("maximumAsserts") + .desc("Maximum number of assert calls in a test method") + .require(NumericConstraints.positive()) + .defaultValue(1) + .build(); + + private static final PropertyDescriptor> EXTRA_ASSERT_METHOD_NAMES = + PropertyFactory.stringProperty("extraAssertMethodNames") + .desc("Extra valid assertion methods names") + .map(Collectors.toSet()) + .emptyDefaultValue() + .build(); + + + public UnitTestContainsTooManyAssertsRule() { + super(ASTMethodDeclaration.class); + definePropertyDescriptor(MAX_ASSERTS); + definePropertyDescriptor(EXTRA_ASSERT_METHOD_NAMES); + } + + @Override + public Object visit(ASTMethodDeclaration method, Object data) { + ASTBlock body = method.getBody(); + if (body != null && TestFrameworksUtil.isTestMethod(method)) { + Set extraAsserts = getProperty(EXTRA_ASSERT_METHOD_NAMES); + int assertCount = body.descendants(ASTMethodCall.class) + .filter(call -> TestFrameworksUtil.isProbableAssertCall(call) + || extraAsserts.contains(call.getMethodName())) + .count(); + if (assertCount > getProperty(MAX_ASSERTS)) { + asCtx(data).addViolation(method); + } + } + return data; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestsShouldIncludeAssertRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestsShouldIncludeAssertRule.java new file mode 100644 index 0000000000..867ae52504 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnitTestsShouldIncludeAssertRule.java @@ -0,0 +1,46 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import java.util.Set; +import java.util.stream.Collectors; + +import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; + +public class UnitTestsShouldIncludeAssertRule extends AbstractJavaRulechainRule { + + private static final PropertyDescriptor> EXTRA_ASSERT_METHOD_NAMES = + PropertyFactory.stringProperty("extraAssertMethodNames") + .desc("Extra valid assertion methods names") + .map(Collectors.toSet()) + .emptyDefaultValue() + .build(); + + public UnitTestsShouldIncludeAssertRule() { + super(ASTMethodDeclaration.class); + definePropertyDescriptor(EXTRA_ASSERT_METHOD_NAMES); + } + + @Override + public Object visit(ASTMethodDeclaration method, Object data) { + ASTBlock body = method.getBody(); + Set extraAsserts = getProperty(EXTRA_ASSERT_METHOD_NAMES); + if (body != null + && TestFrameworksUtil.isTestMethod(method) + && !TestFrameworksUtil.isExpectAnnotated(method) + && body.descendants(ASTMethodCall.class) + .none(call -> TestFrameworksUtil.isProbableAssertCall(call) + || extraAsserts.contains(call.getMethodName()))) { + asCtx(data).addViolation(method); + } + return data; + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 4bf4a370bf..2698fbab73 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -672,148 +672,11 @@ public class GoodTest { - - -In JUnit 3, the tearDown method was used to clean up all data entities required in running tests. -JUnit 4 skips the tearDown method and executes all methods annotated with @After after running each test. -JUnit 5 introduced @AfterEach and @AfterAll annotations to execute methods after each test or after all tests in the class, respectively. - - 3 - - - - - - - - - - - + - - -In JUnit 3, the setUp method was used to set up all data entities required in running tests. -JUnit 4 skips the setUp method and executes all methods annotated with @Before before all tests. -JUnit 5 introduced @BeforeEach and @BeforeAll annotations to execute methods before each test or before all tests in the class, respectively. - - 3 - - - - -]]> - - - - - - - - - - -In JUnit 3, the framework executed all methods which started with the word test as a unit test. -In JUnit 4, only methods annotated with the @Test annotation are executed. -In JUnit 5, one of the following annotations should be used for tests: @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. -In TestNG, only methods annotated with the @Test annotation are executed. - - 3 - - - - - - - - - - - - + - - -JUnit assertions should include an informative message - i.e., use the three-argument version of -assertEquals(), not the two-argument version. - - 3 - - - - + - - -Unit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which -it is harder to verify correctness. Consider breaking the test scenario into multiple, shorter test scenarios. -Customize the maximum number of assertions used by this Rule to suit your needs. + -This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods starting with "test". - - 3 - - - - - - - -JUnit tests should include at least one assertion. This makes the tests more robust, and using assert -with messages provide the developer a clearer idea of what the test does. - - 3 - - - - + + + + +Unit assertions should include an informative message - i.e., use the three-argument version of +`assertEquals()`, not the two-argument version. + +This rule supports tests using JUnit (3, 4 and 5) and TestNG. + + 3 + + + + + + + + Unit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which + it is harder to verify correctness. Consider breaking the test scenario into multiple, shorter test scenarios. + Customize the maximum number of assertions used by this Rule to suit your needs. + + This rule checks for JUnit (3, 4 and 5) and TestNG Tests. + + 3 + + + + + + + + This rule detects methods called `tearDown()` that are not properly annotated as a cleanup method. + This is primarily intended to assist in upgrading from JUnit 3, where tear down methods were required to be called `tearDown()`. + To a lesser extent, this may help detect omissions under newer JUnit versions, as long as you are following this convention to name the methods. + + JUnit 4 will only execute methods annotated with `@After` after running each test. + JUnit 5 introduced `@AfterEach` and `@AfterAll` annotations to execute methods after each test or after all tests in the class, respectively. + + 3 + + + + + + + + + + + + + + + This rule detects methods called `setUp()` that are not properly annotated as a setup method. + This is primarily intended to assist in upgrading from JUnit 3, where setup methods were required to be called `setUp()`. + To a lesser extent, this may help detect omissions even under newer JUnit versions, as long as you are following this convention to name the methods. + + JUnit 4 will only execute methods annotated with `@Before` before all tests. + JUnit 5 introduced `@BeforeEach` and `@BeforeAll` annotations to execute methods before each test or before all tests in the class, respectively. + + 3 + + + + + + + + + + + + + + + The rule will detect any test method starting with "test" that is not properly annotated, and will therefore not be run. + + In JUnit 4, only methods annotated with the `@Test` annotation are executed. + In JUnit 5, one of the following annotations should be used for tests: `@Test`, `@RepeatedTest`, `@TestFactory`, `@TestTemplate` or `@ParameterizedTest`. + In TestNG, only methods annotated with the `@Test` annotation are executed. + + 3 + + + + + + + + + + + + + + + + Unit tests should include at least one assertion. This makes the tests more robust, and using assert + with messages provide the developer a clearer idea of what the test does. + + This rule checks for JUnit (3, 4 and 5) and TestNG Tests. + + 3 + + + + + [java] JUnit4TestShouldUseBeforeAnnotation false positive when overriding setUp #1592 0 [java] JUnit4TestShouldUseBeforeAnnotation false positive when overriding setUp #1592 0