From def406ee54a061c2afb430031bdae1d2f455ba20 Mon Sep 17 00:00:00 2001 From: tomasz-tylenda-sonarsource Date: Fri, 13 Dec 2024 12:02:54 +0100 Subject: [PATCH] SONARJAVA-4973 Fix FP on S1118 with Lombok @UtilityClass in automatic analysis (#4960) Co-authored-by: Dorian Burihabwa <75226315+dorian-burihabwa-sonarsource@users.noreply.github.com> --- .../org/sonar/java/filters/LombokFilter.java | 25 +++++++++++++++++-- .../filters/LombokFilterWithoutSemantic.java | 16 ++++++++++-- .../sonar/java/filters/LombokFilterTest.java | 10 ++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) 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 5b0fa18efbb..3894bb57663 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,6 +22,7 @@ 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; @@ -181,12 +182,32 @@ public void visitAssignmentExpression(AssignmentExpressionTree tree) { } private static boolean usesAnnotation(ClassTree classTree, List annotations) { + return usesAnnotation(classTree, annotations, false); + } + + private static boolean usesAnnotation(ClassTree classTree, List annotations, boolean shouldCheckAnnotationLocalName) { SymbolMetadata classMetadata = classTree.symbol().metadata(); - return annotations.stream().anyMatch(classMetadata::isAnnotatedWith); + + for(String fullyQualified: annotations) { + if(classMetadata.isAnnotatedWith(fullyQualified)) { + return true; + } + // In automatic analysis use only the last part of the annotation. + if(shouldCheckAnnotationLocalName && classMetadata.isAnnotatedWith(annotationTypeIdentifier(fullyQualified))) { + return true; + } + } + + 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)) { + if (usesAnnotation(classTree, UTILITY_CLASS, true)) { return true; } SymbolMetadata metadata = classTree.symbol().metadata(); diff --git a/java-checks/src/test/files/filters/LombokFilterWithoutSemantic.java b/java-checks/src/test/files/filters/LombokFilterWithoutSemantic.java index 526b96b6a65..8194a56c4ab 100644 --- a/java-checks/src/test/files/filters/LombokFilterWithoutSemantic.java +++ b/java-checks/src/test/files/filters/LombokFilterWithoutSemantic.java @@ -1,8 +1,20 @@ import lombok.experimental.UtilityClass; -// FP happening without semantics. +class Utility { // WithIssue + public static int triple(int in) { + return in * 3; + } +} + @UtilityClass -public class Utility { // WithIssue +class UtilityAnnotated { // NoIssue + public static int triple(int in) { + return in * 3; + } +} + +@lombok.experimental.UtilityClass +class UtilityFullyQualified { // NoIssue public static int triple(int in) { return in * 3; } 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 f91149f1050..a82b4e0f2b6 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,6 +18,9 @@ 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 @@ -32,4 +35,11 @@ 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"); + } }