diff --git a/src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java b/src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java new file mode 100644 index 00000000..2379e623 --- /dev/null +++ b/src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java @@ -0,0 +1,122 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * 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 + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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 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 getVisitor() { + return Preconditions.check(usesAnyLogger(), new JavaIsoVisitor() { + @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 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 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); + } +} diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index c59b5899..9115feb0 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/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: '' diff --git a/src/main/resources/META-INF/rewrite/slf4j.yml b/src/main/resources/META-INF/rewrite/slf4j.yml index 2d96d580..a4399905 100644 --- a/src/main/resources/META-INF/rewrite/slf4j.yml +++ b/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 diff --git a/src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java b/src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java new file mode 100644 index 00000000..5349f9cd --- /dev/null +++ b/src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java @@ -0,0 +1,197 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * 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 + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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"); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java b/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java index afb79d7f..4d5475a0 100644 --- a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java +++ b/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");