diff --git a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected index 2cff4a3eaa62..835e2bd3ee99 100644 --- a/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected +++ b/java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected @@ -5,6 +5,7 @@ ql/java/ql/src/Likely Bugs/Comparison/IncomparableEquals.ql ql/java/ql/src/Likely Bugs/Comparison/InconsistentEqualsHashCode.ql ql/java/ql/src/Likely Bugs/Comparison/MissingInstanceofInEquals.ql ql/java/ql/src/Likely Bugs/Comparison/RefEqBoxed.ql +ql/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql ql/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql ql/java/ql/src/Likely Bugs/Likely Typos/SuspiciousDateFormat.ql ql/java/ql/src/Likely Bugs/Resource Leaks/CloseReader.ql diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll index d0fb6849f422..f229440e4eed 100644 --- a/java/ql/lib/semmle/code/java/UnitTests.qll +++ b/java/ql/lib/semmle/code/java/UnitTests.qll @@ -125,6 +125,57 @@ class JUnitJupiterTestMethod extends Method { } } +/** + * A JUnit 5 test method. + * + * A test method is defined by JUnit as "any instance method + * that is directly annotated or meta-annotated with `@Test`, + * `@RepeatedTest`, `@ParameterizedTest`, `@TestFactory`, or + * `@TestTemplate`." + * + * See https://junit.org/junit5/docs/current/user-guide/#writing-tests-definitions + */ +class JUnit5TestMethod extends Method { + JUnit5TestMethod() { + this instanceof JUnitJupiterTestMethod or + this.getAnAnnotation() + .getType() + .hasQualifiedName("org.junit.jupiter.api", ["RepeatedTest", "TestFactory", "TestTemplate"]) or + this.getAnAnnotation() + .getType() + .hasQualifiedName("org.junit.jupiter.params", "ParameterizedTest") + } +} + +/** + * A JUnit 5 test class. + * + * A test class must contain at least one test method, and + * cannot be abstract. + * + * See https://junit.org/junit5/docs/current/user-guide/#writing-tests-definitions + */ +class JUnit5TestClass extends Class { + JUnit5TestClass() { + this.getAMethod() instanceof JUnit5TestMethod and + not this.isAbstract() + } +} + +/** + * A JUnit inner test class that is non-anonymous, non-local, + * and non-private. + */ +class JUnit5InnerTestClass extends JUnit5TestClass { + JUnit5InnerTestClass() { + // `InnerClass` is a non-static nested class. + this instanceof InnerClass and + not this.isAnonymous() and + not this.isLocal() and + not this.isPrivate() + } +} + /** * A JUnit `@Ignore` annotation. */ diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md new file mode 100644 index 000000000000..f8c604a7026d --- /dev/null +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md @@ -0,0 +1,45 @@ +## Overview + +JUnit tests are grouped in a class, and starting from JUnit 5, users can group the test classes in a larger class so they can share the local environment of the enclosing class. While this helps organize the unit tests and foster code reuse, if an inner test class is not annotated with `@Nested`, the unit tests in it will fail to execute during builds. + +## Recommendation + +If you want the tests defined in an inner class to be recognized by the build plugin and be executed, annotate the class with `@Nested`, imported from `org.junit.jupiter.api`. + +## Example + +```java +import org.junit.jupiter.api.Nested; +import static org.junit.Assert.assertEquals; + +public class IntegerOperationTest { + private int i; // Shared variable among the inner classes. + + @BeforeEach + public void initTest() { i = 0; } + + @Nested + public class AdditionTest { // COMPLIANT: Inner test class annotated with `@Nested`. + @Test + public void addTest1() { + assertEquals(1, i + 1); + } + } + + public class SubtractionTest { // NON_COMPLIANT: Inner test class missing `@Nested`. + @Test + public void addTest1() { + assertEquals(-1, i - 1); + } + } +} +``` + +## Implementation Notes + +This rule is focused on missing `@Nested` annotations on non-static nested (inner) test classes. Static nested test classes should not be annotated with `@Nested`. As a result, the absence of a `@Nested` annotation on such classes is compliant. Identifying incorrect application of a `@Nested` annotation to static nested classes is out of scope for this rule. + +## References + +- JUnit 5 API Documentation: [Annotation Interface Nested](https://junit.org/junit5/docs/current/api/org.junit.jupiter.api/org/junit/jupiter/api/Nested.html). +- JUnit 5 User Guide: [Nested Tests](https://junit.org/junit5/docs/current/user-guide/#writing-tests-nested). diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql new file mode 100644 index 000000000000..8072a68c61db --- /dev/null +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -0,0 +1,22 @@ +/** + * @id java/junit5-missing-nested-annotation + * @previous-id java/junit5-non-static-inner-class-missing-nested-annotation + * @name Missing `@Nested` annotation on JUnit 5 inner test class + * @description A JUnit 5 inner test class that is missing a `@Nested` annotation will be + * excluded from execution and may indicate a mistake from the + * programmer. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags quality + * reliability + * correctness + * testability + * frameworks/junit + */ + +import java + +from JUnit5InnerTestClass testClass +where not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") +select testClass, "This JUnit 5 inner test class lacks a '@Nested' annotation." diff --git a/java/ql/src/codeql-suites/java-code-quality.qls b/java/ql/src/codeql-suites/java-code-quality.qls index 2eafe785532e..ece2d170dd25 100644 --- a/java/ql/src/codeql-suites/java-code-quality.qls +++ b/java/ql/src/codeql-suites/java-code-quality.qls @@ -6,10 +6,11 @@ - java/inconsistent-equals-and-hashcode - java/input-resource-leak - java/integer-multiplication-cast-to-long + - java/junit5-missing-nested-annotation - java/output-resource-leak - java/reference-equality-of-boxed-types - java/string-replace-all-with-non-regex - java/suspicious-date-format - java/type-variable-hides-type - java/unchecked-cast-in-equals - - java/unused-container + - java/unused-container \ No newline at end of file diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java new file mode 100644 index 000000000000..0050d56dadee --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -0,0 +1,117 @@ +import java.util.Collection; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +public class AnnotationTest { + @Nested + public class Test1 { // COMPLIANT: Inner test class has `@Nested` + @Test + public void test() { + } + } + + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2_Test { // $ Alert + @Test + public void test() { + } + } + + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2_RepeatedTest { // $ Alert + @RepeatedTest(2) + public void test() { + } + } + + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2_ParameterizedTest { // $ Alert + @ParameterizedTest + @ValueSource(strings = { "" }) + public void test(String s) { + } + } + + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2_TestFactory { // $ Alert + @TestFactory + Collection test() { + return null; + } + } + + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2_TestTemplate { // $ Alert + @TestTemplate + public void test() { + } + } + + public class Test3 { // COMPLIANT: Since it is empty, it is not a test class + } + + public class Test4 { // COMPLIANT: Since no methods have `@Test`, it is not a test class + public void f() { + } + + public void g() { + } + + public void h() { + } + } + + public static class Test5 { // COMPLIANT: Static nested test classes don't need `@Nested` + @Test + public void test() { + } + } + + // COMPLIANT: Invalid to use `@Nested` on a static class, but + // this matter is out of scope (see QHelp Implementation Notes) + @Nested + public static class Test6 { + @Test + public void test() { + } + } + + public abstract class Test7 { // COMPLIANT: Abstract nested test classes don't need `@Nested` + @Test + public void test() { + } + } + + interface Test8 { + } + + public void f() { + // COMPLIANT: anonymous classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + new Test8() { + @Test + public void test() { + } + }; + // COMPLIANT: local classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + class Test9 { + @Test + void test() { + } + } + } + + // COMPLIANT: private classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + private class Test10 { + @Test + public void test() { + } + } +} diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected new file mode 100644 index 000000000000..866afc0926a5 --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected @@ -0,0 +1,5 @@ +| AnnotationTest.java:19:16:19:25 | Test2_Test | This JUnit 5 inner test class lacks a '@Nested' annotation. | +| AnnotationTest.java:26:16:26:33 | Test2_RepeatedTest | This JUnit 5 inner test class lacks a '@Nested' annotation. | +| AnnotationTest.java:33:16:33:38 | Test2_ParameterizedTest | This JUnit 5 inner test class lacks a '@Nested' annotation. | +| AnnotationTest.java:41:16:41:32 | Test2_TestFactory | This JUnit 5 inner test class lacks a '@Nested' annotation. | +| AnnotationTest.java:49:16:49:33 | Test2_TestTemplate | This JUnit 5 inner test class lacks a '@Nested' annotation. | diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.qlref b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.qlref new file mode 100644 index 000000000000..1d4c5febd14d --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.qlref @@ -0,0 +1,2 @@ +query: Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/options b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/options new file mode 100644 index 000000000000..8ad0239c5f0f --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/junit-jupiter-api-5.2.0 diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/Nested.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/Nested.java new file mode 100644 index 000000000000..c1ca1fe9163f --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/Nested.java @@ -0,0 +1,4 @@ +package org.junit.jupiter.api; + +public @interface Nested { +} diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/RepeatedTest.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/RepeatedTest.java new file mode 100644 index 000000000000..1ea7e576ab2b --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/RepeatedTest.java @@ -0,0 +1,25 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.api; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@TestTemplate +public @interface RepeatedTest { + int value(); +} diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestFactory.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestFactory.java new file mode 100644 index 000000000000..4da3585c8229 --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestFactory.java @@ -0,0 +1,23 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.api; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface TestFactory { +} diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestTemplate.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestTemplate.java new file mode 100644 index 000000000000..5ff7ca49131f --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestTemplate.java @@ -0,0 +1,23 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.api; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface TestTemplate { +} diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/ParameterizedTest.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/ParameterizedTest.java new file mode 100644 index 000000000000..3f8d0c4ba328 --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/ParameterizedTest.java @@ -0,0 +1,26 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.params; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.junit.jupiter.api.TestTemplate; + +@Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@TestTemplate +public @interface ParameterizedTest { +} diff --git a/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/provider/ValueSource.java b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/provider/ValueSource.java new file mode 100644 index 000000000000..fd698ac98a07 --- /dev/null +++ b/java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/provider/ValueSource.java @@ -0,0 +1,24 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.params.provider; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ ElementType.ANNOTATION_TYPE, ElementType.METHOD, ElementType.TYPE }) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface ValueSource { + String[] strings() default {}; +}