Add recipe to change logger fields to private (#221)

* Add ChangeLoggersToPrivate recipe

* Add a precondition on using any of the loggers

* Cheap checks first, and use ListUtils to change modifiers

* Include `ChangeLoggersToPrivate` with `Slf4jBestPractices`

* Expect logger to be changed to private in Slf4jBestPracticesTest

* Update src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: 3 people

src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright 2025 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.java.logging;
+
+import lombok.EqualsAndHashCode;
+import lombok.Value;
+import org.jspecify.annotations.Nullable;
+import org.openrewrite.*;
+import org.openrewrite.internal.ListUtils;
+import org.openrewrite.java.JavaIsoVisitor;
+import org.openrewrite.java.search.UsesType;
+import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.JavaType;
+import org.openrewrite.java.tree.Space;
+import org.openrewrite.java.tree.TypeUtils;
+import org.openrewrite.marker.Markers;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+
+import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toSet;
+
+@Value
+@EqualsAndHashCode(callSuper = false)
+public class ChangeLoggersToPrivate extends Recipe {
+
+    private static final Set<String> LOGGER_TYPES = Arrays.stream(LoggingFramework.values())
+            .map(LoggingFramework::getLoggerType)
+            .collect(toSet());
+
+    @Override
+    public String getDisplayName() {
+        return "Change logger fields to `private`";
+    }
+
+    @Override
+    public String getDescription() {
+        return "Ensures that logger fields are declared as `private` to encapsulate logging mechanics within the class.";
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getVisitor() {
+        return Preconditions.check(usesAnyLogger(), new JavaIsoVisitor<ExecutionContext>() {
+            @Override
+            public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
+                J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx);
+                if (mv.getTypeExpression() == null ||
+                        !isLoggerType(mv.getTypeExpression().getType()) ||
+                        mv.hasModifier(J.Modifier.Type.Private)) {
+                    return mv;
+                }
+
+                Cursor parent = getCursor().getParentTreeCursor();
+                if (!(parent.getValue() instanceof J.Block)) {
+                    return mv;
+                }
+
+                parent = parent.getParentTreeCursor();
+                if (!(parent.getValue() instanceof J.ClassDeclaration)) {
+                    return mv;
+                }
+
+                J.ClassDeclaration classDeclaration = parent.getValue();
+                if (classDeclaration.getKind() == J.ClassDeclaration.Kind.Type.Interface) {
+                    return mv;
+                }
+
+                List<J.Modifier> mapped = ListUtils.map(mv.getModifiers(), mod -> {
+                    if (mod.getType() == J.Modifier.Type.Public ||
+                            mod.getType() == J.Modifier.Type.Protected ||
+                            mod.getType() == J.Modifier.Type.Private) {
+                        return mod.withType(J.Modifier.Type.Private);
+                    }
+                    return mod;
+                });
+                if (mapped == mv.getModifiers()) {
+                    mapped = ListUtils.insert(mapped, new J.Modifier(
+                            Tree.randomId(),
+                            Space.EMPTY,
+                            Markers.EMPTY,
+                            null,
+                            J.Modifier.Type.Private,
+                            emptyList()
+                    ), 0);
+                }
+                return autoFormat(mv.withModifiers(mapped), mv.getTypeExpression(), ctx, getCursor().getParentTreeCursor());
+            }
+
+            private boolean isLoggerType(@Nullable JavaType type) {
+                JavaType.FullyQualified fqnType = TypeUtils.asFullyQualified(type);
+                if (fqnType != null) {
+                    return LOGGER_TYPES.contains(fqnType.getFullyQualifiedName());
+                }
+                return false;
+            }
+        });
+    }
+
+    private static TreeVisitor<?, ExecutionContext> usesAnyLogger() {
+        UsesType[] usesTypes = new UsesType[LOGGER_TYPES.size()];
+        int i = 0;
+        for (String fqn : LOGGER_TYPES) {
+            usesTypes[i++] = new UsesType<>(fqn, true);
+        }
+        return Preconditions.or(usesTypes);
+    }
+}

src/main/resources/META-INF/rewrite/examples.yml

@@ -128,6 +128,27 @@ examples:
     language: java
 ---
 type: specs.openrewrite.org/v1beta/example
