Fix a crash in FloggerArgumentToString

handle `toString()` (without a receiver) as a log argument. Fixes #2346 PiperOrigin-RevId: 374960322
google · May 20, 2021 · b2c3930 · b2c3930
1 parent 7c92574
commit b2c3930
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 11 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerArgumentToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerArgumentToString.java
@@ -30,7 +30,6 @@
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
 import static com.google.errorprone.util.ASTHelpers.getType;
 import static java.util.Arrays.stream;
-import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.Ascii;
 import com.google.common.collect.ImmutableMap;
@@ -41,6 +40,7 @@
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.suppliers.Supplier;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
@@ -101,18 +101,27 @@ public class FloggerArgumentToString extends BugChecker implements MethodInvocat
 
   static class Parameter {
 
-    final ExpressionTree expression;
+    final Supplier<String> source;
+    final Type type;
     @Nullable final Character placeholder;
 
-    private Parameter(ExpressionTree expression) {
-      this.expression = requireNonNull(expression);
-      this.placeholder = null;
+    private Parameter(ExpressionTree expression, char placeholder) {
+      this(s -> s.getSourceForNode(expression), getType(expression), placeholder);
     }
 
-    private Parameter(ExpressionTree expression, char placeholder) {
-      this.expression = requireNonNull(expression);
+    private Parameter(Supplier<String> source, Type type, char placeholder) {
+      this.source = source;
+      this.type = type;
       this.placeholder = placeholder;
     }
+
+    private static Parameter receiver(MethodInvocationTree invocation, char placeholder) {
+      ExpressionTree receiver = getReceiver(invocation);
+      if (receiver != null) {
+        return new Parameter(getReceiver(invocation), placeholder);
+      }
+      return new Parameter(s -> "this", null, placeholder);
+    }
   }
 
   private enum Unwrapper {
@@ -121,7 +130,7 @@ private enum Unwrapper {
 
       @Override
       Parameter unwrap(MethodInvocationTree invocation, char placeholder) {
-        return new Parameter(getReceiver(invocation), placeholder);
+        return Parameter.receiver(invocation, placeholder);
       }
     },
     // Unwrap things like: String.valueOf(x) --> x
@@ -144,7 +153,7 @@ Parameter unwrap(MethodInvocationTree invocation, char placeholder) {
 
       @Override
       Parameter unwrap(MethodInvocationTree invocation, char placeholder) {
-        return new Parameter(getOnlyArgument(invocation), placeholder);
+        return Parameter.receiver(invocation, placeholder);
       }
     },
     // Unwrap things like: Integer.toString(n) --> n
@@ -321,10 +330,10 @@ Description unwrapArguments(
           char placeholder = term.charAt(1);
           Parameter unwrapped = unwrap(argument, placeholder, state);
           if (unwrapped != null) {
-            fix.replace(argument, state.getSourceForNode(unwrapped.expression));
+            fix.replace(argument, unwrapped.source.get(state));
             placeholder = firstNonNull(unwrapped.placeholder, 's');
             if (placeholder == STRING_FORMAT) {
-              placeholder = inferFormatSpecifier(unwrapped.expression, state);
+              placeholder = inferFormatSpecifier(unwrapped.type, state);
             }
             term = "%" + placeholder;
             fixed = true;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerHelpers.java b/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerHelpers.java
@@ -29,6 +29,10 @@ final class FloggerHelpers {
 
   static char inferFormatSpecifier(Tree piece, VisitorState state) {
     Type type = getType(piece);
+    return inferFormatSpecifier(type, state);
+  }
+
+  static char inferFormatSpecifier(Type type, VisitorState state) {
     if (type == null) {
       return STRING_FORMAT;
     }

diff --git a/.../src/test/java/com/google/errorprone/bugpatterns/flogger/FloggerArgumentToStringTest.java b/.../src/test/java/com/google/errorprone/bugpatterns/flogger/FloggerArgumentToStringTest.java
@@ -98,4 +98,28 @@ public void negative() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void selfToString() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import com.google.common.flogger.FluentLogger;",
+            "class Test {",
+            "  private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
+            "  public void f() {",
+            "    logger.atInfo().log(\"hello '%s'\", toString());",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import com.google.common.flogger.FluentLogger;",
+            "class Test {",
+            "  private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
+            "  public void f() {",
+            "    logger.atInfo().log(\"hello '%s'\", this);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }