Fix crashes / wrong suggestions in FloggerStringConcatenation

It assumed every binary operation would be string concatenation. PiperOrigin-RevId: 391825940
google · Aug 19, 2021 · 7b03871 · 7b03871
1 parent 43d34de
commit 7b03871
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 11 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenation.java b/core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenation.java
@@ -16,7 +16,6 @@
 
 package com.google.errorprone.bugpatterns.flogger;
 
-import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.bugpatterns.flogger.FloggerHelpers.inferFormatSpecifier;
@@ -77,20 +76,20 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
         new SimpleTreeVisitor<Void, Void>() {
           @Override
           public Void visitBinary(BinaryTree tree, Void unused) {
-            checkState(tree.getKind().equals(Kind.PLUS));
-            tree.getLeftOperand().accept(this, null);
-            tree.getRightOperand().accept(this, null);
-            return null;
+            if (tree.getKind().equals(Kind.PLUS)
+                && isSameType(getType(tree), state.getSymtab().stringType, state)) {
+              // + yielding a String is concatenation, and should use placeholders for each part.
+              tree.getLeftOperand().accept(this, null);
+              return tree.getRightOperand().accept(this, null);
+            } else {
+              // Otherwise it's not concatenation, and should be its own placeholder
+              return defaultAction(tree, null);
+            }
           }
 
           @Override
           public Void visitParenthesized(ParenthesizedTree node, Void unused) {
-            if (isSameType(getType(node), state.getSymtab().stringType, state)) {
-              node.getExpression().accept(this, null);
-            } else {
-              pieces.add(node.getExpression());
-            }
-            return null;
+            return node.getExpression().accept(this, null);
           }
 
           @Override

diff --git a/...c/test/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenationTest.java b/...c/test/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenationTest.java
@@ -93,4 +93,28 @@ public void minus() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void numericOps() {
+    testHelper
+        .addInputLines(
+            "in/Test.java",
+            "import com.google.common.flogger.FluentLogger;",
+            "class Test {",
+            "  private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
+            "  public void method(int x, int y) {",
+            "    logger.atInfo().log(x + y + \" sum; mean \" + (x + y) / 2);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "out/Test.java",
+            "import com.google.common.flogger.FluentLogger;",
+            "class Test {",
+            "  private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
+            "  public void method(int x, int y) {",
+            "    logger.atInfo().log(\"%d sum; mean %d\", x + y, (x + y) / 2);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }