From f0d296c6ffda1f3e0d8c511ea6be10571b628da9 Mon Sep 17 00:00:00 2001
From: Joan Viladrosa <joan@moderne.io>
Date: Tue, 7 Nov 2023 15:05:44 +0100
Subject: [PATCH] do not modify when not Object::toString() (#118)

---
 .../slf4j/Slf4jLogShouldBeConstant.java       |  9 ++--
 .../slf4j/Slf4jLogShouldBeConstantTest.java   | 50 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstant.java b/src/main/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstant.java
index 69a92cc..ec1af44 100644
--- a/src/main/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstant.java
+++ b/src/main/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstant.java
@@ -51,6 +51,7 @@ public Duration getEstimatedEffortPerOccurrence() {
     }
 
     private static final MethodMatcher STRING_VALUE_OF = new MethodMatcher("java.lang.String valueOf(..)");
+    private static final MethodMatcher OBJECT_TO_STRING = new MethodMatcher("java.lang.Object toString()");
 
     @Override
     public String getDisplayName() {
@@ -106,11 +107,11 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext execu
                                 m = m.withSelect(method.getSelect());
                                 return m;
                             }
-                        } else if (args.get(0) instanceof J.MethodInvocation && "toString".equals(((J.MethodInvocation) args.get(0)).getSimpleName())) {
-                            Expression valueOf = ((J.MethodInvocation) args.get(0)).getSelect();
-                            if (valueOf != null) {
+                        } else if (OBJECT_TO_STRING.matches(args.get(0))) {
+                            Expression toString = ((J.MethodInvocation) args.get(0)).getSelect();
+                            if (toString != null) {
                                 J.MethodInvocation m = JavaTemplate.builder("\"{}\", #{any()}").contextSensitive().build()
-                                        .apply(getCursor(), method.getCoordinates().replaceArguments(), valueOf);
+                                        .apply(getCursor(), method.getCoordinates().replaceArguments(), toString);
                                 m = m.withSelect(method.getSelect());
                                 return m;
                             }
diff --git a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstantTest.java b/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstantTest.java
index 8b18c5f..b75d9ab 100644
--- a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstantTest.java
+++ b/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstantTest.java
@@ -155,7 +155,7 @@ void toStringWithoutSelect() {
           java("""
             import org.slf4j.Logger;
             import org.slf4j.LoggerFactory;
-            class A implements Cloneable {
+            class A {
                 private static final Logger log = LoggerFactory.getLogger(A.class);
             
                 public void foo() {
@@ -316,4 +316,52 @@ void method() {
         );
     }
 
+    @Test
+    void notObjectToString() {
+        //language=java
+        rewriteRun(
+          java("""
+            import org.slf4j.Logger;
+            import org.slf4j.LoggerFactory;
+            import java.util.Arrays;
+            class A {
+                private static final Logger log = LoggerFactory.getLogger(A.class);
+                String[] values = new String[]{"test1", "test2"};
+                public void foo() {
+                    log.error(Arrays.toString(values));
+                }
+            }
+            """)
+        );
+    }
+
+    @Test
+    void doNotUseToStringOnAnyClass() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import org.slf4j.Logger;
+              class Test {
+                  Logger log;
+                  void test() {
+                      Test t = new Test();
+                      log.info(t.toString());
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+              class Test {
+                  Logger log;
+                  void test() {
+                      Test t = new Test();
+                      log.info("{}", t);
+                  }
+              }
+              """
+          )
+        );
+    }
+
 }