Skip to content

Commit

Permalink
Loosen the rules for AutoBuilder copy-constructors.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Feb 11, 2023
1 parent 8ba4531 commit 1440a25
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.junit.Assert.assertThrows;

import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -31,6 +32,10 @@ static KotlinDataBuilder builder() {
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataBuilder();
}

static KotlinDataBuilder builder(KotlinData kotlinData) {
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataBuilder(kotlinData);
}

abstract KotlinDataBuilder setInt(int x);

abstract KotlinDataBuilder setString(String x);
Expand All @@ -43,6 +48,11 @@ public void simpleKotlin() {
KotlinData x = KotlinDataBuilder.builder().setInt(23).setString("skidoo").build();
assertThat(x.getInt()).isEqualTo(23);
assertThat(x.getString()).isEqualTo("skidoo");

KotlinData y = KotlinDataBuilder.builder(x).setString("chromosomes").build();
assertThat(y.getInt()).isEqualTo(23);
assertThat(y.getString()).isEqualTo("chromosomes");

assertThrows(IllegalStateException.class, () -> KotlinDataBuilder.builder().build());
}

Expand Down Expand Up @@ -240,4 +250,34 @@ public void kotlinSomeDefaults_missingRequired() {
assertThat(e).hasMessageThat().contains("requiredInt");
assertThat(e).hasMessageThat().contains("requiredString");
}

@AutoBuilder(ofClass = KotlinDataWithList.class)
interface KotlinDataWithListBuilder {
static KotlinDataWithListBuilder builder() {
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithListBuilder();
}

static KotlinDataWithListBuilder builder(KotlinDataWithList kotlinData) {
return new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithListBuilder(kotlinData);
}

KotlinDataWithListBuilder list(List<? extends CharSequence> list);

KotlinDataWithListBuilder number(int number);

KotlinDataWithList build();
}

// The `getList()` method returns `List<CharSequence>` as seen from Java, but the `list` parameter
// to the constructor has type `List<? extends CharSequence>`.
@Test
public void kotlinWildcards() {
List<String> strings = ImmutableList.of("foo");
KotlinDataWithList x = KotlinDataWithListBuilder.builder().list(strings).number(17).build();
assertThat(x.getList()).isEqualTo(strings);
assertThat(x.getNumber()).isEqualTo(17);
KotlinDataWithList y = KotlinDataWithListBuilder.builder(x).number(23).build();
assertThat(y.getList()).isEqualTo(strings);
assertThat(y.getNumber()).isEqualTo(23);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ public void propertyBuilder() {
}

static <T> String concatList(ImmutableList<T> list) {
// We're avoiding streams for now so we compile this in Java 7 mode in CompileWithEclipseTest.
// We're avoiding streams for now since we compile this in Java 7 mode in CompileWithEclipseTest
StringBuilder sb = new StringBuilder();
for (T element : list) {
sb.append(element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,7 @@ data class KotlinDataSomeDefaults(
val optionalInt: Int = 23,
val optionalString: String = "Skidoo"
)

// CharSequence is an interface so the parameter appears from Java as List<? extends CharSequence>,
// but getList() appears as returning List<CharSequence>.
data class KotlinDataWithList(val list: List<CharSequence>, val number: Int)
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ private ImmutableMap<String, String> propertyToGetterName(
return ImmutableMap.of();
}
TypeElement type = MoreTypes.asTypeElement(builtType);
Map<String, ExecutableElement> noArgInstanceMethods =
Map<String, ExecutableElement> nameToMethod =
MoreElements.getLocalAndInheritedMethods(type, typeUtils(), elementUtils()).stream()
.filter(m -> m.getParameters().isEmpty())
.filter(m -> !m.getModifiers().contains(Modifier.STATIC))
Expand All @@ -405,16 +405,18 @@ private ImmutableMap<String, String> propertyToGetterName(
String name = param.getSimpleName().toString();
// Parameter name is `bar`; we look for `bar()` and `getBar()` (or `getbar()` etc)
// in that order. If `bar` is boolean we also look for `isBar()`.
ExecutableElement getter = noArgInstanceMethods.get(name);
ExecutableElement getter = nameToMethod.get(name);
if (getter == null) {
getter = noArgInstanceMethods.get("get" + name);
getter = nameToMethod.get("get" + name);
if (getter == null && param.asType().getKind() == TypeKind.BOOLEAN) {
getter = noArgInstanceMethods.get("is" + name);
getter = nameToMethod.get("is" + name);
}
}
if (getter != null
&& !typeUtils().isAssignable(getter.getReturnType(), param.asType())
&& !MoreTypes.equivalence()
.equivalent(getter.getReturnType(), param.asType())) {
// TODO(b/268680785): we should not need to have two type checks here
getter = null;
}
return new SimpleEntry<>(name, getter);
Expand Down

0 comments on commit 1440a25

Please sign in to comment.