Skip to content

Commit 7b03871

Browse files
amalloyError Prone Team
authored andcommitted
Fix crashes / wrong suggestions in FloggerStringConcatenation
It assumed every binary operation would be string concatenation. PiperOrigin-RevId: 391825940
1 parent 43d34de commit 7b03871

File tree

2 files changed

+34
-11
lines changed

2 files changed

+34
-11
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenation.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.google.errorprone.bugpatterns.flogger;
1818

19-
import static com.google.common.base.Preconditions.checkState;
2019
import static com.google.common.collect.Iterables.getOnlyElement;
2120
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
2221
import static com.google.errorprone.bugpatterns.flogger.FloggerHelpers.inferFormatSpecifier;
@@ -77,20 +76,20 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
7776
new SimpleTreeVisitor<Void, Void>() {
7877
@Override
7978
public Void visitBinary(BinaryTree tree, Void unused) {
80-
checkState(tree.getKind().equals(Kind.PLUS));
81-
tree.getLeftOperand().accept(this, null);
82-
tree.getRightOperand().accept(this, null);
83-
return null;
79+
if (tree.getKind().equals(Kind.PLUS)
80+
&& isSameType(getType(tree), state.getSymtab().stringType, state)) {
81+
// + yielding a String is concatenation, and should use placeholders for each part.
82+
tree.getLeftOperand().accept(this, null);
83+
return tree.getRightOperand().accept(this, null);
84+
} else {
85+
// Otherwise it's not concatenation, and should be its own placeholder
86+
return defaultAction(tree, null);
87+
}
8488
}
8589

8690
@Override
8791
public Void visitParenthesized(ParenthesizedTree node, Void unused) {
88-
if (isSameType(getType(node), state.getSymtab().stringType, state)) {
89-
node.getExpression().accept(this, null);
90-
} else {
91-
pieces.add(node.getExpression());
92-
}
93-
return null;
92+
return node.getExpression().accept(this, null);
9493
}
9594

9695
@Override

core/src/test/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenationTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,28 @@ public void minus() {
9393
"}")
9494
.doTest();
9595
}
96+
97+
@Test
98+
public void numericOps() {
99+
testHelper
100+
.addInputLines(
101+
"in/Test.java",
102+
"import com.google.common.flogger.FluentLogger;",
103+
"class Test {",
104+
" private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
105+
" public void method(int x, int y) {",
106+
" logger.atInfo().log(x + y + \" sum; mean \" + (x + y) / 2);",
107+
" }",
108+
"}")
109+
.addOutputLines(
110+
"out/Test.java",
111+
"import com.google.common.flogger.FluentLogger;",
112+
"class Test {",
113+
" private static final FluentLogger logger = FluentLogger.forEnclosingClass();",
114+
" public void method(int x, int y) {",
115+
" logger.atInfo().log(\"%d sum; mean %d\", x + y, (x + y) / 2);",
116+
" }",
117+
"}")
118+
.doTest();
119+
}
96120
}

0 commit comments

Comments
 (0)