Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Commit

Permalink
Merge pull request #200 from google/shared.lists
Browse files Browse the repository at this point in the history 
Reduce ImmutableList duplication
  • Loading branch information
@alicederyn
alicederyn authored Oct 5, 2016
2 parents 36a2ee8 + 4eba68d commit 7607b6e
Show file tree
Hide file tree
Showing 3 changed files with 336 additions and 81 deletions.
123 changes: 95 additions & 28 deletions src/main/java/org/inferred/freebuilder/processor/ListPropertyFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.inferred.freebuilder.processor.PropertyCodeGenerator.Config;
import org.inferred.freebuilder.processor.excerpt.CheckedList;
import org.inferred.freebuilder.processor.util.Block;
import org.inferred.freebuilder.processor.util.Excerpt;
import org.inferred.freebuilder.processor.util.ParameterizedType;
import org.inferred.freebuilder.processor.util.QualifiedName;
import org.inferred.freebuilder.processor.util.SourceBuilder;
Expand Down Expand Up @@ -113,11 +114,19 @@ private static boolean hasAddMethodOverride(Config config, TypeMirror keyType) {

@Override
public void addBuilderFieldDeclaration(SourceBuilder code) {
code.addLine("private final %1$s<%2$s> %3$s = new %1$s%4$s();",
ArrayList.class,
elementType,
property.getName(),
diamondOperator(elementType));
if (code.feature(GUAVA).isAvailable()) {
code.addLine("private %s<%s> %s = %s.of();",
List.class,
elementType,
property.getName(),
ImmutableList.class);
} else {
code.addLine("private final %1$s<%2$s> %3$s = new %1$s%4$s();",
ArrayList.class,
elementType,
property.getName(),
diamondOperator(elementType));
}
}

@Override
Expand All @@ -143,6 +152,14 @@ private void addAdd(SourceBuilder code, Metadata metadata) {
code.addLine(" */")
.addLine("public %s %s(%s element) {",
metadata.getBuilder(), addMethod(property), unboxedType.or(elementType));
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (this.%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" this.%1$s = new %2$s%3$s(this.%1$s);",
property.getName(),
ArrayList.class,
diamondOperator(elementType))
.addLine(" }");
}
if (unboxedType.isPresent()) {
code.addLine(" this.%s.add(element);", property.getName());
} else {
Expand All @@ -168,9 +185,18 @@ private void addVarargsAdd(SourceBuilder code, Metadata metadata) {
.addLine("public %s %s(%s... elements) {",
metadata.getBuilder(),
addMethod(property),
unboxedType.or(elementType))
.addLine(" %1$s.ensureCapacity(%1$s.size() + elements.length);", property.getName())
.addLine(" for (%s element : elements) {", unboxedType.or(elementType))
unboxedType.or(elementType));
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" %1$s = new %2$s%3$s(%1$s);",
property.getName(), ArrayList.class, diamondOperator(elementType))
.addLine(" }")
.add(" ((%s<?>) %s)", ArrayList.class, property.getName());
} else {
code.add(" %s", property.getName());
}
code.add(".ensureCapacity(%s.size() + elements.length);%n", property.getName());
code.addLine(" for (%s element : elements) {", unboxedType.or(elementType))
.addLine(" %s(element);", addMethod(property))
.addLine(" }")
.addLine(" return (%s) this;", metadata.getBuilder())
Expand All @@ -192,15 +218,37 @@ private void addAddAll(SourceBuilder code, Metadata metadata) {
metadata.getBuilder(),
addAllMethod(property),
Iterable.class,
elementType)
.addLine(" if (elements instanceof %s) {", Collection.class)
.addLine(" %1$s.ensureCapacity(%1$s.size() + ((%2$s<?>) elements).size());",
property.getName(), Collection.class)
.addLine(" }")
.addLine(" for (%s element : elements) {", unboxedType.or(elementType))
.addLine(" %s(element);", addMethod(property))
.addLine(" }")
.addLine(" return (%s) this;", metadata.getBuilder())
elementType);
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (%1$s == %2$s.<%3$s>of() && elements instanceof %2$s) {",
property.getName(), ImmutableList.class, elementType)
.addLine(" // Use ImmutableList.copyOf to avoid an unchecked cast.")
.addLine(" %s = %s.copyOf(elements);", property.getName(), ImmutableList.class)
.addLine(" } else {")
.addLine(" if (%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" %1$s = new %2$s%3$s(%1$s);",
property.getName(),
ArrayList.class,
diamondOperator(elementType))
.addLine(" }");
}
code.addLine(" if (elements instanceof %s) {", Collection.class)
.add(" ");
if (code.feature(GUAVA).isAvailable()) {
code.add("((%s<?>) %s)", ArrayList.class, property.getName());
} else {
code.add("%s", property.getName());
}
code.add(".ensureCapacity(%s.size() + ((%s<?>) elements).size());%n",
property.getName(), Collection.class);
code.addLine(" }")
.addLine(" for (%s element : elements) {", unboxedType.or(elementType))
.addLine(" %s(element);", addMethod(property))
.addLine(" }");
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" }");
}
code.addLine(" return (%s) this;", metadata.getBuilder())
.addLine("}");
}

