SONARJAVA-7163 Implement rule S9436 - Classes should not declare more than one logger

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

@@ -0,0 +1,245 @@
+package checks;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.logging.log4j.LogManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.Marker;
+import org.slf4j.MarkerFactory;
+
+class MoreThanOneLoggerCheckSample {
+
+  private static final Logger LOG = LoggerFactory.getLogger(MoreThanOneLoggerCheckSample.class);
+//                            ^^^> {{Logger already declared here.}}
+  private static final Logger LOGGER = LoggerFactory.getLogger(MoreThanOneLoggerCheckSample.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^^
+
+  void log() {
+    LOG.info("one");
+    LOGGER.info("two");
+  }
+}
+
+class SingleLogger {
+
+  private static final Logger LOG = LoggerFactory.getLogger(SingleLogger.class); // Compliant
+
+  void log() {
+    LOG.info("one");
+  }
+}
+
+class NoLogger {
+
+  private static final String NAME = "no logger here"; // Compliant
+
+  private final Object delegate = new Object(); // Compliant
+}
+
+class ThreeLoggers {
+
+  private static final Logger FIRST = LoggerFactory.getLogger(ThreeLoggers.class);
+//                            ^^^^^> {{Logger already declared here.}}
+  private static final Logger SECOND = LoggerFactory.getLogger(ThreeLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^^
+  private static final Logger THIRD = LoggerFactory.getLogger(ThreeLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^
+//                            ^^^^^@-5< {{Logger already declared here.}}
+}
+
+class MixedFacades {
+
+  private static final Logger SLF4J = LoggerFactory.getLogger(MixedFacades.class);
+//                            ^^^^^> {{Logger already declared here.}}
+  private static final org.apache.logging.log4j.Logger LOG4J2 = LogManager.getLogger(MixedFacades.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                                                     ^^^^^^
+}
+
+class JulLoggers {
+
+  private static final java.util.logging.Logger LOG = java.util.logging.Logger.getLogger("first");
+//                                              ^^^> {{Logger already declared here.}}
+  private static final java.util.logging.Logger OTHER = java.util.logging.Logger.getLogger("first"); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                                              ^^^^^
+}
+
+class CommonsLoggingLoggers {
+
+  private static final Log LOG = LogFactory.getLog(CommonsLoggingLoggers.class);
+//                         ^^^> {{Logger already declared here.}}
+  private static final Log OTHER = LogFactory.getLog(CommonsLoggingLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                         ^^^^^
+}
+
+class NonStaticNextToStatic {
+
+  private static final Logger LOG = LoggerFactory.getLogger(NonStaticNextToStatic.class);
+//                            ^^^> {{Logger already declared here.}}
+  private final Logger instanceLogger = LoggerFactory.getLogger(NonStaticNextToStatic.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                     ^^^^^^^^^^^^^^
+}
+
+class LoggerWithMarker {
+
+  private static final Logger LOG = LoggerFactory.getLogger(LoggerWithMarker.class); // Compliant
+  private static final Marker AUDIT = MarkerFactory.getMarker("AUDIT"); // Compliant, a marker is not a logger
+
+  void log() {
+    LOG.info(AUDIT, "audited");
+  }
+}
+
+class OuterWithNestedClasses {
+
+  private static final Logger LOG = LoggerFactory.getLogger(OuterWithNestedClasses.class); // Compliant
+
+  static class Nested {
+    private static final Logger NESTED_LOG = LoggerFactory.getLogger(Nested.class); // Compliant, a nested class is a class of its own
+  }
+
+  class Inner {
+    private final Logger innerLog = LoggerFactory.getLogger(Inner.class); // Compliant, an inner class is a class of its own
+  }
+}
+
+class NestedClassWithTwoLoggers {
+
+  private static final Logger LOG = LoggerFactory.getLogger(NestedClassWithTwoLoggers.class); // Compliant
+
+  static class Nested {
+    private static final Logger FIRST = LoggerFactory.getLogger(Nested.class);
+//                              ^^^^^> {{Logger already declared here.}}
+    private static final Logger SECOND = LoggerFactory.getLogger(Nested.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                              ^^^^^^
+  }
+}
+
+class AnonymousClassLogger {
+
+  private static final Logger LOG = LoggerFactory.getLogger(AnonymousClassLogger.class); // Compliant
+
+  Runnable task() {
+    return new Runnable() {
+      private final Logger anonymousLog = LoggerFactory.getLogger(Runnable.class); // Compliant, the anonymous class body is its own class
+
+      @Override
+      public void run() {
+        anonymousLog.info("run");
+      }
+    };
+  }
+}
+
+class LocalLoggerVariables {
+
+  private static final Logger LOG = LoggerFactory.getLogger(LocalLoggerVariables.class); // Compliant
+
+  void log() {
+    Logger local = LoggerFactory.getLogger(Object.class); // Compliant, a local variable is not a field
+    Logger otherLocal = LoggerFactory.getLogger(String.class); // Compliant
+    local.info("one");
+    otherLocal.info("two");
+    LOG.info("three");
+  }
+}
+
+enum EnumWithTwoLoggers {
+  INSTANCE;
+
+  private static final Logger LOG = LoggerFactory.getLogger(EnumWithTwoLoggers.class);
+//                            ^^^> {{Logger already declared here.}}
+  private static final Logger OTHER = LoggerFactory.getLogger(EnumWithTwoLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^
+}
+
+record RecordWithTwoLoggers(String name) {
+
+  private static final Logger LOG = LoggerFactory.getLogger(RecordWithTwoLoggers.class);
+//                            ^^^> {{Logger already declared here.}}
+  private static final Logger OTHER = LoggerFactory.getLogger(RecordWithTwoLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^
+}
+
+interface InterfaceWithTwoLoggers {
+
+  Logger LOG = LoggerFactory.getLogger(InterfaceWithTwoLoggers.class);
+//       ^^^> {{Logger already declared here.}}
+  Logger OTHER = LoggerFactory.getLogger(InterfaceWithTwoLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//       ^^^^^
+}
+
+class ConfigurableLoggerProperty {
+
+  private static final Logger LOG = LoggerFactory.getLogger(ConfigurableLoggerProperty.class); // Compliant
+  private Logger contextLogger; // Compliant, not initialized here, it is a configurable property
+
+  Logger getContextLogger() {
+    return contextLogger;
+  }
+
+  void setContextLogger(Logger logger) {
+    contextLogger = logger;
+  }
+
+  void start() {
+    if (contextLogger == null) {
+      contextLogger = LoggerFactory.getLogger(ConfigurableLoggerProperty.class.getName() + ".context");
+    }
+    LOG.info("started");
+  }
+}
+
+class DerivedChildLogger {
+
+  private static final Logger LOG = LoggerFactory.getLogger(DerivedChildLogger.class); // Compliant
+  private static final Logger LOG_CONFIG = LoggerFactory.getLogger(LOG.getName() + ".config"); // Compliant, a child category inherits the parent configuration
+
+  void log() {
+    LOG.info("one");
+    LOG_CONFIG.debug("configuration detail");
+  }
+}
+
+class AliasedLogger {
+
+  private static final Logger LOG = LoggerFactory.getLogger(AliasedLogger.class);
+//                            ^^^> {{Logger already declared here.}}
+  private static final Logger OTHER = LOG; // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^
+}
+
+class SameCategoryLogger {
+
+  private static final Logger LOG = LoggerFactory.getLogger(SameCategoryLogger.class);
+//                            ^^^> {{Logger already declared here.}}
+  private static final Logger OTHER = LoggerFactory.getLogger(LOG.getName()); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^
+}
+
+class FinalLoggersAssignedInStaticBlock {
+
+  private static final Logger FIRST;
+//                            ^^^^^> {{Logger already declared here.}}
+  private static final Logger SECOND; // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^^
+
+  static {
+    FIRST = LoggerFactory.getLogger(FinalLoggersAssignedInStaticBlock.class);
+    SECOND = LoggerFactory.getLogger("other");
+  }
+}
+
+@interface AnnotationTypeWithTwoLoggers {
+
+  Logger LOG = LoggerFactory.getLogger(AnnotationTypeWithTwoLoggers.class);
+//       ^^^> {{Logger already declared here.}}
+  Logger OTHER = LoggerFactory.getLogger(AnnotationTypeWithTwoLoggers.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//       ^^^^^
+}
+
+class MultipleLoggerFieldsInOneDeclaration {
+
+  private static final Logger FIRST = LoggerFactory.getLogger(MultipleLoggerFieldsInOneDeclaration.class), SECOND = LoggerFactory.getLogger(MultipleLoggerFieldsInOneDeclaration.class); // Noncompliant {{Remove this logger and use the one already declared in this class.}}
+//                            ^^^^^> {{Logger already declared here.}}                                     ^^^^^^
+}

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

@@ -0,0 +1,150 @@
+/*
+ * 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.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+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.Symbol;
+import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
+import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
+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.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
+
+@Rule(key = "S9436")
+public class MoreThanOneLoggerCheck extends IssuableSubscriptionVisitor {
+
+  private static final String MESSAGE = "Remove this logger and use the one already declared in this class.";
+  private static final String SECONDARY_MESSAGE = "Logger already declared here.";
+
+  private static final Set<String> LOGGER_TYPES = Set.of(
+    "org.slf4j.Logger",
+    "org.apache.logging.log4j.Logger",
+    "org.apache.log4j.Logger",
+    "java.util.logging.Logger",
+    "org.apache.commons.logging.Log");
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return Tree.Kind.CLASS_KINDS;
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    if (context.getSemanticModel() == null) {
+      return;
+    }
+    List<VariableTree> loggerFields = loggerFields((ClassTree) tree);
+    if (loggerFields.size() < 2) {
+      return;
+    }
+    List<JavaFileScannerContext.Location> secondary =
+      Collections.singletonList(new JavaFileScannerContext.Location(SECONDARY_MESSAGE, loggerFields.get(0).simpleName()));
+    loggerFields.subList(1, loggerFields.size())
+      .forEach(extraLogger -> reportIssue(extraLogger.simpleName(), MESSAGE, secondary, null));
+  }
+
+  private static List<VariableTree> loggerFields(ClassTree classTree) {
+    List<VariableTree> loggerFields = classTree.members().stream()
+      .filter(member -> member.is(Tree.Kind.VARIABLE))
+      .map(VariableTree.class::cast)
+      .filter(MoreThanOneLoggerCheck::isDeclaredLogger)
+      .toList();
+    return withoutDerivedLoggers(loggerFields);
+  }
+
+  /**
+   * A non-final logger field without an initializer is a configurable property, assigned at startup
+   * or injected from outside, rather than a category the class gives itself. A final one has to be
+   * assigned in an initializer or a constructor, so it is a category of the class.
+   */
+  private static boolean isDeclaredLogger(VariableTree field) {
+    return (field.initializer() != null || field.symbol().isFinal()) && isLoggerType(field.type().symbolType());
+  }
+
+  /**
+   * Drops the loggers whose name is derived from another logger of the same class, such as
+   * {@code LoggerFactory.getLogger(LOG.getName() + ".config")}. Those create a child category,
+   * which inherits the level and the appenders configured for the parent. An initializer that
+   * merely reuses another logger, as in {@code LoggerFactory.getLogger(LOG.getName())}, is not a
+   * child category and is still reported.
+   */
+  private static List<VariableTree> withoutDerivedLoggers(List<VariableTree> loggerFields) {
+    Set<Symbol> loggerSymbols = loggerFields.stream()
+      .map(VariableTree::symbol)
+      .collect(Collectors.toSet());
+    return loggerFields.stream()
+      .filter(field -> !buildsChildCategory(field, loggerSymbols))
+      .toList();
+  }
+
+  private static boolean buildsChildCategory(VariableTree field, Set<Symbol> loggerSymbols) {
+    if (field.initializer() == null) {
+      return false;
+    }
+    ChildCategoryVisitor visitor = new ChildCategoryVisitor(loggerSymbols);
+    field.initializer().accept(visitor);
+    return visitor.derivesLoggerName && visitor.concatenates;
+  }
+
+  private static boolean isLoggerType(Type type) {
+    return LOGGER_TYPES.stream().anyMatch(type::is);
+  }
+
+  private static class ChildCategoryVisitor extends BaseTreeVisitor {
+
+    private final Set<Symbol> loggerSymbols;
+    private boolean derivesLoggerName = false;
+    private boolean concatenates = false;
+
+    private ChildCategoryVisitor(Set<Symbol> loggerSymbols) {
+      this.loggerSymbols = loggerSymbols;
+    }
+
+    @Override
+    public void visitBinaryExpression(BinaryExpressionTree tree) {
+      if (tree.is(Tree.Kind.PLUS)) {
+        concatenates = true;
+      }
+      super.visitBinaryExpression(tree);
+    }
+
+    @Override
+    public void visitMethodInvocation(MethodInvocationTree tree) {
+      if (tree.methodSelect() instanceof MemberSelectExpressionTree select
+        && "getName".equals(select.identifier().name())
+        && isLoggerField(select.expression())) {
+        derivesLoggerName = true;
+      }
+      super.visitMethodInvocation(tree);
+    }
+
+    private boolean isLoggerField(ExpressionTree expression) {
+      return expression instanceof IdentifierTree identifier && loggerSymbols.contains(identifier.symbol());
+    }
+  }
+}