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

Reduce ImmutableList duplication #200

Merged
merged 1 commit into from
Oct 5, 2016
Merged

Reduce ImmutableList duplication #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 95 additions & 28 deletions src/main/java/org/inferred/freebuilder/processor/ListPropertyFactory.java
View file
Open in desktop
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
Open in desktop
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