diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/IgnoredTestsCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/IgnoredTestsCheckSample.java index c441a509c78..eb13bdbc57c 100644 --- a/java-checks-test-sources/default/src/test/java/checks/tests/IgnoredTestsCheckSample.java +++ b/java-checks-test-sources/default/src/test/java/checks/tests/IgnoredTestsCheckSample.java @@ -6,13 +6,17 @@ abstract class IgnoredTestsCheckSample { @org.junit.Ignore +//^^^^^^^^^^^^^^^^^> void foo() {} // Noncompliant {{Either add an explanation about why this test is skipped or remove the "@Ignore" annotation.}} // ^^^ @Ignore +//^^^^^^^> void bar() {} // Noncompliant +// ^^^ @Disabled +//^^^^^^^^^> void disabledJunit5() {} // Noncompliant {{Either add an explanation about why this test is skipped or remove the "@Disabled" annotation.}} // ^^^^^^^^^^^^^^ diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/AnnotationsHelper.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/AnnotationsHelper.java index 162653aa7eb..0945a9707e8 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/AnnotationsHelper.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/AnnotationsHelper.java @@ -28,4 +28,10 @@ public static boolean hasUnknownAnnotation(SymbolMetadata symbolMetadata) { return symbolMetadata.annotations().stream().anyMatch(annotation -> annotation.symbol().isUnknown()); } + /** + * Returns the `name` part of a `fully.qualified.name`, that is, the part after the last dot. + */ + public static String annotationTypeIdentifier(String fullyQualified) { + return fullyQualified.substring(fullyQualified.lastIndexOf('.') + 1); + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/IgnoredTestsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/IgnoredTestsCheck.java index 9c285c2d8f7..030b674917f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/IgnoredTestsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/IgnoredTestsCheck.java @@ -27,12 +27,15 @@ import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.SymbolMetadata; import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.AnnotationTree; import org.sonar.plugins.java.api.tree.BlockTree; import org.sonar.plugins.java.api.tree.ExpressionStatementTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.MethodTree; import org.sonar.plugins.java.api.tree.Tree; +import static org.sonar.java.checks.helpers.AnnotationsHelper.annotationTypeIdentifier; + @Rule(key = "S1607") public class IgnoredTestsCheck extends IssuableSubscriptionVisitor { @@ -51,12 +54,20 @@ public List nodesToVisit() { public void visitNode(Tree tree) { MethodTree methodTree = (MethodTree) tree; SymbolMetadata symbolMetadata = methodTree.symbol().metadata(); + // check for @Ignore or @Disabled annotations - boolean hasIgnoreAnnotation = isSilentlyIgnored(symbolMetadata, "org.junit.Ignore"); - boolean hasDisabledAnnotation = isSilentlyIgnored(symbolMetadata, "org.junit.jupiter.api.Disabled"); - if (hasIgnoreAnnotation || hasDisabledAnnotation) { - reportIssue(methodTree.simpleName(), String.format("Either add an explanation about why this test is skipped or remove the " + - "\"@%s\" annotation.", hasIgnoreAnnotation ? "Ignore" : "Disabled")); + for (String annotationName : List.of("org.junit.Ignore", "org.junit.jupiter.api.Disabled")) { + getSilentlyIgnoredAnnotation(symbolMetadata, annotationName) + .ifPresent(annotationTree -> { + String shortName = annotationTypeIdentifier(annotationName); + String message = String.format( + "Either add an explanation about why this test is skipped or remove the \"@%s\" annotation.", + shortName + ); + var secondaryLocation = + new JavaFileScannerContext.Location(String.format("@%s annotation skips the test", shortName), annotationTree); + context.reportIssue(this, methodTree.simpleName(), message, Collections.singletonList(secondaryLocation), null); + }); } // check for "assumeFalse(true)" and "assumeTrue(false)"-calls, which may also result in permanent skipping of the given test @@ -80,7 +91,10 @@ public void visitNode(Tree tree) { } } - private static boolean isSilentlyIgnored(SymbolMetadata symbolMetadata, String fullyQualifiedName) { + /** + * If a test method is silently ignored, returns the annotation that causes this behavior. + */ + private static Optional getSilentlyIgnoredAnnotation(SymbolMetadata symbolMetadata, String fullyQualifiedName) { // This code duplicates the behavior of SymbolMetadata.valuesForAnnotation but checks for broken semantics for (SymbolMetadata.AnnotationInstance annotation : symbolMetadata.annotations()) { Type type = annotation.symbol().type(); @@ -88,13 +102,15 @@ private static boolean isSilentlyIgnored(SymbolMetadata symbolMetadata, String f // As a consequence, fetching the values from the annotation returns an empty list, as if there were no value, even though there might be one or more. // In such cases, it is best to consider that the test is not ignored. if (type.isUnknown()) { - return false; + return Optional.empty(); } if (type.is(fullyQualifiedName)) { - return annotation.values().isEmpty(); + return annotation.values().isEmpty() + ? Optional.ofNullable(symbolMetadata.findAnnotationTree(annotation)) + : Optional.empty(); } } - return false; + return Optional.empty(); } private static boolean hasConstantOppositeArg(MethodInvocationTree mit) { diff --git a/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java b/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java index 3894bb57663..46a41ace86e 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java @@ -22,7 +22,6 @@ import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; -import org.sonar.java.annotations.VisibleForTesting; import org.sonar.java.checks.AtLeastOneConstructorCheck; import org.sonar.java.checks.CollectionInappropriateCallsCheck; import org.sonar.java.checks.ConstantsShouldBeStaticFinalCheck; @@ -52,6 +51,8 @@ import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; +import static org.sonar.java.checks.helpers.AnnotationsHelper.annotationTypeIdentifier; + public class LombokFilter extends BaseTreeVisitorIssueFilter { private static final Set> FILTERED_RULES = Set.of( @@ -201,11 +202,6 @@ private static boolean usesAnnotation(ClassTree classTree, List annotati return false; } - @VisibleForTesting - static String annotationTypeIdentifier(String fullyQualified) { - return fullyQualified.substring(fullyQualified.lastIndexOf('.') + 1); - } - private static boolean generatesNonPublicConstructor(ClassTree classTree) { if (usesAnnotation(classTree, UTILITY_CLASS, true)) { return true; diff --git a/java-checks/src/test/java/org/sonar/java/checks/helpers/AnnotationsHelperTest.java b/java-checks/src/test/java/org/sonar/java/checks/helpers/AnnotationsHelperTest.java new file mode 100644 index 00000000000..08485441bca --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/helpers/AnnotationsHelperTest.java @@ -0,0 +1,31 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.helpers; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.java.checks.helpers.AnnotationsHelper.annotationTypeIdentifier; + +class AnnotationsHelperTest { + @Test + void testAnnotationTypeIdentifier() { + assertThat(annotationTypeIdentifier("noDot")).isEqualTo("noDot"); + assertThat(annotationTypeIdentifier("one.dot")).isEqualTo("dot"); + assertThat(annotationTypeIdentifier("many.many.dots")).isEqualTo("dots"); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/filters/LombokFilterTest.java b/java-checks/src/test/java/org/sonar/java/filters/LombokFilterTest.java index a82b4e0f2b6..f91149f1050 100644 --- a/java-checks/src/test/java/org/sonar/java/filters/LombokFilterTest.java +++ b/java-checks/src/test/java/org/sonar/java/filters/LombokFilterTest.java @@ -18,9 +18,6 @@ import org.junit.jupiter.api.Test; -import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.java.filters.LombokFilter.annotationTypeIdentifier; - class LombokFilterTest { @Test @@ -35,11 +32,4 @@ void testWithoutSemantic() { .withoutSemantic() .verify("src/test/files/filters/LombokFilterWithoutSemantic.java", new LombokFilter()); } - - @Test - void testAnnotationTypeIdentifier() { - assertThat(annotationTypeIdentifier("noDot")).isEqualTo("noDot"); - assertThat(annotationTypeIdentifier("one.dot")).isEqualTo("dot"); - assertThat(annotationTypeIdentifier("many.many.dots")).isEqualTo("dots"); - } }