SONARJAVA-7164 Implement rule S9437 - "getInstance()" methods should not return a new instance on every call

java-checks-test-sources/default/src/main/files/non-compiling/checks/GetInstanceReturningNewInstanceCheckSample.java

@@ -0,0 +1,11 @@
+package checks;
+
+class GetInstanceReturningNewInstanceCheckSample {
+
+  static class UnresolvedCreatedType {
+
+    static UnresolvedCreatedType getInstance() {
+      return new Unknown();
+    }
+  }
+}

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

@@ -0,0 +1,211 @@
+package checks;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+class GetInstanceReturningNewInstanceCheckSample {
+
+  static class NoCache {
+
+    private final Map<String, Boolean> flags = new HashMap<>();
+
+    public static NoCache getInstance() {
+//                        ^^^^^^^^^^^>
+      return new NoCache(); // Noncompliant {{Rename this method, as "getInstance" is conventionally reserved for singletons, or make it return a cached instance instead of a new one.}}
+//           ^^^^^^^^^^^^^
+    }
+
+    void enable(String name) {
+      flags.put(name, true);
+    }
+  }
+
+  static class NoCacheWithArguments {
+
+    private NoCacheWithArguments(String name) {
+    }
+
+    static NoCacheWithArguments getInstance() {
+//                              ^^^^^^^^^^^>
+      return new NoCacheWithArguments("default"); // Noncompliant
+//           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    }
+  }
+
+  static class GenericNoCache<T> {
+
+    static <U> GenericNoCache<U> getInstance() {
+//                               ^^^^^^^^^^^>
+      return new GenericNoCache<>(); // Noncompliant
+//           ^^^^^^^^^^^^^^^^^^^^^^
+    }
+  }
+
+  static class RawNoCache<T> {
+
+    static RawNoCache getInstance() {
+//                    ^^^^^^^^^^^>
+      return new RawNoCache(); // Noncompliant
+//           ^^^^^^^^^^^^^^^^
+    }
+  }
+
+  static class EagerlyCached {
+
+    private static final EagerlyCached INSTANCE = new EagerlyCached();
+
+    private EagerlyCached() {
+    }
+
+    static EagerlyCached getInstance() {
+      return INSTANCE;
+    }
+  }
+
+  static class LazilyCached {
+
+    private static LazilyCached instance;
+
+    private LazilyCached() {
+    }
+
+    static LazilyCached getInstance() {
+      if (instance == null) {
+        instance = new LazilyCached();
+      }
+      return instance;
+    }
+  }
+
+  static class SynchronizedLazilyCached {
+
+    private static SynchronizedLazilyCached instance;
+
+    static synchronized SynchronizedLazilyCached getInstance() {
+      if (instance == null) {
+        instance = new SynchronizedLazilyCached();
+      }
+      return instance;
+    }
+  }
+
+  static class HolderCached {
+
+    private static class Holder {
+      private static final HolderCached INSTANCE = new HolderCached();
+    }
+
+    static HolderCached getInstance() {
+      return Holder.INSTANCE;
+    }
+  }
+
+  static class NotStatic {
+
+    NotStatic getInstance() {
+      return new NotStatic();
+    }
+  }
+
+  static class ParameterizedFactory {
+
+    static ParameterizedFactory getInstance(Locale locale) {
+      return new ParameterizedFactory();
+    }
+
+    static ParameterizedFactory getInstance(Locale locale, String variant) {
+      return new ParameterizedFactory();
+    }
+  }
+
+  static class DifferentlyNamedFactories {
+
+    static DifferentlyNamedFactories newInstance() {
+      return new DifferentlyNamedFactories();
+    }
+
+    static DifferentlyNamedFactories create() {
+      return new DifferentlyNamedFactories();
+    }
+
+    static DifferentlyNamedFactories of() {
+      return new DifferentlyNamedFactories();
+    }
+
+    static DifferentlyNamedFactories valueOf() {
+      return new DifferentlyNamedFactories();
+    }
+  }
+
+  static class ReturnsAnotherType {
+
+    static Map<String, String> getInstance() {
+      return new HashMap<>();
+    }
+  }
+
+  static class ReturnsAnonymousSubclass {
+
+    static ReturnsAnonymousSubclass getInstance() {
+      return new ReturnsAnonymousSubclass() {
+      };
+    }
+  }
+
+  static class DoesMoreThanCreate {
+
+    private static int calls;
+
+    static DoesMoreThanCreate getInstance() {
+      calls++;
+      return new DoesMoreThanCreate();
+    }
+  }
+
+  static class ReturnsNull {
+
+    static ReturnsNull getInstance() {
+      return null;
+    }
+  }
+
+  static class NativeAccessor {
+
+    static native NativeAccessor getInstance();
+  }
+
+  static class Throwing {
+
+    static Throwing getInstance() {
+      throw new UnsupportedOperationException();
+    }
+  }
+
+  static class ReturnsNothing {
+
+    static void getInstance() {
+      return;
+    }
+  }
+
+  interface WithoutBody {
+
+    static WithoutBody getInstance() {
+      return null;
+    }
+  }
+
+  static abstract class AbstractGetInstance {
+
+    abstract AbstractGetInstance getInstance();
+  }
+
+  enum EnumSingleton {
+    INSTANCE;
+
+    static EnumSingleton getInstance() {
+      return INSTANCE;
+    }
+  }
+}

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

@@ -0,0 +1,98 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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;
+
+import java.util.List;
+import javax.annotation.CheckForNull;
+import org.sonar.check.Rule;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.BlockTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.MethodTree;
+import org.sonar.plugins.java.api.tree.NewClassTree;
+import org.sonar.plugins.java.api.tree.ReturnStatementTree;
+import org.sonar.plugins.java.api.tree.StatementTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.java.model.ExpressionUtils;
+
+@Rule(key = "S9437")
+public class GetInstanceReturningNewInstanceCheck extends IssuableSubscriptionVisitor {
+
+  private static final String MESSAGE = "Rename this method, as \"getInstance\" is conventionally reserved for singletons,"
+    + " or make it return a cached instance instead of a new one.";
+  private static final String SECONDARY_MESSAGE = "This name promises a single shared instance.";
+
+  private static final MethodMatchers GET_INSTANCE = MethodMatchers.create()
+    .ofAnyType()
+    .names("getInstance")
+    .addWithoutParametersMatcher()
+    .build();
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.METHOD);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    MethodTree method = (MethodTree) tree;
+    if (!method.symbol().isStatic() || !GET_INSTANCE.matches(method)) {
+      return;
+    }
+    NewClassTree instantiation = onlyStatementInstantiation(method);
+    if (instantiation != null && instantiatesOwnClass(instantiation, method)) {
+      List<JavaFileScannerContext.Location> secondary =
+        List.of(new JavaFileScannerContext.Location(SECONDARY_MESSAGE, method.simpleName()));
+      reportIssue(instantiation, MESSAGE, secondary, null);
+    }
+  }
+
+  /**
+   * Returns the instance creation of a method whose body is nothing but {@code return new Something(…);}, or {@code null}
+   * when the body is anything else. A lazily initialised accessor therefore never matches, because testing and assigning
+   * the cached field takes more than that single statement.
+   */
+  @CheckForNull
+  private static NewClassTree onlyStatementInstantiation(MethodTree method) {
+    BlockTree body = method.block();
+    if (body == null || body.body().size() != 1) {
+      return null;
+    }
+    StatementTree statement = body.body().get(0);
+    if (!statement.is(Tree.Kind.RETURN_STATEMENT)) {
+      return null;
+    }
+    ExpressionTree returned = ((ReturnStatementTree) statement).expression();
+    if (returned == null) {
+      return null;
+    }
+    ExpressionTree expression = ExpressionUtils.skipParentheses(returned);
+    if (!expression.is(Tree.Kind.NEW_CLASS)) {
+      return null;
+    }
+    NewClassTree instantiation = (NewClassTree) expression;
+    return instantiation.classBody() == null ? instantiation : null;
+  }
+
+  private static boolean instantiatesOwnClass(NewClassTree instantiation, MethodTree method) {
+    Type createdType = instantiation.symbolType();
+    return !createdType.isUnknown() && createdType.erasure().equals(method.symbol().owner().type().erasure());
+  }
+}

java-checks/src/test/java/org/sonar/java/checks/GetInstanceReturningNewInstanceCheckTest.java

@@ -0,0 +1,54 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;
+
+class GetInstanceReturningNewInstanceCheckTest {
+
+  private static final String SAMPLE = "checks/GetInstanceReturningNewInstanceCheckSample.java";
+  private static final GetInstanceReturningNewInstanceCheck CHECK = new GetInstanceReturningNewInstanceCheck();
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(SAMPLE))
+      .withCheck(CHECK)
+      .verifyIssues();
+  }
+
+  @Test
+  void test_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath(SAMPLE))
+      .withCheck(CHECK)
+      .withoutSemantic()
+      .verifyIssues();
+  }
+
+  @Test
+  void test_non_compiling() {
+    CheckVerifier.newVerifier()
+      .onFile(nonCompilingTestSourcesPath(SAMPLE))
+      .withCheck(CHECK)
+      .verifyNoIssues();
+  }
+}