From ccbe77eb099b9200ae8100f2bc89c6020cb6214f Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 11:08:10 -0400 Subject: [PATCH 01/18] Java: move original files --- ...StaticInnerClassMissingNestedAnnotation.md | 64 +++++++++++++++++++ ...StaticInnerClassMissingNestedAnnotation.ql | 30 +++++++++ .../Frameworks/JUnit/AnnotationTest.java | 44 +++++++++++++ ...InnerClassMissingNestedAnnotation.expected | 1 + ...ticInnerClassMissingNestedAnnotation.qlref | 1 + .../Likely Bugs/Frameworks/JUnit/options | 1 + .../org/junit/jupiter/api/Nested.java | 4 ++ 7 files changed, 145 insertions(+) create mode 100644 java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md create mode 100644 java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/options create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/Nested.java diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md new file mode 100644 index 000000000000..78719b295564 --- /dev/null +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md @@ -0,0 +1,64 @@ +# J-T-004: Non-static inner class defined in a JUnit 5 is missing a `@Nested` annotation + +A non-static inner class defined in a JUnit 5 test missing a `@Nested` annotation will be excluded from execution and it may indicate a misunderstanding from the programmer. + +## Overview + +JUnit tests are grouped in a class, and starting from JUnit 5 users can group the test classes in a bigger class so they can share the local environment of the enclosing class. While this helps to organize the unit tests and foster code reuse, if the inner class is not annotated with `@Nested`, the unit tests in it will fail to execute during builds. + +Note that static classes, whether static or not, behave as independent sets of unit tests. Thus, inner static classes do not share the information provided by the outer class with other inner classes. Thus, this rule only applies to non-static JUnit 5 inner classes. + +## Recommendation + +If you want the tests defined in an inner class to be recognized by the build plugin and be executed, annotate 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() { // Test of an inner class, implying `AdditionTest` is a test class. + assertEquals(1, i+1); + } + } + + public class SubtractionTest { // NON_COMPLIANT: Inner test class missing `@Nested`. + @Test + public void addTest1() { // Test of an inner class, implying `SubtractionTest` is a test class. + assertEquals(-1, i-1); + } + } + + static public class MultiplicationTest { // COMPLIANT: static test class should not be annotated as `@Nested`. + ... + } +} +``` + +## Implementation Notes + +The `@Nested` annotation does not apply to inner static classes, since the meaning of the annotation is to mark a class as "a *non-static* inner class containing `@Test` methods to be picked up by a build system". Therefore, this rule does not aim to target static inner test classes with a `@Nested` annotation, nor does it try to enforce such correct usage of `@Nested`. Therefore, any code that resembles below is not non-compliant to this rule. + +``` java +@Nested +public static class Test6 { // COMPLIANT: Although invalid, this matter is out of the scope + @Test + public void test() { + } +} +``` + +## References + +- JUnit: [Documentation on `@Nested`](https://junit.org/junit5/docs/current/api/org.junit.jupiter.api/org/junit/jupiter/api/Nested.html). +- Baeldung: [JUnit 5 @Nested Test Classes](https://www.baeldung.com/junit-5-nested-test-classes). diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql new file mode 100644 index 000000000000..cb1988dca052 --- /dev/null +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql @@ -0,0 +1,30 @@ +/** + * @id java/junit5-non-static-inner-class-missing-nested-annotation + * @name J-T-004: Non-static inner class defined in a JUnit5 test is missing a `@Nested` annotation + * @description A non-static inner class defined in a JUnit5 test missing a `@Nested` annotation + * will be excluded from execution and it may indicate a misunderstanding from the + * programmer. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags maintainability + * correctness + */ + +import java + +class JUnit5TestClass extends Class { + JUnit5TestClass() { + this.getAMethod().getAnAnnotation().getType().hasQualifiedName("org.junit.jupiter.api", "Test") + } +} + +from JUnit5TestClass testClass // `TestClass` by definition should have at least one @Test method. +where + not testClass.isStatic() and + testClass instanceof InnerClass and // `InnerClass` is by definition a non-static nested class. + not exists(Annotation annotation | + annotation.getType().hasQualifiedName("org.junit.jupiter.api", "Nested") and + annotation.getAnnotatedElement() = testClass + ) +select testClass, "This JUnit5 inner test class lacks a @Nested annotation." 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..91dd1f5b597a --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -0,0 +1,44 @@ +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Nested; + +public class AnnotationTest { + @Nested + public class Test1 { // COMPLIANT: Inner test class has `@Nested` + @Test + public void test() { + } + } + + public class Test2 { // NON_COMPLIANT: Inner test class is missing a `@Nested` + @Test + 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 inner test classes don't need `@Nested` + @Test + public void test() { + } + } + + @Nested + public static class Test6 { // COMPLIANT: Although invalid, this matter is out of the scope (see Implementation Details) + @Test + public void test() { + } + } +} diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected new file mode 100644 index 000000000000..0964e894cbd2 --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected @@ -0,0 +1 @@ +| AnnotationTest.java:12:16:12:20 | Test2 | This JUnit5 inner test class lacks a @Nested annotation. | diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref new file mode 100644 index 000000000000..4b0732a04c5b --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref @@ -0,0 +1 @@ +Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.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 { +} From f17e7266cfeb2bb0fe89ed96de5b5353dbab1922 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 15:22:59 -0400 Subject: [PATCH 02/18] Java: refactor QL --- java/ql/lib/semmle/code/java/UnitTests.qll | 8 ++++++ ...StaticInnerClassMissingNestedAnnotation.ql | 26 +++++++------------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll index 38f37fa4ff01..af1cd88f4f16 100644 --- a/java/ql/lib/semmle/code/java/UnitTests.qll +++ b/java/ql/lib/semmle/code/java/UnitTests.qll @@ -112,6 +112,14 @@ class JUnitJupiterTestMethod extends Method { } } +/** + * A JUnit test class that contains at least one method annotated with + * `org.junit.jupiter.api.Test`. + */ +class JUnit5TestClass extends Class { + JUnit5TestClass() { this.getAMethod() instanceof JUnitJupiterTestMethod } +} + /** * A JUnit `@Ignore` annotation. */ diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql index cb1988dca052..4b521567784a 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql @@ -1,30 +1,22 @@ /** * @id java/junit5-non-static-inner-class-missing-nested-annotation - * @name J-T-004: Non-static inner class defined in a JUnit5 test is missing a `@Nested` annotation - * @description A non-static inner class defined in a JUnit5 test missing a `@Nested` annotation + * @name Non-static inner class defined in a JUnit 5 test is missing a `@Nested` annotation + * @description A non-static inner class defined in a JUnit 5 test missing a `@Nested` annotation * will be excluded from execution and it may indicate a misunderstanding from the * programmer. * @kind problem * @precision very-high * @problem.severity warning - * @tags maintainability + * @tags quality + * maintainability * correctness */ import java -class JUnit5TestClass extends Class { - JUnit5TestClass() { - this.getAMethod().getAnAnnotation().getType().hasQualifiedName("org.junit.jupiter.api", "Test") - } -} - -from JUnit5TestClass testClass // `TestClass` by definition should have at least one @Test method. +from JUnit5TestClass testClass where - not testClass.isStatic() and - testClass instanceof InnerClass and // `InnerClass` is by definition a non-static nested class. - not exists(Annotation annotation | - annotation.getType().hasQualifiedName("org.junit.jupiter.api", "Nested") and - annotation.getAnnotatedElement() = testClass - ) -select testClass, "This JUnit5 inner test class lacks a @Nested annotation." + // `InnerClass` is a non-static, nested class. + testClass instanceof InnerClass and + not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") +select testClass, "This JUnit5 inner test class lacks a '@Nested' annotation." From b08c8d020de9429c94b21f98ca4fda55b4614526 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 15:33:15 -0400 Subject: [PATCH 03/18] Java: tests to inline expectations --- .../Likely Bugs/Frameworks/JUnit/AnnotationTest.java | 7 +++++-- ...nit5NonStaticInnerClassMissingNestedAnnotation.expected | 2 +- .../JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) 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 index 91dd1f5b597a..4aeceb51f9e6 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -9,7 +9,8 @@ public void test() { } } - public class Test2 { // NON_COMPLIANT: Inner test class is missing a `@Nested` + // NON_COMPLIANT: Inner test class is missing `@Nested` + public class Test2 { // $ Alert @Test public void test() { } @@ -35,8 +36,10 @@ 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 { // COMPLIANT: Although invalid, this matter is out of the scope (see Implementation Details) + public static class Test6 { @Test public void test() { } diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected index 0964e894cbd2..5133a3223319 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected @@ -1 +1 @@ -| AnnotationTest.java:12:16:12:20 | Test2 | This JUnit5 inner test class lacks a @Nested annotation. | +| AnnotationTest.java:13:16:13:20 | Test2 | This JUnit5 inner test class lacks a '@Nested' annotation. | diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref index 4b0732a04c5b..3a819b36f79c 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref @@ -1 +1,2 @@ -Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql +query: Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql From ed57bc7858ab7436145c504e78b1dfa3bc98ddce Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 16:11:11 -0400 Subject: [PATCH 04/18] Java: exclude abstract classes --- ...5NonStaticInnerClassMissingNestedAnnotation.md | 11 +++++++++-- ...5NonStaticInnerClassMissingNestedAnnotation.ql | 4 +++- .../Frameworks/JUnit/AnnotationTest.java | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md index 78719b295564..7f336fd7c105 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md @@ -47,11 +47,18 @@ public class IntegerOperationTest { ## Implementation Notes -The `@Nested` annotation does not apply to inner static classes, since the meaning of the annotation is to mark a class as "a *non-static* inner class containing `@Test` methods to be picked up by a build system". Therefore, this rule does not aim to target static inner test classes with a `@Nested` annotation, nor does it try to enforce such correct usage of `@Nested`. Therefore, any code that resembles below is not non-compliant to this rule. +The `@Nested` annotation does not apply to inner static classes, since the meaning of the annotation is to mark a class as "a *non-static* inner class containing `@Test` methods to be picked up by a build system". It also does not apply to inner abstract classes since there is no use case for an `@Nested` annotation on an abstract class. Therefore, this rule does not aim to target static or abstract inner test classes with a `@Nested` annotation, nor does it try to enforce such correct usage of `@Nested`. Therefore, any code that resembles the below is not non-compliant to this rule. ``` java @Nested -public static class Test6 { // COMPLIANT: Although invalid, this matter is out of the scope +public static class TestStatic { // COMPLIANT: Although invalid, this matter is out of the scope + @Test + public void test() { + } +} + +@Nested +public abstract class TestAbstract { // COMPLIANT: Although invalid, this matter is out of the scope @Test public void test() { } diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql index 4b521567784a..1c5b4234e758 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql @@ -18,5 +18,7 @@ from JUnit5TestClass testClass where // `InnerClass` is a non-static, nested class. testClass instanceof InnerClass and - not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") + not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") and + // An abstract class should not have a `@Nested` annotation + not testClass.isAbstract() select testClass, "This JUnit5 inner test class lacks a '@Nested' annotation." 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 index 4aeceb51f9e6..6c095e9e4ecd 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -44,4 +44,19 @@ public static class Test6 { public void test() { } } + + public abstract class Test7 { // COMPLIANT: Abstract inner test classes don't need `@Nested` + @Test + public void test() { + } + } + + // COMPLIANT: Invalid to use `@Nested` on an abstract class, but + // this matter is out of scope (see QHelp Implementation Notes) + @Nested + public abstract class Test8 { + @Test + public void test() { + } + } } From 640096c822cae7994dd53eaf509d540fbee7ffad Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 16:15:56 -0400 Subject: [PATCH 05/18] Java: change note --- .../2025-03-21-junit-missing-nested-annotation.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md diff --git a/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md b/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md new file mode 100644 index 000000000000..b014655b42c5 --- /dev/null +++ b/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* Added a new quality query, `java/junit5-non-static-inner-class-missing-nested-annotation`, to detect missing `@Nested` annotations on non-static, inner JUnit 5 test classes. From 3e13f0ed41ec85e20d96e86e7fd7f1ff7c71b1c3 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 21 Mar 2025 22:43:35 -0400 Subject: [PATCH 06/18] Java: remove redundant 'non-static' wording and update qhelp --- .../JUnit/JUnit5MissingNestedAnnotation.md | 45 ++++++++++++ ...on.ql => JUnit5MissingNestedAnnotation.ql} | 11 +-- ...StaticInnerClassMissingNestedAnnotation.md | 71 ------------------- ...5-03-21-junit-missing-nested-annotation.md | 2 +- .../Frameworks/JUnit/AnnotationTest.java | 4 +- .../JUnit5MissingNestedAnnotation.expected | 1 + .../JUnit/JUnit5MissingNestedAnnotation.qlref | 2 + ...InnerClassMissingNestedAnnotation.expected | 1 - ...ticInnerClassMissingNestedAnnotation.qlref | 2 - 9 files changed, 57 insertions(+), 82 deletions(-) create mode 100644 java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md rename java/ql/src/Likely Bugs/Frameworks/JUnit/{JUnit5NonStaticInnerClassMissingNestedAnnotation.ql => JUnit5MissingNestedAnnotation.ql} (50%) delete mode 100644 java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected create mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.qlref delete mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected delete mode 100644 java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref 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..2c4fbd7d61ee --- /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 bigger class so they can share the local environment of the enclosing class. While this helps to 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 and abstract 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 and abstract 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/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql similarity index 50% rename from java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql rename to java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index 1c5b4234e758..a8cf5d14e3c0 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -1,8 +1,8 @@ /** - * @id java/junit5-non-static-inner-class-missing-nested-annotation - * @name Non-static inner class defined in a JUnit 5 test is missing a `@Nested` annotation - * @description A non-static inner class defined in a JUnit 5 test missing a `@Nested` annotation - * will be excluded from execution and it may indicate a misunderstanding from the + * @id java/junit5-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 it may indicate a misunderstanding from the * programmer. * @kind problem * @precision very-high @@ -10,6 +10,7 @@ * @tags quality * maintainability * correctness + * previous-id:java/junit5-non-static-inner-class-missing-nested-annotation */ import java @@ -21,4 +22,4 @@ where not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") and // An abstract class should not have a `@Nested` annotation not testClass.isAbstract() -select testClass, "This JUnit5 inner test class lacks a '@Nested' annotation." +select testClass, "This JUnit 5 inner test class lacks a '@Nested' annotation." diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md deleted file mode 100644 index 7f336fd7c105..000000000000 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.md +++ /dev/null @@ -1,71 +0,0 @@ -# J-T-004: Non-static inner class defined in a JUnit 5 is missing a `@Nested` annotation - -A non-static inner class defined in a JUnit 5 test missing a `@Nested` annotation will be excluded from execution and it may indicate a misunderstanding from the programmer. - -## Overview - -JUnit tests are grouped in a class, and starting from JUnit 5 users can group the test classes in a bigger class so they can share the local environment of the enclosing class. While this helps to organize the unit tests and foster code reuse, if the inner class is not annotated with `@Nested`, the unit tests in it will fail to execute during builds. - -Note that static classes, whether static or not, behave as independent sets of unit tests. Thus, inner static classes do not share the information provided by the outer class with other inner classes. Thus, this rule only applies to non-static JUnit 5 inner classes. - -## Recommendation - -If you want the tests defined in an inner class to be recognized by the build plugin and be executed, annotate 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() { // Test of an inner class, implying `AdditionTest` is a test class. - assertEquals(1, i+1); - } - } - - public class SubtractionTest { // NON_COMPLIANT: Inner test class missing `@Nested`. - @Test - public void addTest1() { // Test of an inner class, implying `SubtractionTest` is a test class. - assertEquals(-1, i-1); - } - } - - static public class MultiplicationTest { // COMPLIANT: static test class should not be annotated as `@Nested`. - ... - } -} -``` - -## Implementation Notes - -The `@Nested` annotation does not apply to inner static classes, since the meaning of the annotation is to mark a class as "a *non-static* inner class containing `@Test` methods to be picked up by a build system". It also does not apply to inner abstract classes since there is no use case for an `@Nested` annotation on an abstract class. Therefore, this rule does not aim to target static or abstract inner test classes with a `@Nested` annotation, nor does it try to enforce such correct usage of `@Nested`. Therefore, any code that resembles the below is not non-compliant to this rule. - -``` java -@Nested -public static class TestStatic { // COMPLIANT: Although invalid, this matter is out of the scope - @Test - public void test() { - } -} - -@Nested -public abstract class TestAbstract { // COMPLIANT: Although invalid, this matter is out of the scope - @Test - public void test() { - } -} -``` - -## References - -- JUnit: [Documentation on `@Nested`](https://junit.org/junit5/docs/current/api/org.junit.jupiter.api/org/junit/jupiter/api/Nested.html). -- Baeldung: [JUnit 5 @Nested Test Classes](https://www.baeldung.com/junit-5-nested-test-classes). diff --git a/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md b/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md index b014655b42c5..c663ddc04f6d 100644 --- a/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md +++ b/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md @@ -1,4 +1,4 @@ --- category: newQuery --- -* Added a new quality query, `java/junit5-non-static-inner-class-missing-nested-annotation`, to detect missing `@Nested` annotations on non-static, inner JUnit 5 test classes. +* Added a new quality query, `java/junit5-missing-nested-annotation`, to detect missing `@Nested` annotations on JUnit 5 inner test classes. 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 index 6c095e9e4ecd..12b65294045e 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -30,7 +30,7 @@ public void h() { } } - public static class Test5 { // COMPLIANT: Static inner test classes don't need `@Nested` + public static class Test5 { // COMPLIANT: Static nested test classes don't need `@Nested` @Test public void test() { } @@ -45,7 +45,7 @@ public void test() { } } - public abstract class Test7 { // COMPLIANT: Abstract inner test classes don't need `@Nested` + public abstract class Test7 { // COMPLIANT: Abstract nested test classes don't need `@Nested` @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..bb7f1b036f3b --- /dev/null +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected @@ -0,0 +1 @@ +| AnnotationTest.java:13:16:13:20 | Test2 | 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/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected deleted file mode 100644 index 5133a3223319..000000000000 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.expected +++ /dev/null @@ -1 +0,0 @@ -| AnnotationTest.java:13:16:13:20 | Test2 | This JUnit5 inner test class lacks a '@Nested' annotation. | diff --git a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref deleted file mode 100644 index 3a819b36f79c..000000000000 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.qlref +++ /dev/null @@ -1,2 +0,0 @@ -query: Likely Bugs/Frameworks/JUnit/JUnit5NonStaticInnerClassMissingNestedAnnotation.ql -postprocess: utils/test/InlineExpectationsTestQuery.ql From 4d7bed6181e5e35ed27bfa280bafaf657c18f673 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 23 Mar 2025 14:31:11 -0400 Subject: [PATCH 07/18] Java: exclude anonymous, local, and private classes --- java/ql/lib/semmle/code/java/UnitTests.qll | 14 ++++++++++ .../JUnit/JUnit5MissingNestedAnnotation.ql | 4 +-- .../Frameworks/JUnit/AnnotationTest.java | 28 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll index af1cd88f4f16..810732356aff 100644 --- a/java/ql/lib/semmle/code/java/UnitTests.qll +++ b/java/ql/lib/semmle/code/java/UnitTests.qll @@ -120,6 +120,20 @@ class JUnit5TestClass extends Class { JUnit5TestClass() { this.getAMethod() instanceof JUnitJupiterTestMethod } } +/** + * 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.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index a8cf5d14e3c0..2f69cea0c7a1 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -15,10 +15,8 @@ import java -from JUnit5TestClass testClass +from JUnit5InnerTestClass testClass where - // `InnerClass` is a non-static, nested class. - testClass instanceof InnerClass and not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") and // An abstract class should not have a `@Nested` annotation not testClass.isAbstract() 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 index 12b65294045e..ad229192fd4d 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -59,4 +59,32 @@ public abstract class Test8 { public void test() { } } + + interface Test9 { + } + + public void f() { + // COMPLIANT: anonymous classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + new Test9() { + @Test + public void test() { + } + }; + // COMPLIANT: local classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + class Test10 { + @Test + void test() { + } + } + } + + // COMPLIANT: private classes are not considered as inner test + // classes by JUnit and therefore don't need `@Nested` + private class Test11 { + @Test + public void test() { + } + } } From 35b647839c45f2b81cfa787243272d070d5682c0 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 23 Mar 2025 16:36:08 -0400 Subject: [PATCH 08/18] Java: include RepeatedTest, ParameterizedTest, TestFactory, and TestTemplate when identifying JUnit 5 test methods --- java/ql/lib/semmle/code/java/UnitTests.qll | 31 +++++++++++++-- .../JUnit/JUnit5MissingNestedAnnotation.ql | 5 +-- .../Frameworks/JUnit/AnnotationTest.java | 38 ++++++++++++++++++- .../JUnit5MissingNestedAnnotation.expected | 6 ++- .../org/junit/jupiter/api/RepeatedTest.java | 25 ++++++++++++ .../org/junit/jupiter/api/TestFactory.java | 23 +++++++++++ .../org/junit/jupiter/api/TestTemplate.java | 23 +++++++++++ .../jupiter/params/ParameterizedTest.java | 26 +++++++++++++ .../jupiter/params/provider/ValueSource.java | 24 ++++++++++++ 9 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/RepeatedTest.java create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestFactory.java create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/api/TestTemplate.java create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/ParameterizedTest.java create mode 100644 java/ql/test/stubs/junit-jupiter-api-5.2.0/org/junit/jupiter/params/provider/ValueSource.java diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll index 810732356aff..84a2ce4d02b2 100644 --- a/java/ql/lib/semmle/code/java/UnitTests.qll +++ b/java/ql/lib/semmle/code/java/UnitTests.qll @@ -113,11 +113,36 @@ class JUnitJupiterTestMethod extends Method { } /** - * A JUnit test class that contains at least one method annotated with - * `org.junit.jupiter.api.Test`. + * 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 JUnitJupiterTestMethod } + JUnit5TestClass() { + this.getAMethod() instanceof JUnit5TestMethod and + not this.isAbstract() + } } /** diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index 2f69cea0c7a1..f60ed0d4e04d 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -16,8 +16,5 @@ import java from JUnit5InnerTestClass testClass -where - not testClass.hasAnnotation("org.junit.jupiter.api", "Nested") and - // An abstract class should not have a `@Nested` annotation - not testClass.isAbstract() +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/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java index ad229192fd4d..4ea60f4404c4 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -1,5 +1,11 @@ +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 @@ -10,12 +16,42 @@ public void test() { } // NON_COMPLIANT: Inner test class is missing `@Nested` - public class Test2 { // $ Alert + 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 } 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 index bb7f1b036f3b..866afc0926a5 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.expected @@ -1 +1,5 @@ -| AnnotationTest.java:13:16:13:20 | Test2 | This JUnit 5 inner test class lacks a '@Nested' annotation. | +| 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/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 {}; +} From 37092f4411cc6161c0a2e38ee34fbcdc35229493 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 23 Mar 2025 16:39:22 -0400 Subject: [PATCH 09/18] Java: add 'testability' and 'frameworks/junit' tags --- .../Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index f60ed0d4e04d..7b7786309cf0 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -10,6 +10,8 @@ * @tags quality * maintainability * correctness + * testability + * frameworks/junit * previous-id:java/junit5-non-static-inner-class-missing-nested-annotation */ From dca4c58b29639f75519c971aa3eae8a08c30d0df Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 23 Mar 2025 16:43:44 -0400 Subject: [PATCH 10/18] Java: add to ccr/quality suite --- java/ql/src/codeql-suites/java-ccr.qls | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/ql/src/codeql-suites/java-ccr.qls b/java/ql/src/codeql-suites/java-ccr.qls index ac1f52624c4f..0ee1e8516e6d 100644 --- a/java/ql/src/codeql-suites/java-ccr.qls +++ b/java/ql/src/codeql-suites/java-ccr.qls @@ -11,4 +11,5 @@ - java/unused-container - java/input-resource-leak - java/output-resource-leak - - java/type-variable-hides-type \ No newline at end of file + - java/type-variable-hides-type + - java/junit5-missing-nested-annotation From 0f002624d60495fc4317258b650e05a6c9bab284 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Sun, 23 Mar 2025 18:30:39 -0400 Subject: [PATCH 11/18] Java: remove mention of abstract classes from qhelp --- .../JUnit/JUnit5MissingNestedAnnotation.md | 2 +- .../Frameworks/JUnit/AnnotationTest.java | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md index 2c4fbd7d61ee..652f35e85cf8 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md @@ -37,7 +37,7 @@ public class IntegerOperationTest { ## Implementation Notes -This rule is focused on missing `@Nested` annotations on non-static nested (inner) test classes. Static nested test classes and abstract 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 and abstract classes is out of scope for this rule. +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 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 index 4ea60f4404c4..0050d56dadee 100644 --- a/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java +++ b/java/ql/test/query-tests/Likely Bugs/Frameworks/JUnit/AnnotationTest.java @@ -87,29 +87,20 @@ public void test() { } } - // COMPLIANT: Invalid to use `@Nested` on an abstract class, but - // this matter is out of scope (see QHelp Implementation Notes) - @Nested - public abstract class Test8 { - @Test - public void test() { - } - } - - interface Test9 { + interface Test8 { } public void f() { // COMPLIANT: anonymous classes are not considered as inner test // classes by JUnit and therefore don't need `@Nested` - new Test9() { + 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 Test10 { + class Test9 { @Test void test() { } @@ -118,7 +109,7 @@ void test() { // COMPLIANT: private classes are not considered as inner test // classes by JUnit and therefore don't need `@Nested` - private class Test11 { + private class Test10 { @Test public void test() { } From b9bf192c09c71664b35751a31d65ec44b87aeae8 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 24 Mar 2025 14:37:05 -0400 Subject: [PATCH 12/18] Java: previous-id property instead of tag, see #19097 --- .../Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index 7b7786309cf0..db67a2c4843e 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -1,5 +1,6 @@ /** * @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 it may indicate a misunderstanding from the @@ -12,7 +13,6 @@ * correctness * testability * frameworks/junit - * previous-id:java/junit5-non-static-inner-class-missing-nested-annotation */ import java From e169c21f8bef540103c1140f24c72e7d9e45a8ff Mon Sep 17 00:00:00 2001 From: Jami <57204504+jcogs33@users.noreply.github.com> Date: Tue, 25 Mar 2025 07:19:39 -0400 Subject: [PATCH 13/18] Apply suggestions from docs review Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com> --- .../Frameworks/JUnit/JUnit5MissingNestedAnnotation.md | 2 +- .../Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md index 652f35e85cf8..f8c604a7026d 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.md @@ -1,6 +1,6 @@ ## Overview -JUnit tests are grouped in a class, and starting from JUnit 5 users can group the test classes in a bigger class so they can share the local environment of the enclosing class. While this helps to 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. +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 diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index db67a2c4843e..3540ad8d9427 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -3,7 +3,7 @@ * @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 it may indicate a misunderstanding from the + * excluded from execution and may indicate a mistake from the * programmer. * @kind problem * @precision very-high From 92cdddf6047c12d773845a2db35186bbce87937e Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 27 Mar 2025 21:29:20 -0400 Subject: [PATCH 14/18] Java: resolve filename conflict --- java/ql/src/codeql-suites/{java-ccr.qls => java-code-quality.qls} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename java/ql/src/codeql-suites/{java-ccr.qls => java-code-quality.qls} (100%) diff --git a/java/ql/src/codeql-suites/java-ccr.qls b/java/ql/src/codeql-suites/java-code-quality.qls similarity index 100% rename from java/ql/src/codeql-suites/java-ccr.qls rename to java/ql/src/codeql-suites/java-code-quality.qls From faeb7ab8909eefb087cdc1cf235506c875bbf451 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Tue, 1 Apr 2025 14:54:46 -0400 Subject: [PATCH 15/18] Java: add blank lines to qldocs --- java/ql/lib/semmle/code/java/UnitTests.qll | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll index 25452d0fd514..f229440e4eed 100644 --- a/java/ql/lib/semmle/code/java/UnitTests.qll +++ b/java/ql/lib/semmle/code/java/UnitTests.qll @@ -127,10 +127,12 @@ 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 { @@ -147,8 +149,10 @@ class JUnit5TestMethod extends Method { /** * 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 { From 6ade97892f3a43dddd60b3fe4402bba4c34d9d26 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 2 Apr 2025 19:06:02 -0400 Subject: [PATCH 16/18] Java: update maintainability tag to reliability instead --- .../Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql index 3540ad8d9427..8072a68c61db 100644 --- a/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql +++ b/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql @@ -9,7 +9,7 @@ * @precision very-high * @problem.severity warning * @tags quality - * maintainability + * reliability * correctness * testability * frameworks/junit From 77eeab33a6a985038b0b6c64ad90d93ca5a86bb5 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 4 Apr 2025 13:57:34 -0400 Subject: [PATCH 17/18] Java: remove change note --- .../2025-03-21-junit-missing-nested-annotation.md | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md diff --git a/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md b/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md deleted file mode 100644 index c663ddc04f6d..000000000000 --- a/java/ql/src/change-notes/2025-03-21-junit-missing-nested-annotation.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -category: newQuery ---- -* Added a new quality query, `java/junit5-missing-nested-annotation`, to detect missing `@Nested` annotations on JUnit 5 inner test classes. From 07a694e804af154191de208820a9d41a70ec66cf Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 21 Apr 2025 09:52:52 -0400 Subject: [PATCH 18/18] Java: add new query to java-code-quality.qls.expected --- .../java/query-suite/java-code-quality.qls.expected | 1 + 1 file changed, 1 insertion(+) 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