Expand Down Expand Up @@ -228,6 +276,14 @@ private void addMutate(SourceBuilder code, Metadata metadata) {
consumer.getQualifiedName(),
List.class,
elementType);
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (this.%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" this.%1$s = new %2$s%3$s(this.%1$s);",
property.getName(),
ArrayList.class,
diamondOperator(elementType))
.addLine(" }");
}
if (overridesAddMethod) {
code.addLine(" mutator.accept(new CheckedList<>(%s, this::%s));",
property.getName(), addMethod(property));
Expand All @@ -248,9 +304,17 @@ private void addClear(SourceBuilder code, Metadata metadata) {
.addLine(" *")
.addLine(" * @return this {@code %s} object", metadata.getBuilder().getSimpleName())
.addLine(" */")
.addLine("public %s %s() {", metadata.getBuilder(), clearMethod(property))
.addLine(" this.%s.clear();", property.getName())
.addLine(" return (%s) this;", metadata.getBuilder())
.addLine("public %s %s() {", metadata.getBuilder(), clearMethod(property));
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" %s = %s.of();", property.getName(), ImmutableList.class)
.addLine(" } else {");
}
code.addLine(" %s.clear();", property.getName());
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" }");
}
code.addLine(" return (%s) this;", metadata.getBuilder())
.addLine("}");
}

Expand All @@ -261,8 +325,14 @@ private void addGetter(SourceBuilder code, Metadata metadata) {
.addLine(" * %s.", metadata.getType().javadocNoArgMethodLink(property.getGetterName()))
.addLine(" * Changes to this builder will be reflected in the view.")
.addLine(" */")
.addLine("public %s<%s> %s() {", List.class, elementType, getter(property))
.addLine(" return %s.unmodifiableList(%s);", Collections.class, property.getName())
.addLine("public %s<%s> %s() {", List.class, elementType, getter(property));
if (code.feature(GUAVA).isAvailable()) {
code.addLine(" if (%s instanceof %s) {", property.getName(), ImmutableList.class)
.addLine(" %1$s = new %2$s%3$s(%1$s);",
property.getName(), ArrayList.class, diamondOperator(elementType))
.addLine(" }");
}
code.addLine(" return %s.unmodifiableList(%s);", Collections.class, property.getName())
.addLine("}");
}

