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

Commit a138a0a

Browse files
j-bakerj-baker
authored
Merge pull request #233 from google/issues/229.collection.of.parameters
Fix illegal code generated for sets/lists of parameters
2 parents 183d824 + 7c7a760 commit a138a0a

File tree

5 files changed

+123
-25
lines changed

5 files changed

+123
-25
lines changed

‎src/main/java/org/inferred/freebuilder/processor/ListPropertyFactory.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,7 @@ public void addFinalFieldAssignment(SourceBuilder code, String finalField, Strin
396396
code.addLine("%s = %s.copyOf(%s.%s);",
397397
finalField, ImmutableList.class, builder, property.getName());
398398
} else {
399-
code.addLine("%s = immutableList(%s.%s, %s.class);",
400-
finalField, builder, property.getName(), elementType);
399+
code.addLine("%s = immutableList(%s.%s);", finalField, builder, property.getName());
401400
}
402401
}
403402

@@ -406,7 +405,7 @@ public void addMergeFromValue(Block code, String value) {
406405
if (code.feature(GUAVA).isAvailable()) {
407406
code.addLine("if (%s instanceof %s && %s == %s.<%s>of()) {",
408407
value,
409-
metadata.getValueType(),
408+
metadata.getValueType().getQualifiedName(),
410409
property.getName(),
411410
ImmutableList.class,
412411
elementType)
@@ -453,17 +452,16 @@ public void addTo(SourceBuilder code) {
453452
if (!code.feature(GUAVA).isAvailable()) {
454453
code.addLine("")
455454
.addLine("@%s(\"unchecked\")", SuppressWarnings.class)
456-
.addLine("private static <E> %1$s<E> immutableList(%1$s<E> elements, %2$s<E> type) {",
457-
List.class, Class.class)
455+
.addLine("private static <E> %1$s<E> immutableList(%1$s<E> elements) {", List.class)
458456
.addLine(" switch (elements.size()) {")
459457
.addLine(" case 0:")
460458
.addLine(" return %s.emptyList();", Collections.class)
461459
.addLine(" case 1:")
462460
.addLine(" return %s.singletonList(elements.get(0));", Collections.class)
463461
.addLine(" default:")
464-
.addLine(" return %s.unmodifiableList(%s.asList(elements.toArray(",
465-
Collections.class, Arrays.class)
466-
.addLine(" (E[]) %s.newInstance(type, elements.size()))));", Array.class)
462+
.addLine(" return (%1$s<E>)(%1$s<?>) %2$s.unmodifiableList(%3$s.asList(",
463+
List.class, Collections.class, Arrays.class)
464+
.addLine(" elements.toArray()));", Array.class)
467465
.addLine(" }")
468466
.addLine("}");
469467
}

‎src/main/java/org/inferred/freebuilder/processor/SetPropertyFactory.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,11 @@ public void addFinalFieldAssignment(SourceBuilder code, String finalField, Strin
380380
public void addMergeFromValue(Block code, String value) {
381381
if (code.feature(GUAVA).isAvailable()) {
382382
code.addLine("if (%s instanceof %s && %s == %s.<%s>of()) {",
383-
value, metadata.getValueType(), property.getName(), ImmutableSet.class, elementType)
383+
value,
384+
metadata.getValueType().getQualifiedName(),
385+
property.getName(),
386+
ImmutableSet.class,
387+
elementType)
384388
.addLine(" %s = %s.copyOf(%s.%s());",
385389
property.getName(), ImmutableSet.class, value, property.getGetterName())
386390
.addLine("} else {");

‎src/test/java/org/inferred/freebuilder/processor/ListPrefixlessPropertyTest.java

+49
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,55 @@ public void testCollectionProperty_withWildcard() {
635635
.runTest();
636636
}
637637

638+
@Test
639+
public void testListOfParameters() {
640+
// See also
641+
// - https://github.com/google/FreeBuilder/issues/178
642+
// - https://github.com/google/FreeBuilder/issues/229
643+
behaviorTester
644+
.with(new Processor(features))
645+
.with(new SourceBuilder()
646+
.addLine("package com.example;")
647+
.addLine("@%s", FreeBuilder.class)
648+
.addLine("public abstract class DataType<E> {")
649+
.addLine(" public abstract %s<E> items();", List.class)
650+
.addLine("")
651+
.addLine(" public static class Builder<E> extends DataType_Builder<E> {}")
652+
.addLine("}")
653+
.build())
654+
.with(testBuilder()
655+
.addLine("DataType<String> value = new DataType.Builder<String>()")
656+
.addLine(" .addItems(\"one\")")
657+
.addLine(" .addItems(\"two\")")
658+
.addLine(" .build();")
659+
.addLine("assertThat(value.items()).containsExactly(\"one\", \"two\").inOrder();")
660+
.build())
661+
.runTest();
662+
}
663+
664+
@Test
665+
public void testListOfParameterWildcards() {
666+
behaviorTester
667+
.with(new Processor(features))
668+
.with(new SourceBuilder()
669+
.addLine("package com.example;")
670+
.addLine("@%s", FreeBuilder.class)
671+
.addLine("public abstract class DataType<E> {")
672+
.addLine(" public abstract %s<? extends E> items();", List.class)
673+
.addLine("")
674+
.addLine(" public static class Builder<E> extends DataType_Builder<E> {}")
675+
.addLine("}")
676+
.build())
677+
.with(testBuilder()
678+
.addLine("DataType<Number> value = new DataType.Builder<Number>()")
679+
.addLine(" .addItems(1)")
680+
.addLine(" .addItems(2.7)")
681+
.addLine(" .build();")
682+
.addLine("assertThat(value.items()).containsExactly(1, 2.7).inOrder();")
683+
.build())
684+
.runTest();
685+
}
686+
638687
@Test
639688
public void testImmutableListProperty() {
640689
assumeGuavaAvailable();

‎src/test/java/org/inferred/freebuilder/processor/ListSourceTest.java

+14-16
Original file line numberDiff line numberDiff line change
@@ -1272,8 +1272,8 @@ public void test_noGuava_j6() {
12721272
" private final List<Integer> age;",
12731273
"",
12741274
" private Value(Person_Builder builder) {",
1275-
" this.name = immutableList(builder.name, String.class);",
1276-
" this.age = immutableList(builder.age, Integer.class);",
1275+
" this.name = immutableList(builder.name);",
1276+
" this.age = immutableList(builder.age);",
12771277
" }",
12781278
"",
12791279
" @Override",
@@ -1317,8 +1317,8 @@ public void test_noGuava_j6() {
13171317
" private final List<Integer> age;",
13181318
"",
13191319
" Partial(Person_Builder builder) {",
1320-
" this.name = immutableList(builder.name, String.class);",
1321-
" this.age = immutableList(builder.age, Integer.class);",
1320+
" this.name = immutableList(builder.name);",
1321+
" this.age = immutableList(builder.age);",
13221322
" }",
13231323
"",
13241324
" @Override",
@@ -1363,16 +1363,15 @@ public void test_noGuava_j6() {
13631363
" }",
13641364
"",
13651365
" @SuppressWarnings(\"unchecked\")",
1366-
" private static <E> List<E> immutableList(List<E> elements, Class<E> type) {",
1366+
" private static <E> List<E> immutableList(List<E> elements) {",
13671367
" switch (elements.size()) {",
13681368
" case 0:",
13691369
" return Collections.emptyList();",
13701370
" case 1:",
13711371
" return Collections.singletonList(elements.get(0));",
13721372
" default:",
1373-
" return Collections.unmodifiableList(",
1374-
" Arrays.asList(elements.toArray((E[]) Array.newInstance(type, "
1375-
+ "elements.size()))));",
1373+
" return (List<E>) (List<?>) Collections.unmodifiableList("
1374+
+ "Arrays.asList(elements.toArray()));",
13761375
" }",
13771376
" }",
13781377
"}\n"));
@@ -1576,8 +1575,8 @@ public void test_noGuava_j7() {
15761575
" private final List<Integer> age;",
15771576
"",
15781577
" private Value(Person_Builder builder) {",
1579-
" this.name = immutableList(builder.name, String.class);",
1580-
" this.age = immutableList(builder.age, Integer.class);",
1578+
" this.name = immutableList(builder.name);",
1579+
" this.age = immutableList(builder.age);",
15811580
" }",
15821581
"",
15831582
" @Override",
@@ -1615,8 +1614,8 @@ public void test_noGuava_j7() {
16151614
" private final List<Integer> age;",
16161615
"",
16171616
" Partial(Person_Builder builder) {",
1618-
" this.name = immutableList(builder.name, String.class);",
1619-
" this.age = immutableList(builder.age, Integer.class);",
1617+
" this.name = immutableList(builder.name);",
1618+
" this.age = immutableList(builder.age);",
16201619
" }",
16211620
"",
16221621
" @Override",
@@ -1655,16 +1654,15 @@ public void test_noGuava_j7() {
16551654
" }",
16561655
"",
16571656
" @SuppressWarnings(\"unchecked\")",
1658-
" private static <E> List<E> immutableList(List<E> elements, Class<E> type) {",
1657+
" private static <E> List<E> immutableList(List<E> elements) {",
16591658
" switch (elements.size()) {",
16601659
" case 0:",
16611660
" return Collections.emptyList();",
16621661
" case 1:",
16631662
" return Collections.singletonList(elements.get(0));",
16641663
" default:",
1665-
" return Collections.unmodifiableList(",
1666-
" Arrays.asList(elements.toArray((E[]) Array.newInstance(type, "
1667-
+ "elements.size()))));",
1664+
" return (List<E>) (List<?>) Collections.unmodifiableList("
1665+
+ "Arrays.asList(elements.toArray()));",
16681666
" }",
16691667
" }",
16701668
"}\n"));

‎src/test/java/org/inferred/freebuilder/processor/SetPropertyTest.java

+49
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,55 @@ public void testImmutableSetProperty_withWildcard() {
617617
.runTest();
618618
}
619619

620+
@Test
621+
public void testSetOfParameters() {
622+
// See also https://github.com/google/FreeBuilder/issues/229
623+
behaviorTester
624+
.with(new Processor(features))
625+
.with(new SourceBuilder()
626+
.addLine("package com.example;")
627+
.addLine("@%s", FreeBuilder.class)
628+
.addLine("public interface DataType<E> {")
629+
.addLine(" %s<E> %s;", Set.class, convention.getter())
630+
.addLine("")
631+
.addLine(" class Builder<E> extends DataType_Builder<E> {}")
632+
.addLine("}")
633+
.build())
634+
.with(testBuilder()
635+
.addLine("DataType<%1$s> value = new DataType.Builder<%1$s>()", elements.type())
636+
.addLine(" .addItems(%s)", elements.example(0))
637+
.addLine(" .addItems(%s)", elements.example(1))
638+
.addLine(" .build();")
639+
.addLine("assertThat(value.%s).containsExactly(%s).inOrder();",
640+
convention.getter(), elements.examples(0, 1))
641+
.build())
642+
.runTest();
643+
}
644+
645+
@Test
646+
public void testSetOfParameterWildcards() {
647+
behaviorTester
648+
.with(new Processor(features))
649+
.with(new SourceBuilder()
650+
.addLine("package com.example;")
651+
.addLine("@%s", FreeBuilder.class)
652+
.addLine("public interface DataType<E> {")
653+
.addLine(" %s<? extends E> %s;", Set.class, convention.getter())
654+
.addLine("")
655+
.addLine(" class Builder<E> extends DataType_Builder<E> {}")
656+
.addLine("}")
657+
.build())
658+
.with(testBuilder()
659+
.addLine("DataType<%1$s> value = new DataType.Builder<%1$s>()", elements.type())
660+
.addLine(" .addItems(%s)", elements.example(0))
661+
.addLine(" .addItems(%s)", elements.example(1))
662+
.addLine(" .build();")
663+
.addLine("assertThat(value.%s).containsExactly(%s).inOrder();",
664+
convention.getter(), elements.examples(0, 1))
665+
.build())
666+
.runTest();
667+
}
668+
620669
@Test
621670
public void testValidation_varargsAdd() {
622671
thrown.expect(IllegalArgumentException.class);

0 commit comments

Comments
 (0)