+recipeName: org.openrewrite.java.logging.ChangeLoggersToPrivate
+examples:
+- description: ''
+  sources:
+  - before: |
+      import org.slf4j.Logger;
+      import org.slf4j.LoggerFactory;
+
+      class Test {
+          public static final Logger LOGGER = LoggerFactory.getLogger(Test.class);
+      }
+    after: |
+      import org.slf4j.Logger;
+      import org.slf4j.LoggerFactory;
+
+      class Test {
+          private static final Logger LOGGER = LoggerFactory.getLogger(Test.class);
+      }
+    language: java
+---
+type: specs.openrewrite.org/v1beta/example
 recipeName: org.openrewrite.java.logging.ParameterizedLogging
 examples:
 - description: ''

src/main/resources/META-INF/rewrite/slf4j.yml

@@ -143,6 +143,7 @@ recipeList:
   - org.openrewrite.java.logging.slf4j.Slf4jLogShouldBeConstant
   - org.openrewrite.java.logging.slf4j.CompleteExceptionLogging
   - org.openrewrite.java.logging.CatchBlockLogLevel
+  - org.openrewrite.java.logging.ChangeLoggersToPrivate
   - org.openrewrite.java.logging.slf4j.WrapExpensiveLogStatementsInConditionals
 ---
 type: specs.openrewrite.org/v1beta/recipe

src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java

@@ -0,0 +1,197 @@
+/*
+ * Copyright 2025 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.java.logging;
+
+import org.junit.jupiter.api.Test;
+import org.openrewrite.DocumentExample;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import static org.openrewrite.java.Assertions.java;
+
+class ChangeLoggersToPrivateTest implements RewriteTest {
+
+    @Override
+    public void defaults(RecipeSpec spec) {
+        spec.recipe(new ChangeLoggersToPrivate());
+    }
+
+    @DocumentExample
+    @Test
+    void changePublicSlf4jLoggerPrivate() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.LoggerFactory;
+
+              class Test {
+                  public static final Logger LOGGER = LoggerFactory.getLogger(Test.class);
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.LoggerFactory;
+
+              class Test {
+                  private static final Logger LOGGER = LoggerFactory.getLogger(Test.class);
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void changePublicLog4j2LoggerPrivate() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.apache.logging.log4j.Logger;
+              import org.apache.logging.log4j.LogManager;
+
+              class Test {
+                  public static final Logger LOGGER = LogManager.getLogger(Test.class);
+              }
+              """,
+            """
+              import org.apache.logging.log4j.Logger;
+              import org.apache.logging.log4j.LogManager;
+
+              class Test {
+                  private static final Logger LOGGER = LogManager.getLogger(Test.class);
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void changeProtectedLog4jLoggerPrivate() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.apache.log4j.Logger;
+
+              class Test {
+                  protected Logger log = Logger.getLogger(Test.class);
+              }
+              """,
+            """
+              import org.apache.log4j.Logger;
+
+              class Test {
+                  private Logger log = Logger.getLogger(Test.class);
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void changeDefaultJulLoggerPrivate() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.util.logging.Logger;
+
+              class Test {
+                  static final Logger LOG = Logger.getLogger(Test.class.getName());
+              }
+              """,
+            """
+              import java.util.logging.Logger;
+
+              class Test {
+                  private static final Logger LOG = Logger.getLogger(Test.class.getName());
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void keepExistingPrivateLogger() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.LoggerFactory;
+
+              class Test {
+                  private final Logger logger = LoggerFactory.getLogger(Test.class);
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void notALoggerField() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  public String name = "test";
+                  protected int count = 0;
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void loggerInInterfaceShouldNotChange() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.LoggerFactory;
+
+              interface Constants {
+                  Logger logger = LoggerFactory.getLogger(Constants.class);
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void localVariableLoggerShouldNotChange() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.LoggerFactory;
+
+              class Test {
+                  public void doSomething() {
+                      Logger localLog = LoggerFactory.getLogger(Test.class);
+                      localLog.info("Hello");
+                  }
+              }
+              """
+          )
+        );
+    }
+}

src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java

@@ -63,7 +63,7 @@ void test() {
               import org.slf4j.Logger;
               import org.slf4j.LoggerFactory;
               class Test {
-                  Logger logger = LoggerFactory.getLogger(Test.class);
+                  private Logger logger = LoggerFactory.getLogger(Test.class);
                   void test() {
                       Object obj1 = new Object();
                       Object obj2 = new Object();
@@ -105,7 +105,7 @@ void test() {
               import org.slf4j.Logger;
               import org.slf4j.LoggerFactory;
               class Test {
-                  Logger logger = LoggerFactory.getLogger(Test.class);
+                  private Logger logger = LoggerFactory.getLogger(Test.class);
                   void test() {
                       try {
                         throw new IllegalStateException("oops");