From 32fc4b8faf690a2a2082ee5133fde6d51b14cd06 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Mon, 23 Dec 2024 16:36:06 +0100 Subject: [PATCH 1/9] Fix FP on S2187 in Cucumber tests with JUnit5. --- java-checks-test-sources/default/pom.xml | 17 +++++++++++ .../NoTestInTestClassCheckCucumberTest.java | 23 ++++++++++++++ .../checks/tests/NoTestInTestClassCheck.java | 30 ++++++++++++++++++- .../tests/NoTestInTestClassCheckTest.java | 18 +++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml index 4f5c42669c..d84d6e2d7c 100644 --- a/java-checks-test-sources/default/pom.xml +++ b/java-checks-test-sources/default/pom.xml @@ -998,6 +998,23 @@ jspecify 1.0.0 + + io.cucumber + cucumber-java + 7.20.1 + provided + + + io.cucumber + cucumber-junit-platform-engine + 7.20.1 + provided + + + org.junit.platform + junit-platform-suite + provided + diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java new file mode 100644 index 0000000000..c20c90ad3b --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java @@ -0,0 +1,23 @@ +package checks.tests; + +import static io.cucumber.junit.platform.engine.Constants.*; + +import org.junit.platform.suite.api.*; + +@Suite +@IncludeEngines("cucumber") +@SelectClasspathResource("com.project.class.path") +@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") +class NoTestInTestClassCheckCucumberStandardTest {} + +@Suite +@org.junit.platform.suite.api.IncludeEngines("cucumber") +@SelectClasspathResource("com.project.class.path") +@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") +class NoTestInTestClassCheckCucumberFullyQualifiedTest {} + +@Suite +@IncludeEngines("bellpepper") +@SelectClasspathResource("com.project.class.path") +@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") +class NoTestInTestClassCheckBellPepperTest {} // Noncompliant diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index b91e394256..465f6207ce 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -152,6 +152,7 @@ private static boolean containsJUnit3Tests(List members) { private void checkJunit4AndAboveTestClass(IdentifierTree className, Symbol.TypeSymbol symbol, List members) { addUsedAnnotations(symbol); + if (!runWithCucumberOrSuiteOrTheoriesRunner(symbol) && members.stream().noneMatch(this::isTestFieldOrMethod)) { reportClass(className); @@ -170,7 +171,8 @@ private void addUsedAnnotations(Symbol.TypeSymbol classSymbol) { } private static boolean runWithCucumberOrSuiteOrTheoriesRunner(Symbol.TypeSymbol symbol) { - return checkRunWith(symbol, "Cucumber", "Suite", "Theories"); + return annotatedIncludeEnginesCucumber(symbol) + || checkRunWith(symbol, "Cucumber", "Suite", "Theories"); } private static boolean runWithZohhak(Symbol.TypeSymbol symbol) { @@ -199,6 +201,32 @@ private static boolean checkRunWithType(Symbol.TypeSymbol value, String... runne return false; } + /** + * True is the symbol is annotated {@code @IncludeEngines("cucumber")}. + */ + private static boolean annotatedIncludeEnginesCucumber(Symbol.TypeSymbol symbol) { + SymbolMetadata metadata = symbol.metadata(); + + List annotations = metadata.annotations(); + for (SymbolMetadata.AnnotationInstance annotation: annotations) { + if (annotation.symbol().type().fullyQualifiedName().endsWith("IncludeEngines")) { + // values are not available in automatic analysis, so assume "cucumber" is there + if (annotation.values().isEmpty()) { + return true; + } + // otherwise check the list + boolean containsCucumber = annotation.values().stream().anyMatch(annotationValue -> + annotationValue.value() instanceof Object[] vals + && vals.length == 1 + && "cucumber".equals(vals[0])); + if (containsCucumber) { + return true; + } + } + } + return false; + } + private boolean isTestFieldOrMethod(Symbol member) { return member.metadata().annotations().stream().anyMatch(input -> { Type type = input.symbol().type(); diff --git a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java index 78f236f6a8..aceba2bc90 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java @@ -93,4 +93,22 @@ void testNg() { .withCheck(new NoTestInTestClassCheck()) .verifyIssues(); } + + @Test + void testCucumber() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckCucumberTest.java")) + .withCheck(new NoTestInTestClassCheck()) + .verifyIssues(); + } + + @Test + void testCucumberWithoutSemantic() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckCucumberTest.java")) + .withCheck(new NoTestInTestClassCheck()) + .withoutSemantic() + // Note, that the sample file contains a noncompliant test, but we are fine with FN in automatic analysis. + .verifyNoIssues(); + } } From 2b6c2456da9ad5f8687574e4d67c6682e1e19eba Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Tue, 24 Dec 2024 15:09:59 +0100 Subject: [PATCH 2/9] Formatting and minor refactoring. --- .../sonar/java/checks/tests/NoTestInTestClassCheck.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index 465f6207ce..11ae2ab525 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -152,7 +152,6 @@ private static boolean containsJUnit3Tests(List members) { private void checkJunit4AndAboveTestClass(IdentifierTree className, Symbol.TypeSymbol symbol, List members) { addUsedAnnotations(symbol); - if (!runWithCucumberOrSuiteOrTheoriesRunner(symbol) && members.stream().noneMatch(this::isTestFieldOrMethod)) { reportClass(className); @@ -202,13 +201,11 @@ private static boolean checkRunWithType(Symbol.TypeSymbol value, String... runne } /** - * True is the symbol is annotated {@code @IncludeEngines("cucumber")}. + * True is the symbol is annotated {@code @IncludeEngines("cucumber")} + * (with some approximation for automatic analysis). */ private static boolean annotatedIncludeEnginesCucumber(Symbol.TypeSymbol symbol) { - SymbolMetadata metadata = symbol.metadata(); - - List annotations = metadata.annotations(); - for (SymbolMetadata.AnnotationInstance annotation: annotations) { + for (var annotation: symbol.metadata().annotations()) { if (annotation.symbol().type().fullyQualifiedName().endsWith("IncludeEngines")) { // values are not available in automatic analysis, so assume "cucumber" is there if (annotation.values().isEmpty()) { From 8ef0cd0f648dcb46bde0be8e0b5d5c999d276dd9 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Tue, 7 Jan 2025 11:51:53 +0100 Subject: [PATCH 3/9] Empty commit to refresh the branch. From 9ec2d3d28143ce89bee8372753de726c2c7d5e04 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Tue, 7 Jan 2025 15:51:20 +0100 Subject: [PATCH 4/9] Use Objects.deepEquals --- .../sonar/java/checks/tests/NoTestInTestClassCheck.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index 11ae2ab525..ce13accfed 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -201,7 +202,7 @@ private static boolean checkRunWithType(Symbol.TypeSymbol value, String... runne } /** - * True is the symbol is annotated {@code @IncludeEngines("cucumber")} + * True if the symbol is annotated {@code @IncludeEngines("cucumber")} * (with some approximation for automatic analysis). */ private static boolean annotatedIncludeEnginesCucumber(Symbol.TypeSymbol symbol) { @@ -213,9 +214,7 @@ private static boolean annotatedIncludeEnginesCucumber(Symbol.TypeSymbol symbol) } // otherwise check the list boolean containsCucumber = annotation.values().stream().anyMatch(annotationValue -> - annotationValue.value() instanceof Object[] vals - && vals.length == 1 - && "cucumber".equals(vals[0])); + Objects.deepEquals(annotationValue.value(), new Object[]{"cucumber"})); if (containsCucumber) { return true; } From b038737f80013ace4c08b44fe6bc66bbd9b7bd3d Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Tue, 7 Jan 2025 17:09:25 +0100 Subject: [PATCH 5/9] Add autoscan FN introduced by the unit test. --- its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json index 3e0c2f6a44..67320da791 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json @@ -1,6 +1,6 @@ { "ruleKey": "S2187", "hasTruePositives": true, - "falseNegatives": 13, + "falseNegatives": 14, "falsePositives": 1 } From 94abde614b3df1183abe82929636a18e8351e489 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Fri, 10 Jan 2025 13:15:09 +0100 Subject: [PATCH 6/9] Support multiple engines --- .../NoTestInTestClassCheckCucumberTest.java | 16 ++++------------ .../checks/tests/NoTestInTestClassCheck.java | 2 +- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java index c20c90ad3b..5da740df8f 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java @@ -1,23 +1,15 @@ package checks.tests; -import static io.cucumber.junit.platform.engine.Constants.*; +import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.*; - -@Suite @IncludeEngines("cucumber") -@SelectClasspathResource("com.project.class.path") -@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") class NoTestInTestClassCheckCucumberStandardTest {} -@Suite @org.junit.platform.suite.api.IncludeEngines("cucumber") -@SelectClasspathResource("com.project.class.path") -@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") class NoTestInTestClassCheckCucumberFullyQualifiedTest {} -@Suite @IncludeEngines("bellpepper") -@SelectClasspathResource("com.project.class.path") -@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value="com.project.class.path") class NoTestInTestClassCheckBellPepperTest {} // Noncompliant + +@IncludeEngines({"spring", "cucumber"}) +class NoTestInTestClassCheckTwoEnginesTest{} diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index ce13accfed..5c5f131da6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -214,7 +214,7 @@ private static boolean annotatedIncludeEnginesCucumber(Symbol.TypeSymbol symbol) } // otherwise check the list boolean containsCucumber = annotation.values().stream().anyMatch(annotationValue -> - Objects.deepEquals(annotationValue.value(), new Object[]{"cucumber"})); + annotationValue.value() instanceof Object[] vals && Arrays.asList(vals).contains("cucumber")); if (containsCucumber) { return true; } From 9eb7ff703971312923f8bf84fb7e0cf83195d986 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Fri, 10 Jan 2025 13:26:37 +0100 Subject: [PATCH 7/9] Do not add cucumber as pom dependency --- java-checks-test-sources/default/pom.xml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml index d84d6e2d7c..3a13809867 100644 --- a/java-checks-test-sources/default/pom.xml +++ b/java-checks-test-sources/default/pom.xml @@ -998,18 +998,6 @@ jspecify 1.0.0 - - io.cucumber - cucumber-java - 7.20.1 - provided - - - io.cucumber - cucumber-junit-platform-engine - 7.20.1 - provided - org.junit.platform junit-platform-suite From 90b5544c9a9d4c7d5e9e655a221eb67b3ae93001 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Fri, 10 Jan 2025 13:31:32 +0100 Subject: [PATCH 8/9] imports --- .../java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index 5c5f131da6..050df4176d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; From 9ad2c81879feb5609d2387aa6f7729d4901b1089 Mon Sep 17 00:00:00 2001 From: Tomasz Tylenda Date: Fri, 10 Jan 2025 21:33:38 +0100 Subject: [PATCH 9/9] Review - improve test coverage --- .../NoTestInTestClassCheckCucumberTest.java | 9 +++++++++ ...tClassCheckCucumberWithoutSemanticTest.java | 18 ++++++++++++++++++ .../tests/NoTestInTestClassCheckTest.java | 3 +-- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberWithoutSemanticTest.java diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java index 5da740df8f..79589d1014 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberTest.java @@ -13,3 +13,12 @@ class NoTestInTestClassCheckBellPepperTest {} // Noncompliant @IncludeEngines({"spring", "cucumber"}) class NoTestInTestClassCheckTwoEnginesTest{} + +@NoTestInTestClassIncompatibleAnnotations.IncludeEngines(42) +class ClassWithFakeIncludeEnginesAnnotationTest {} // Noncompliant + +class NoTestInTestClassIncompatibleAnnotations { + @interface IncludeEngines { + int value(); + } +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberWithoutSemanticTest.java b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberWithoutSemanticTest.java new file mode 100644 index 0000000000..e9016ff134 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckCucumberWithoutSemanticTest.java @@ -0,0 +1,18 @@ +package checks.tests; + +import org.junit.platform.suite.api.IncludeEngines; + +@IncludeEngines("cucumber") +class NoTestInTestClassCheckCucumberStandardWSTest {} + +@IncludeEngines("cucumber") +class NoTestInTestClassCheckCucumberFullyQualifiedWSTest {} + +@IncludeEngines("bellpepper") +class NoTestInTestClassCheckBellPepperWSTest {} // FN in automatic analysis + +@IncludeEngines({"spring", "cucumber"}) +class NoTestInTestClassCheckTwoEnginesWSTest{} + +@NoTestInTestClassIncompatibleAnnotations.IncludeEngines(42) +class ClassWithFakeIncludeEnginesAnnotationWSTest {} // FN in automatic analysis diff --git a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java index aceba2bc90..12d1a9d42c 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java @@ -105,10 +105,9 @@ void testCucumber() { @Test void testCucumberWithoutSemantic() { CheckVerifier.newVerifier() - .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckCucumberTest.java")) + .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckCucumberWithoutSemanticTest.java")) .withCheck(new NoTestInTestClassCheck()) .withoutSemantic() - // Note, that the sample file contains a noncompliant test, but we are fine with FN in automatic analysis. .verifyNoIssues(); } }