Skip to content

Commit cb3a6de

Browse files
dwoffindenError Prone Team
authored andcommitted
Update Varifier to only suggest var in builder chains where the factory is on the resulting type
So _don't_ suggest `var` if we have `Foo foo = Bar.builder().build()`. PiperOrigin-RevId: 817691542
1 parent 144b5b1 commit cb3a6de

File tree

2 files changed

+75
-1
lines changed

2 files changed

+75
-1
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/Varifier.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static com.google.errorprone.matchers.Matchers.anyOf;
2323
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
2424
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
25+
import static com.google.errorprone.util.ASTHelpers.getReceiverType;
2526
import static com.google.errorprone.util.ASTHelpers.getSymbol;
2627
import static com.google.errorprone.util.ASTHelpers.getType;
2728
import static com.google.errorprone.util.ASTHelpers.hasImplicitType;
@@ -104,8 +105,13 @@ && isSameType(getType(newClassTree.getIdentifier()), getType(tree.getType()), st
104105
return fix(tree);
105106
}
106107
// Foo foo = Foo.builder()...build();
108+
// (but not Bar bar = Foo.builder()...build())
107109
if (BUILD_METHOD.matches(initializer, state)
108-
&& streamReceivers(initializer).anyMatch(t -> BUILDER_FACTORY.matches(t, state))) {
110+
&& streamReceivers(initializer)
111+
.anyMatch(
112+
t ->
113+
BUILDER_FACTORY.matches(t, state)
114+
&& isSameType(getReceiverType(t), getType(tree.getType()), state))) {
109115
return fix(tree);
110116
}
111117
// Foo foo = Foo.createFoo(..);

core/src/test/java/com/google/errorprone/bugpatterns/VarifierTest.java

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,74 @@ public void t() {
183183
.doTest();
184184
}
185185

186+
@Test
187+
public void builderChainGeneric() {
188+
refactoringHelper
189+
.addInputLines(
190+
"Test.java",
191+
"""
192+
import com.google.common.collect.ImmutableList;
193+
194+
class Test {
195+
static class Foo {}
196+
197+
public void t() {
198+
ImmutableList<Foo> foos = ImmutableList.<Foo>builder().add(new Foo()).build();
199+
}
200+
}
201+
""")
202+
.addOutputLines(
203+
"Test.java",
204+
"""
205+
import com.google.common.collect.ImmutableList;
206+
207+
class Test {
208+
static class Foo {}
209+
210+
public void t() {
211+
var foos = ImmutableList.<Foo>builder().add(new Foo()).build();
212+
}
213+
}
214+
""")
215+
.doTest();
216+
}
217+
218+
@Test
219+
public void builderChain_typeMismatch() {
220+
refactoringHelper
221+
.addInputLines(
222+
"Test.java",
223+
"""
224+
import com.google.common.collect.ImmutableList;
225+
226+
class Test {
227+
static class Foo {
228+
static Builder newBuilder() {
229+
return new Builder();
230+
}
231+
232+
static class Builder {
233+
Builder setX(int x) {
234+
return this;
235+
}
236+
237+
Bar build() {
238+
return new Bar();
239+
}
240+
}
241+
}
242+
243+
static class Bar {}
244+
245+
public void t() {
246+
Bar bar = Foo.newBuilder().setX(1).build();
247+
}
248+
}
249+
""")
250+
.expectUnchanged()
251+
.doTest();
252+
}
253+
186254
@Test
187255
public void fromFactoryMethod() {
188256
refactoringHelper

0 commit comments

Comments
 (0)