Expand All @@ -284,11 +354,8 @@ public void addMergeFromValue(Block code, String value) {

@Override
public void addMergeFromBuilder(Block code, String builder) {
code.addLine("%s(((%s) %s).%s);",
addAllMethod(property),
metadata.getGeneratedBuilder(),
builder,
property.getName());
Excerpt base = Declarations.upcastToGeneratedBuilder(code, metadata, builder);
code.addLine("%s(%s.%s);", addAllMethod(property), base, property.getName());
}

@Override
Expand All @@ -298,7 +365,7 @@ public void addSetFromResult(SourceBuilder code, String builder, String variable

@Override
public void addClearField(Block code) {
code.addLine("%s.clear();", property.getName());
code.addLine("%s();", clearMethod(property));
}

@Override
Expand Down
94 changes: 93 additions & 1 deletion src/test/java/org/inferred/freebuilder/processor/ListPropertyFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ public void testAddAllIterable_onlyIteratesOnce() {
.runTest();
}

@Test
public void testAddAllIterable_reusesImmutableListInstance() {
assumeTrue("Guava available", features.get(GUAVA).isAvailable());
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.with(testBuilder()
.addLine("ImmutableList<String> values = ImmutableList.of(\"one\", \"two\");")
.addLine("DataType value = new DataType.Builder()")
.addLine(" .addAllItems(values)", DodgySingleIterable.class)
.addLine(" .build();")
.addLine("assertThat(value.getItems()).isSameAs(values);")
.build())
.runTest();
}

/** Throws a {@link NullPointerException} on second call to {@link #iterator()}. */
public static class DodgySingleIterable implements Iterable<String> {
private ImmutableList<String> values;
Expand Down Expand Up @@ -202,6 +218,21 @@ public void testClear() {
.runTest();
}

@Test
public void testClear_emptyList() {
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.with(testBuilder()
.addLine("DataType value = new DataType.Builder()")
.addLine(" .clearItems()")
.addLine(" .addItems(\"three\", \"four\")")
.addLine(" .build();")
.addLine("assertThat(value.getItems()).containsExactly(\"three\", \"four\").inOrder();")
.build())
.runTest();
}

@Test
public void testGetter_returnsLiveView() {
behaviorTester
Expand Down Expand Up @@ -339,6 +370,58 @@ public void testEquality() {
.runTest();
}

@Test
public void testInstanceReuse() {
assumeTrue("Guava available", features.get(GUAVA).isAvailable());
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.with(testBuilder()
.addLine("DataType value = new DataType.Builder()")
.addLine(" .addItems(\"one\", \"two\")")
.addLine(" .build();")
.addLine("DataType copy = DataType.Builder.from(value).build();")
.addLine("assertThat(value.getItems()).isSameAs(copy.getItems());")
.build())
.runTest();
}

@Test
public void testPropertyClearAfterMergeFromValue() {
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.with(testBuilder()
.addLine("DataType value = new DataType.Builder()")
.addLine(" .addItems(\"one\", \"two\")")
.addLine(" .build();")
.addLine("DataType copy = DataType.Builder")
.addLine(" .from(value)")
.addLine(" .clearItems()")
.addLine(" .build();")
.addLine("assertThat(copy.getItems()).isEmpty();")
.build())
.runTest();
}

@Test
public void testBuilderClearAfterMergeFromValue() {
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.with(testBuilder()
.addLine("DataType value = new DataType.Builder()")
.addLine(" .addItems(\"one\", \"two\")")
.addLine(" .build();")
.addLine("DataType copy = DataType.Builder")
.addLine(" .from(value)")
.addLine(" .clear()")
.addLine(" .build();")
.addLine("assertThat(copy.getItems()).isEmpty();")
.build())
.runTest();
}

@Test
public void testImmutableListProperty() {
assumeTrue("Guava available", features.get(GUAVA).isAvailable());
Expand Down Expand Up @@ -448,7 +531,16 @@ public void testJacksonInteroperability() {
.runTest();
}

@Test
public void testCompilesWithoutWarnings() {
behaviorTester
.with(new Processor(features))
.with(LIST_PROPERTY_AUTO_BUILT_TYPE)
.compiles()
.withNoWarnings();
}

private static TestBuilder testBuilder() {
return new TestBuilder().addImport("com.example.DataType");
return new TestBuilder().addImport("com.example.DataType").addImport(ImmutableList.class);
}
}
Loading

0 comments on commit 7607b6e

Please sign in to comment.