#408 Tighten value field type declarations

inferred · Jan 27, 2019 · 1f3e1a4 · 1f3e1a4
2 parents bc437ad + a6e5420
commit 1f3e1a4
Show file tree

Hide file tree

Showing 20 changed files with 97 additions and 53 deletions.
diff --git a/src/main/java/org/inferred/freebuilder/processor/GeneratedBuilder.java b/src/main/java/org/inferred/freebuilder/processor/GeneratedBuilder.java
@@ -299,9 +299,7 @@ private void addValueType(SourceBuilder code) {
         datatype.getValueTypeVisibility(),
         datatype.getValueType().declaration(),
         extending(datatype.getType(), datatype.isInterfaceType()));
-    generatorsByProperty.forEach((property, generator) -> {
-      generator.addValueFieldDeclaration(code, property.getField());
-    });
+    generatorsByProperty.values().forEach(generator -> generator.addValueFieldDeclaration(code));
     addValueTypeConstructor(code);
     addValueTypeGetters(code);
     if (datatype.getHasToBuilderMethod()) {
@@ -448,9 +446,7 @@ private void addPartialType(SourceBuilder code) {
   }
 
   private void addPartialFields(SourceBuilder code) {
-    generatorsByProperty.forEach((property, generator) -> {
-      generator.addValueFieldDeclaration(code, property.getField());
-    });
+    generatorsByProperty.values().forEach(generator -> generator.addValueFieldDeclaration(code));
     if (generatorsByProperty.values().stream().anyMatch(IS_REQUIRED)) {
       code.addLine("  private final %s<%s> %s;",
           EnumSet.class, datatype.getPropertyEnum(), UNSET_PROPERTIES);

diff --git a/src/main/java/org/inferred/freebuilder/processor/excerpt/BuildableList.java b/src/main/java/org/inferred/freebuilder/processor/excerpt/BuildableList.java
@@ -182,10 +182,10 @@ private void addAddAllValues(SourceBuilder code) {
   }
 
   private void addBuild(SourceBuilder code, String buildMethod) {
-    code.addLine("")
-        .addLine("%s<%s> %s() {", List.class, element.type(), buildMethod);
+    code.addLine("");
     if (code.feature(GUAVA).isAvailable()) {
-      code.addLine("  if (elements instanceof %s) {", ImmutableList.class)
+      code.addLine("%s<%s> %s() {", ImmutableList.class, element.type(), buildMethod)
+          .addLine("  if (elements instanceof %s) {", ImmutableList.class)
           .addLine("    return (%s<%s>) elements;", ImmutableList.class, element.type())
           .addLine("  }")
           .addLine("  %1$s.Builder<%2$s> values = %1$s.builder();",
@@ -195,7 +195,8 @@ private void addBuild(SourceBuilder code, String buildMethod) {
           .addLine("  }")
           .addLine("  return values.build();");
     } else {
-      code.addLine("  switch (elements.size()) {")
+      code.addLine("%s<%s> %s() {", List.class, element.type(), buildMethod)
+          .addLine("  switch (elements.size()) {")
           .addLine("    case 0:")
           .addLine("      return %s.emptyList();", Collections.class)
           .addLine("    case 1:")

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/BuildableListProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/BuildableListProperty.java
@@ -133,6 +133,14 @@ private BuildableListProperty(
     this.element = element;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s> %s;",
+        code.feature(GUAVA).isAvailable() ? ImmutableList.class : List.class,
+        element.type(),
+        property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private final %1$s %2$s = new %1$s();",

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/BuildableProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/BuildableProperty.java
@@ -93,6 +93,11 @@ private BuildableProperty(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s %s;", property.getType(), property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private Object %s = null;", property.getField());

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/DefaultProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/DefaultProperty.java
@@ -114,6 +114,11 @@ public Initially initialState() {
     return hasDefault ? Initially.HAS_DEFAULT : Initially.REQUIRED;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s %s;", property.getType(), property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private %s %s;", property.getType(), property.getField());
@@ -209,11 +214,6 @@ private void addGetter(SourceBuilder code) {
         .addLine("}");
   }
 
-  @Override
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
-    code.add("private final %s %s;\n", property.getType(), finalField);
-  }
-
   @Override
   public void addFinalFieldAssignment(SourceBuilder code, Excerpt finalField, String builder) {
     code.addLine("%s = %s;", finalField, property.getField().on(builder));

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/ListMultimapProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/ListMultimapProperty.java
@@ -148,6 +148,12 @@ private static TypeMirror listMultimap(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s, %s> %s;",
+        ImmutableListMultimap.class, keyType, valueType, property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private final %1$s<%2$s, %3$s> %4$s = %1$s.create();",

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/ListProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/ListProperty.java
@@ -157,6 +157,14 @@ private static TypeMirror wildcardSuperList(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s> %s;",
+        code.feature(GUAVA).isAvailable() ? ImmutableList.class : List.class,
+        elementType,
+        property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     if (code.feature(GUAVA).isAvailable()) {

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/MapProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/MapProperty.java
@@ -144,6 +144,15 @@ private static TypeMirror wildcardSuperMap(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s, %s> %s;",
+        (code.feature(GUAVA).isAvailable()) ? ImmutableMap.class : Map.class,
+        keyType,
+        valueType,
+        property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private final %1$s<%2$s, %3$s> %4$s = new %1$s<>();",

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/MultisetProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/MultisetProperty.java
@@ -153,6 +153,12 @@ private static TypeMirror multiset(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s> %s;",
+        ImmutableMultiset.class, elementType, property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private final %1$s<%2$s> %3$s = %1$s.create();",

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/NullableProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/NullableProperty.java
@@ -30,7 +30,6 @@
 import org.inferred.freebuilder.processor.Declarations;
 import org.inferred.freebuilder.processor.source.Excerpt;
 import org.inferred.freebuilder.processor.source.Excerpts;
-import org.inferred.freebuilder.processor.source.FieldAccess;
 import org.inferred.freebuilder.processor.source.FunctionalType;
 import org.inferred.freebuilder.processor.source.ObjectsExcerpts;
 import org.inferred.freebuilder.processor.source.SourceBuilder;
@@ -98,10 +97,16 @@ public Initially initialState() {
     return Initially.OPTIONAL;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    addGetterAnnotations(code);
+    code.add("private final %s %s;%n", property.getType(), property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     addGetterAnnotations(code);
-    code.add("private %s %s = null;\n", property.getType(), property.getField());
+    code.add("private %s %s = null;%n", property.getType(), property.getField());
   }
 
   @Override
@@ -122,7 +127,7 @@ private void addSetter(SourceBuilder code) {
     addAccessorAnnotations(code);
     code.add("public %s %s(", datatype.getBuilder(), setter(property));
     addGetterAnnotations(code);
-    code.add("%s %s) {\n", property.getType(), property.getName())
+    code.add("%s %s) {%n", property.getType(), property.getName())
         .addLine("  %s = %s;", property.getField(), property.getName())
         .addLine("  return (%s) this;", datatype.getBuilder())
         .addLine("}");
@@ -162,12 +167,6 @@ private void addGetter(SourceBuilder code) {
         .addLine("}");
   }
 
-  @Override
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
-    addGetterAnnotations(code);
-    code.add("private final %s %s;\n", property.getType(), finalField);
-  }
-
   @Override
   public void addFinalFieldAssignment(SourceBuilder code, Excerpt finalField, String builder) {
     code.addLine("%s = %s;", finalField, property.getField().on(builder));

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/OptionalProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/OptionalProperty.java
@@ -34,7 +34,6 @@
 import org.inferred.freebuilder.processor.Datatype;
 import org.inferred.freebuilder.processor.Declarations;
 import org.inferred.freebuilder.processor.source.Excerpt;
-import org.inferred.freebuilder.processor.source.FieldAccess;
 import org.inferred.freebuilder.processor.source.FunctionalType;
 import org.inferred.freebuilder.processor.source.QualifiedName;
 import org.inferred.freebuilder.processor.source.SourceBuilder;
@@ -196,11 +195,11 @@ public Initially initialState() {
   }
 
   @Override
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
+  public void addValueFieldDeclaration(SourceBuilder code) {
     code.addLine("// Store a nullable object instead of an Optional. Escape analysis then")
         .addLine("// allows the JVM to optimize away the Optional objects created by our")
         .addLine("// getter method.")
-        .addLine("private final %s %s;", elementType, finalField);
+        .addLine("private final %s %s;", elementType, property.getField());
   }
 
   @Override

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/PrimitiveOptionalProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/PrimitiveOptionalProperty.java
@@ -22,7 +22,6 @@
 import org.inferred.freebuilder.processor.Declarations;
 import org.inferred.freebuilder.processor.model.MethodIntrospector;
 import org.inferred.freebuilder.processor.source.Excerpt;
-import org.inferred.freebuilder.processor.source.FieldAccess;
 import org.inferred.freebuilder.processor.source.FunctionalType;
 import org.inferred.freebuilder.processor.source.QualifiedName;
 import org.inferred.freebuilder.processor.source.SourceBuilder;
@@ -160,8 +159,8 @@ public Initially initialState() {
   }
 
   @Override
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
-    code.addLine("private final %s %s;", optional.type, finalField);
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s %s;", optional.type, property.getField());
   }
 
   @Override

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/PropertyCodeGenerator.java b/src/main/java/org/inferred/freebuilder/processor/property/PropertyCodeGenerator.java
@@ -24,7 +24,6 @@
 
 import org.inferred.freebuilder.processor.Datatype;
 import org.inferred.freebuilder.processor.source.Excerpt;
-import org.inferred.freebuilder.processor.source.FieldAccess;
 import org.inferred.freebuilder.processor.source.SourceBuilder;
 import org.inferred.freebuilder.processor.source.Variable;
 
@@ -136,9 +135,7 @@ public Initially initialState() {
   }
 
   /** Add the field declaration for the property to the value's source code. */
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
-    code.addLine("private final %s %s;", property.getType(), finalField);
-  }
+  public abstract void addValueFieldDeclaration(SourceBuilder code);
 
   /** Add the field declaration for the property to the builder's source code. */
   public abstract void addBuilderFieldDeclaration(SourceBuilder code);

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/SetMultimapProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/SetMultimapProperty.java
@@ -142,6 +142,12 @@ private static TypeMirror setMultimap(
     this.mutatorType = mutatorType;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s, %s> %s;",
+        ImmutableSetMultimap.class, keyType, valueType, property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private final %1$s<%2$s, %3$s> %4$s = %1$s.create();",

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/SetProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/SetProperty.java
@@ -150,6 +150,14 @@ private static TypeMirror wildcardSuperSet(
     this.overridesVarargsAddMethod = overridesVarargsAddMethod;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s> %s;",
+          code.feature(GUAVA).isAvailable() ? ImmutableSet.class : Set.class,
+          elementType,
+          property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     if (code.feature(GUAVA).isAvailable()) {

diff --git a/src/main/java/org/inferred/freebuilder/processor/property/SortedSetProperty.java b/src/main/java/org/inferred/freebuilder/processor/property/SortedSetProperty.java
@@ -40,7 +40,6 @@
 import org.inferred.freebuilder.processor.Declarations;
 import org.inferred.freebuilder.processor.excerpt.CheckedNavigableSet;
 import org.inferred.freebuilder.processor.source.Excerpt;
-import org.inferred.freebuilder.processor.source.FieldAccess;
 import org.inferred.freebuilder.processor.source.FunctionalType;
 import org.inferred.freebuilder.processor.source.PreconditionExcerpts;
 import org.inferred.freebuilder.processor.source.SourceBuilder;
@@ -154,6 +153,14 @@ private static TypeMirror wildcardSuperSortedSet(
     this.overridesVarargsAddMethod = overridesVarargsAddMethod;
   }
 
+  @Override
+  public void addValueFieldDeclaration(SourceBuilder code) {
+    code.addLine("private final %s<%s> %s;",
+        code.feature(GUAVA).isAvailable() ? ImmutableSortedSet.class : SortedSet.class,
+        elementType,
+        property.getField());
+  }
+
   @Override
   public void addBuilderFieldDeclaration(SourceBuilder code) {
     code.addLine("private %s<%s> %s = null;", NavigableSet.class, elementType, property.getField());
@@ -449,15 +456,6 @@ private void addGetter(SourceBuilder code) {
         .addLine("}");
   }
 
-  @Override
-  public void addValueFieldDeclaration(SourceBuilder code, FieldAccess finalField) {
-    if (code.feature(GUAVA).isAvailable()) {
-      code.addLine("private final %s<%s> %s;", ImmutableSortedSet.class, elementType, finalField);
-    } else {
-      code.addLine("private final %s<%s> %s;", SortedSet.class, elementType, finalField);
-    }
-  }
-
   @Override
   public void addFinalFieldAssignment(SourceBuilder code, Excerpt finalField, String builder) {
     code.addLine("if (%s == null) {", property.getField().on(builder));

diff --git a/src/test/java/org/inferred/freebuilder/processor/property/ListSourceTest.java b/src/test/java/org/inferred/freebuilder/processor/property/ListSourceTest.java
@@ -747,8 +747,8 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Value extends Person {",
-        "    private final List<String> name;",
-        "    private final List<Integer> age;",
+        "    private final ImmutableList<String> name;",
+        "    private final ImmutableList<Integer> age;",
         "",
         "    private Value(Person_Builder builder) {",
         "      this.name = ImmutableList.copyOf(builder.name);",
@@ -786,8 +786,8 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Partial extends Person {",
-        "    private final List<String> name;",
-        "    private final List<Integer> age;",
+        "    private final ImmutableList<String> name;",
+        "    private final ImmutableList<Integer> age;",
         "",
         "    Partial(Person_Builder builder) {",
         "      this.name = ImmutableList.copyOf(builder.name);",

diff --git a/src/test/java/org/inferred/freebuilder/processor/property/MapPropertyTest.java b/src/test/java/org/inferred/freebuilder/processor/property/MapPropertyTest.java
@@ -533,7 +533,6 @@ public void testJacksonInteroperability() {
             .addLine("    .build();")
             .addLine("%1$s mapper = new %1$s();", ObjectMapper.class)
             .addLine("String json = mapper.writeValueAsString(value);")
-            .addLine("%s.out.println(json);", System.class)
             .addLine("DataType clone = mapper.readValue(json, DataType.class);")
             .addLine("assertThat(clone.%s).isEqualTo(%s);",
                 convention.get(), exampleMap(0, 0, 1, 1))

diff --git a/src/test/java/org/inferred/freebuilder/processor/property/MapSourceTest.java b/src/test/java/org/inferred/freebuilder/processor/property/MapSourceTest.java
@@ -440,7 +440,7 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Value extends Person {",
-        "    private final Map<Integer, String> name;",
+        "    private final ImmutableMap<Integer, String> name;",
         "",
         "    private Value(Person_Builder builder) {",
         "      this.name = ImmutableMap.copyOf(builder.name);",
@@ -472,7 +472,7 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Partial extends Person {",
-        "    private final Map<Integer, String> name;",
+        "    private final ImmutableMap<Integer, String> name;",
         "",
         "    Partial(Person_Builder builder) {",
         "      this.name = ImmutableMap.copyOf(builder.name);",

diff --git a/src/test/java/org/inferred/freebuilder/processor/property/SetSourceTest.java b/src/test/java/org/inferred/freebuilder/processor/property/SetSourceTest.java
@@ -532,7 +532,7 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Value extends Person {",
-        "    private final Set<String> name;",
+        "    private final ImmutableSet<String> name;",
         "",
         "    private Value(Person_Builder builder) {",
         "      this.name = ImmutableSet.copyOf(builder.name);",
@@ -564,7 +564,7 @@ public void test_guava() {
         "  }",
         "",
         "  private static final class Partial extends Person {",
-        "    private final Set<String> name;",
+        "    private final ImmutableSet<String> name;",
         "",
         "    Partial(Person_Builder builder) {",
         "      this.name = ImmutableSet.copyOf(builder.name);",