Skip to content

Commit 1440a25

Browse files
eamonnmcmanusGoogle Java Core Libraries
authored andcommitted
Loosen the rules for AutoBuilder copy-constructors.
If the built type has a method `T getFoo()` and its constructor has a parameter `U foo`, then we should still generate a copy-constructor provided `T` is assignable to `U`. The copy-constructor will call `getFoo()` on its parameter and by default that is the value that will get passed to the constructor when `build()` is called. RELNOTES=AutoBuilder copy-constructors no longer require an exact match between a property and the corresponding constructor parameter. PiperOrigin-RevId: 508917172
1 parent 8ba4531 commit 1440a25

File tree

4 files changed

+51
-5
lines changed

4 files changed

+51
-5
lines changed

value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderKotlinTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.junit.Assert.assertThrows;
2020

2121
import com.google.common.collect.ImmutableList;
22+
import java.util.List;
2223
import org.junit.Test;
2324
import org.junit.runner.RunWith;
2425
import org.junit.runners.JUnit4;
@@ -31,6 +32,10 @@ static KotlinDataBuilder builder() {
3132
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataBuilder();
3233
}
3334

35+
static KotlinDataBuilder builder(KotlinData kotlinData) {
36+
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataBuilder(kotlinData);
37+
}
38+
3439
abstract KotlinDataBuilder setInt(int x);
3540

3641
abstract KotlinDataBuilder setString(String x);
@@ -43,6 +48,11 @@ public void simpleKotlin() {
4348
KotlinData x = KotlinDataBuilder.builder().setInt(23).setString("skidoo").build();
4449
assertThat(x.getInt()).isEqualTo(23);
4550
assertThat(x.getString()).isEqualTo("skidoo");
51+
52+
KotlinData y = KotlinDataBuilder.builder(x).setString("chromosomes").build();
53+
assertThat(y.getInt()).isEqualTo(23);
54+
assertThat(y.getString()).isEqualTo("chromosomes");
55+
4656
assertThrows(IllegalStateException.class, () -> KotlinDataBuilder.builder().build());
4757
}
4858

@@ -240,4 +250,34 @@ public void kotlinSomeDefaults_missingRequired() {
240250
assertThat(e).hasMessageThat().contains("requiredInt");
241251
assertThat(e).hasMessageThat().contains("requiredString");
242252
}
253+
254+
@AutoBuilder(ofClass = KotlinDataWithList.class)
255+
interface KotlinDataWithListBuilder {
256+
static KotlinDataWithListBuilder builder() {
257+
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithListBuilder();
258+
}
259+
260+
static KotlinDataWithListBuilder builder(KotlinDataWithList kotlinData) {
261+
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithListBuilder(kotlinData);
262+
}
263+
264+
KotlinDataWithListBuilder list(List<? extends CharSequence> list);
265+
266+
KotlinDataWithListBuilder number(int number);
267+
268+
KotlinDataWithList build();
269+
}
270+
271+
// The `getList()` method returns `List<CharSequence>` as seen from Java, but the `list` parameter
272+
// to the constructor has type `List<? extends CharSequence>`.
273+
@Test
274+
public void kotlinWildcards() {
275+
List<String> strings = ImmutableList.of("foo");
276+
KotlinDataWithList x = KotlinDataWithListBuilder.builder().list(strings).number(17).build();
277+
assertThat(x.getList()).isEqualTo(strings);
278+
assertThat(x.getNumber()).isEqualTo(17);
279+
KotlinDataWithList y = KotlinDataWithListBuilder.builder(x).number(23).build();
280+
assertThat(y.getList()).isEqualTo(strings);
281+
assertThat(y.getNumber()).isEqualTo(23);
282+
}
243283
}

value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ public void propertyBuilder() {
502502
}
503503

504504
static <T> String concatList(ImmutableList<T> list) {
505-
// We're avoiding streams for now so we compile this in Java 7 mode in CompileWithEclipseTest.
505+
// We're avoiding streams for now since we compile this in Java 7 mode in CompileWithEclipseTest
506506
StringBuilder sb = new StringBuilder();
507507
for (T element : list) {
508508
sb.append(element);

value/src/it/functional/src/test/java/com/google/auto/value/KotlinData.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,7 @@ data class KotlinDataSomeDefaults(
4646
val optionalInt: Int = 23,
4747
val optionalString: String = "Skidoo"
4848
)
49+
50+
// CharSequence is an interface so the parameter appears from Java as List<? extends CharSequence>,
51+
// but getList() appears as returning List<CharSequence>.
52+
data class KotlinDataWithList(val list: List<CharSequence>, val number: Int)

value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ private ImmutableMap<String, String> propertyToGetterName(
387387
return ImmutableMap.of();
388388
}
389389
TypeElement type = MoreTypes.asTypeElement(builtType);
390-
Map<String, ExecutableElement> noArgInstanceMethods =
390+
Map<String, ExecutableElement> nameToMethod =
391391
MoreElements.getLocalAndInheritedMethods(type, typeUtils(), elementUtils()).stream()
392392
.filter(m -> m.getParameters().isEmpty())
393393
.filter(m -> !m.getModifiers().contains(Modifier.STATIC))
@@ -405,16 +405,18 @@ private ImmutableMap<String, String> propertyToGetterName(
405405
String name = param.getSimpleName().toString();
406406
// Parameter name is `bar`; we look for `bar()` and `getBar()` (or `getbar()` etc)
407407
// in that order. If `bar` is boolean we also look for `isBar()`.
408-
ExecutableElement getter = noArgInstanceMethods.get(name);
408+
ExecutableElement getter = nameToMethod.get(name);
409409
if (getter == null) {
410-
getter = noArgInstanceMethods.get("get" + name);
410+
getter = nameToMethod.get("get" + name);
411411
if (getter == null && param.asType().getKind() == TypeKind.BOOLEAN) {
412-
getter = noArgInstanceMethods.get("is" + name);
412+
getter = nameToMethod.get("is" + name);
413413
}
414414
}
415415
if (getter != null
416+
&& !typeUtils().isAssignable(getter.getReturnType(), param.asType())
416417
&& !MoreTypes.equivalence()
417418
.equivalent(getter.getReturnType(), param.asType())) {
419+
// TODO(b/268680785): we should not need to have two type checks here
418420
getter = null;
419421
}
420422
return new SimpleEntry<>(name, getter);

0 commit comments

Comments
 (0)