SONARJAVA-4937 S1118 Add support for lombok generated constructors (#…

…4768)

its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java

@@ -197,7 +197,7 @@ public void javaCheckTestSources() throws Exception {
     SoftAssertions softly = new SoftAssertions();
     softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
     softly.assertThat(newTotal).isEqualTo(knownTotal);
-    softly.assertThat(rulesCausingFPs).hasSize(8);
+    softly.assertThat(rulesCausingFPs).hasSize(9);
     softly.assertThat(rulesNotReporting).hasSize(10);
 
     /**

its/autoscan/src/test/resources/autoscan/diffs/diff_S1118.json

@@ -2,5 +2,5 @@
   "ruleKey": "S1118",
   "hasTruePositives": true,
   "falseNegatives": 0,
-  "falsePositives": 0
-}
+  "falsePositives": 3
+}

java-checks-test-sources/default/src/main/java/checks/UtilityClassWithPublicConstructorCheckSample.java

@@ -0,0 +1,206 @@
+package checks;
+
+import java.io.Serializable;
+import javax.annotation.CheckForNull;
+import lombok.AccessLevel;
+import lombok.AllArgsConstructor;
+import lombok.NoArgsConstructor;
+import lombok.RequiredArgsConstructor;
+
+import static lombok.AccessLevel.PRIVATE;
+
+class UtilityClassWithPublicConstructorCheckSample {
+
+  @CheckForNull
+  class Coverage { // Noncompliant
+    public static void foo() {
+    }
+  }
+
+  @NoArgsConstructor(access = AccessLevel.PRIVATE, force = true)
+  class LombokClass1 { // Compliant, a private constructor will be generated
+    public static void foo() {
+    }
+  }
+
+  @NoArgsConstructor(access = AccessLevel.PUBLIC)
+  class LombokClass2 { // Noncompliant
+    public static void foo() {
+    }
+  }
+
+  @NoArgsConstructor(force = true)
+  class LombokClass6 { // Noncompliant
+    public static void foo() {
+    }
+  }
+
+  @AllArgsConstructor(access = PRIVATE)
+  class LombokClass3 { // Compliant, a private constructor will be generated
+    public static void foo() {
+    }
+  }
+
+  @RequiredArgsConstructor(access = PRIVATE)
+  class LombokClass4 { // Compliant, a private constructor will be generated
+    public static void foo() {
+    }
+  }
+
+  class Foo1 {
+  }
+
+  class Foo2 {
+    public void foo() {
+    }
+  }
+
+  class Foo3 { // Noncompliant [[sc=9;ec=13]] {{Add a private constructor to hide the implicit public one.}}
+    public static void foo() {
+    }
+  }
+
+  class Foo4 {
+    public static void foo() {
+    }
+
+    public void bar() {
+    }
+  }
+
+  class Foo5 {
+    public Foo5() { // Noncompliant {{Hide this public constructor.}}
+    }
+
+    public static void foo() {
+    }
+  }
+
+  class Foo6 {
+    private Foo6() {
+    }
+
+    public static void foo() {
+    }
+
+    int foo;
+
+    static int bar;
+  }
+
+  class Foo7 {
+
+    public <T> Foo7(T foo) { // Noncompliant
+    }
+
+    public static <T> void foo(T foo) {
+    }
+
+  }
+
+  class Foo8 extends Bar {
+
+    public static void f() {
+    }
+
+  }
+
+  class Foo9 {
+
+    public int foo;
+
+    public static void foo() {
+    }
+
+  }
+
+  class Foo10 { // Noncompliant
+
+    public static int foo;
+
+    ;
+
+  }
+
+  class Foo11 {
+
+    protected Foo11() {
+    }
+
+    public static int a;
+
+  }
+
+  class Foo12 { // Noncompliant
+    static class plop {
+      int a;
+    }
+  }
+
+  class Foo13 {
+
+    private Foo13() {
+    }
+
+    ;
+  }
+
+  class Foo14 { // Noncompliant [[sc=9;ec=14]] {{Add a private constructor to hide the implicit public one.}}
+    static {
+    }
+  }
+
+  class Foo15 {
+    public Object o = new Object() {
+      public static void foo() {
+      }
+    };
+  }
+
+  class Foo16 implements Serializable { // Compliant
+    private static final long serialVersionUID = 1L;
+  }
+
+  class Foo17 {
+    public Foo17() {
+      // do something
+    }
+  }
+
+  class Main { // Compliant - contains main method
+    public static void main(String[] args) throws Exception {
+      System.out.println("Hello world!");
+    }
+  }
+
+  class NotMain { // Noncompliant
+    static void main(String[] args) throws Exception {
+      System.out.println("Hello world!");
+    }
+
+    static void main2(String[] args) {
+      System.out.println("Hello world!");
+    }
+  }
+
+  public class MySingleton {
+    private void MySingleton2() {
+      // use getInstance()
+    }
+
+    private static class InitializationOnDemandHolderMySingleton { // compliant inner class is private, adding a private constructor won't change anything
+      static final checks.MySingleton INSTANCE = new checks.MySingleton();
+    }
+    static class InitializationOnDemandHolderMySingleton2 { // Noncompliant
+      static final checks.MySingleton INSTANCE = new checks.MySingleton();
+    }
+    private class InitializationOnDemandHolderMySingleton3 {
+      static final checks.MySingleton INSTANCE = new checks.MySingleton();
+    }
+
+    public static checks.MySingleton getInstance() {
+      return InitializationOnDemandHolderMySingleton.INSTANCE;
+    }
+  }
+
+}

java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java

@@ -21,19 +21,29 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.ClassPatternsUtils;
 import org.sonar.java.model.ModifiersUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.tree.AnnotationTree;
+import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Modifier;
 import org.sonar.plugins.java.api.tree.Tree;
 
-
 @Rule(key = "S1118")
 public class UtilityClassWithPublicConstructorCheck extends IssuableSubscriptionVisitor {
 
+  private static final Set<String> LOMBOK_CONSTRUCTOR_GENERATORS = Set.of(
+    "lombok.NoArgsConstructor",
+    "lombok.AllArgsConstructor",
+    "lombok.RequiredArgsConstructor");
+
   @Override
   public List<Tree.Kind> nodesToVisit() {
     return Collections.singletonList(Tree.Kind.CLASS);
@@ -52,16 +62,16 @@ public void visitNode(Tree tree) {
         reportIssue(explicitConstructor.simpleName(), "Hide this public constructor.");
       }
     }
-    if (hasImplicitPublicConstructor) {
+    if (hasImplicitPublicConstructor && !hasCompliantGeneratedConstructors(classTree)) {
       reportIssue(classTree.simpleName(), "Add a private constructor to hide the implicit public one.");
     }
   }
 
   private static List<MethodTree> getExplicitConstructors(ClassTree classTree) {
     return classTree.members().stream()
-        .filter(UtilityClassWithPublicConstructorCheck::isConstructor)
-        .map(MethodTree.class::cast)
-        .toList();
+      .filter(UtilityClassWithPublicConstructorCheck::isConstructor)
+      .map(MethodTree.class::cast)
+      .toList();
   }
 
   private static boolean isConstructor(Tree tree) {
@@ -76,4 +86,30 @@ private static boolean hasPublicModifier(MethodTree methodTree) {
     return ModifiersUtils.hasModifier(methodTree.modifiers(), Modifier.PUBLIC);
   }
 
+  private static boolean hasCompliantGeneratedConstructors(ClassTree classTree) {
+    return classTree.modifiers().annotations().stream().anyMatch(it -> isLombokConstructorGenerator(it) && !hasPublicAccess(it));
+  }
+
+  private static boolean isLombokConstructorGenerator(AnnotationTree annotation) {
+    return LOMBOK_CONSTRUCTOR_GENERATORS.contains(annotation.annotationType().symbolType().fullyQualifiedName());
+  }
+
+  private static boolean hasPublicAccess(AnnotationTree annotation) {
+    return annotation.arguments().stream().noneMatch(it ->
+      isAccessLevelNotPublic(((AssignmentExpressionTree) it).expression())
+    );
+  }
+
+  private static boolean isAccessLevelNotPublic(ExpressionTree tree) {
+    String valueName;
+    if (tree instanceof MemberSelectExpressionTree mset) {
+      valueName = mset.identifier().name();
+    } else if (tree instanceof IdentifierTree identifier) {
+      valueName = identifier.name();
+    } else {
+      return false;
+    }
+    return !"PUBLIC".equals(valueName);
+  }
+
 }