Refinements to the singleton logic. There was to many small inconsist…

…encies. Need Eugene to review

value-fixture/src/org/immutables/fixture/Singletons.java

@@ -0,0 +1,68 @@
+package org.immutables.fixture;
+
+import java.util.Collections;
+import java.util.List;
+import org.immutables.value.Value;
+
+// Compilation tests for singletons/builders
+
+@Value.Immutable(singleton = true)
+interface Sing1 {
+  List<Integer> list();
+
+  default void use() {
+    ImmutableSing1.builder();
+    ImmutableSing1.of()
+        .withList(Collections.emptyList());
+  }
+}
+
+@Value.Immutable(builder = false)
+interface Sing2 {
+  @Value.Default
+  default int a() {
+    return 1;
+  }
+
+  default void use() {
+    ImmutableSing2.of().withA(1);
+  }
+}
+
+@Value.Immutable(builder = false)
+interface Sing3 {
+  default void use() {
+    ImmutableSing3.of();
+  }
+}
+
+@Value.Immutable(singleton = true)
+interface Sing4 {
+
+  default void use() {
+    ImmutableSing4.builder();
+    ImmutableSing4.of();
+  }
+}
+
+@Value.Immutable(builder = false, singleton = true)
+interface Sing5 {
+  @Value.Parameter
+  @Value.Default
+  default int a() {
+    return 1;
+  }
+
+  default void use() {
+    ImmutableSing5.of();
+    ImmutableSing5.of(1);
+  }
+}
+
+@Value.Immutable
+interface Sing6 {
+  default void use() {
+    ImmutableSing6.builder();
+    ImmutableSing6.of();
+  }
+}

value-fixture/src/org/immutables/fixture/jackson/NoBuilderDeserialize.java

@@ -7,6 +7,7 @@
 @Value.Immutable(
     singleton = true,
     builder = false)
+@Value.Style(instance = "instance")
 @JsonDeserialize(as = ImmutableNoBuilderDeserialize.class)
 public abstract class NoBuilderDeserialize {
   public abstract List<String> prop();

value-fixture/src/org/immutables/fixture/style/IncludeTypes.java

@@ -34,9 +34,9 @@ public class IncludeTypes {
 
   void use() {
     // this immutable type (package style used)
-    ImIncludeTypes.of();
+    ImIncludeTypes.builder().build();
     // included on this type (package style used)
-    ImSerializable.of();
+    ImSerializable.builder().build();
     // included on package (package style used)
     ImTicker.builder().read(1).build();
 

value-processor/src/org/immutables/value/processor/Immutables.generator

@@ -236,11 +236,11 @@ private static class SerialForm implements java.io.Serializable {
           builder.[v.names.add](([v.wrappedElementType]) e);
         }
       [else if v.mapType]
-        [atNullable]Object[arr] pairs = (Object[arr]) values[arri];
-        if (pairs != null) {
-          for (int j = 0; j < pairs.length; j += 2) {
+        [atNullable]Object[arr] entries = (Object[arr]) values[arri];
+        if (entries != null) {
+          for (int j = 0; j < entries.length; j += 2) {
           [for String j = '[j]', String j1 = '[j + 1]']
-            builder.[v.names.put](([v.wrappedElementType]) pairs[j], ([v.wrappedSecondaryElementType]) pairs[j1]);
+            builder.[v.names.put](([v.wrappedElementType]) entries[j], ([v.wrappedSecondaryElementType]) entries[j1]);
           [/for]
           }
         }
@@ -284,11 +284,11 @@ private static class SerialForm implements java.io.Serializable {
           [v.name]Builder.add(([v.wrappedElementType]) e);
         }
       [else if v.mapType]
-        Object[arr] pairs = (Object[arr]) values[arri];
-        if (pairs != null) {
-          for (int j = 0; j < pairs.length; j += 2) {
+        Object[arr] entries = (Object[arr]) values[arri];
+        if (entries != null) {
+          for (int j = 0; j < entries.length; j += 2) {
           [for String j = '[j]', String j1 = '[j + 1]']
-            [v.name]Builder.put(([v.wrappedElementType]) pairs[j], ([v.wrappedSecondaryElementType]) pairs[j1]);
+            [v.name]Builder.put(([v.wrappedElementType]) entries[j], ([v.wrappedSecondaryElementType]) entries[j1]);
           [/for]
           }
         }
@@ -306,19 +306,23 @@ private static class SerialForm implements java.io.Serializable {
       default:
       }
     }
+  [if type.useConstructor]
     [type.typeValue.relative] instance = [type.factoryOf.relative]([output.linesShortable][for v in type.constructorArguments][if not for.first],[/if]
         [if v.collectionType or v.mapType][createBuiltCollection type v][v.name]Builder[/createBuiltCollection][else][v.name]Value[/if][/for]);[/output.linesShortable]
-    [if type.useCopyMethods]
-      [for o in type.constructorOmited if o.generateDefault or o.generateAbstract]
-        [if o.collectionType or o.mapType]
+  [else if type.useSingleton]
+    [type.typeValue.relative] instance = [type.factoryInstance.relative]();
+  [else]
+    [output.error]Cannot generate code when there are no builder, constructor or singleton available[/output.error]
+  [/if]
+    [for o in type.withSettableAfterConstruction]
+      [if o.collectionType or o.mapType]
     instance = instance.[o.names.with]([createBuiltCollection type o][o.name]Builder[/createBuiltCollection]);
-        [else]
+      [else]
     if ([o.name]Value != null) {
       instance = instance.[o.names.with]([o.name]Value);
     }
-        [/if]
-      [/for]
-    [/if]
+      [/if]
+    [/for]
     return instance;
   [/for]
 [/if]
@@ -555,9 +559,9 @@ private Object writeReplace() {
     [/if]
       .[type.names.build]();
 [else]
-  return [type.factoryOf.relative]([for v in type.constructorArguments][if not for.first], [/if]instance.[v.names.get]()[/for])[if type.useCopyMethods][for
-          o in type.constructorOmited if o.generateDefault or o.generateAbstract]
-      .[o.names.with](instance.[o.names.get]())[/for][/if];
+  return [type.factoryOf.relative]([for v in type.constructorArguments][if not for.first], [/if]instance.[v.names.get]()[/for])[for
+          o in type.withSettableAfterConstruction]
+      .[o.names.with](instance.[o.names.get]())[/for];
 [/if]
 }
 [/if]
@@ -1049,7 +1053,7 @@ initializedBitset[emptyIfZero pos.index] |= INITIALIZED_BIT_[toConstant v.name];
   [/if]
   [if type.kind.isFactory]
     [returnFactoryBuild type]
-  [else if type.useSingletonOnly]
+  [else if type.useSingleton and (not type.settableAttributes)]
     return [type.factoryInstance.relative]();
   [else]
     return [validated type true]new [type.typeImmutable.relative](this)[/validated];
@@ -1225,7 +1229,7 @@ this.[n] = [if type.generateJdkOnly][objectsUtilRef type]requireNonNull[else][gu
     [generateAfterConstruction type false]
   }
 [/if]
-[if type.useBuilder and (not type.useSingletonOnly)]
+[if type.useBuilder and (not (type.useSingleton and (not type.settableAttributes)))]
 
   private [type.typeImmutable.simple]([type.typeBuilder.relative] builder) {
 [for v in getters if not v.generateDerived, n = v.name]
@@ -1321,9 +1325,7 @@ public static [type.typeValue.relative] fromAllAttributes(
     [a]
     [/for]
     @com.fasterxml.jackson.annotation.JsonProperty("[v.marshaledName]") [atNullable][v.wrapperType] [v.name][/for]) {
-[if type.useSingletonOnly]
-  return [type.factoryInstance.relative]();
-[else if type.useBuilder]
+[if type.useBuilder]
   [let builderIdentifier]builder[for v in type.settableAttributes if v.name eq 'builder']$$[/for][/let]
   [type.typeBuilder.relative] [builderIdentifier] = [type.factoryBuilder.relative]();
   [for v in type.settableAttributes]
@@ -1338,16 +1340,22 @@ public static [type.typeValue.relative] fromAllAttributes(
   }
   [/for]
   return [builderIdentifier].[type.names.build]();
+[else if type.useSingletonOnly]
+  return [type.factoryInstance.relative]();
 [else]
   [let instanceIdentifier]instance[for v in type.settableAttributes if v.name eq 'instance']$$[/for][/let]
+    [if type.useConstructor]
   [type.typeValue.relative] [instanceIdentifier] = [type.factoryOf.relative]([for v in type.constructorArguments][if not for.first], [/if][v.name][/for]);
-  [if type.useCopyMethods]
-    [for o in type.constructorOmited if o.generateDefault or o.generateAbstract]
+    [else if type.useSingleton]
+  [type.typeValue.relative] [instanceIdentifier] = [type.factoryInstance.relative]();
+    [else]
+  [output.error]Cannot generate code when there are no builder, constructor or singleton available[/output.error]
+    [/if]
+  [for o in type.withSettableAfterConstruction]
   if ([o.name] != null) {
     [instanceIdentifier] = [instanceIdentifier].[o.names.with]([o.name]);
   }
-    [/for]
-  [/if]
+  [/for]
   return [instanceIdentifier];
 [/if]
 }

value-processor/src/org/immutables/value/processor/meta/ValueType.java

@@ -299,7 +299,9 @@ public boolean isUseCopyConstructor() {
   }
 
   public boolean isUseSingleton() {
-    return immutableFeatures.singleton() || getImplementedAttributes().isEmpty();
+    return immutableFeatures.singleton()
+        || useSingletonNoOtherWay()
+        || useSingletonForConvenience();
   }
 
   public boolean isUseInterned() {
@@ -399,7 +401,19 @@ public boolean isUseEqualTo() {
 
   public boolean isUseSingletonOnly() {
     return isUseSingleton()
-        && getImplementedAttributes().isEmpty();
+        && !isUseBuilder()
+        && !isUseConstructor()
+        && getWithSettableAfterConstruction().isEmpty();
+  }
+
+  private boolean useSingletonForConvenience() {
+    return getSettableAttributes().isEmpty();
+  }
+
+  private boolean useSingletonNoOtherWay() {
+    return !isUseBuilder()
+        && !isUseConstructor()
+        && getMandatoryAttributes().isEmpty();
   }
 
   public boolean isUseConstructor() {
@@ -416,34 +430,45 @@ public boolean requiresAlternativeStrictConstructor() {
   }
 
   @Nullable
-  private List<ValueAttribute> constructorArguments;
+  private Set<ValueAttribute> constructorArguments;
 
-  public List<ValueAttribute> getConstructorArguments() {
+  public Set<ValueAttribute> getConstructorArguments() {
     if (constructorArguments == null) {
       constructorArguments = computeConstructorArguments();
-      validateConstructorParameters(constructorArguments);
       if (constructorArguments.isEmpty() && constitution.style().allParameters()) {
-        constructorArguments = getSettableAttributes();
+        constructorArguments = ImmutableSet.copyOf(getSettableAttributes());
       }
+      validateConstructorParameters(constructorArguments);
     }
     return constructorArguments;
   }
 
-  public List<ValueAttribute> getConstructableAttributes() {
-    if (!isUseCopyMethods()) {
-      return getConstructorArguments();
+  public List<ValueAttribute> getWithSettableAfterConstruction() {
+    if (isUseCopyMethods()) {
+      return getConstructorExcluded();
     }
-    List<ValueAttribute> attributes = Lists.newArrayList(getConstructorArguments());
-    for (ValueAttribute v : getConstructorOmited()) {
-      if (v.isGenerateDefault || v.isGenerateAbstract) {
-        attributes.add(v);
-      }
+    return ImmutableList.of();
+  }
+
+  @Nullable
+  private List<ValueAttribute> constructorExcluded;
+
+  public List<ValueAttribute> getConstructorExcluded() {
+    if (constructorExcluded == null) {
+      constructorExcluded = FluentIterable.from(getSettableAttributes())
+          .filter(Predicates.not(Predicates.in(getConstructorArguments())))
+          .toList();
     }
-    return attributes;
+    return constructorExcluded;
+  }
 
+  public List<ValueAttribute> getConstructableAttributes() {
+    List<ValueAttribute> attributes = Lists.newArrayList(getConstructorArguments());
+    attributes.addAll(getWithSettableAfterConstruction());
+    return attributes;
   }
 
-  private void validateConstructorParameters(List<ValueAttribute> parameters) {
+  private void validateConstructorParameters(Set<ValueAttribute> parameters) {
     if (kind().isValue() && !parameters.isEmpty()) {
       Set<Element> definingElements = Sets.newHashSet();
       for (ValueAttribute attribute : parameters) {
@@ -460,23 +485,16 @@ private void validateConstructorParameters(List<ValueAttribute> parameters) {
     }
   }
 
-  public List<ValueAttribute> computeConstructorArguments() {
-    return attributes()
+  public Set<ValueAttribute> computeConstructorArguments() {
+    return ImmutableSet.copyOf(attributes()
         .filter(Predicates.compose(Predicates.not(Predicates.equalTo(-1)), ToConstructorArgumentOrder.FUNCTION))
-        .toSortedList(Ordering.natural().onResultOf(ToConstructorArgumentOrder.FUNCTION));
+        .toSortedList(Ordering.natural().onResultOf(ToConstructorArgumentOrder.FUNCTION)));
   }
 
-  @Nullable
-  private List<ValueAttribute> constructorOmmited;
-
   public List<ValueAttribute> getConstructorOmited() {
-    if (constructorOmmited == null) {
-      Set<ValueAttribute> constructorArgumentsSet = Sets.newHashSet(getConstructorArguments());
-      constructorOmmited = FluentIterable.from(getImplementedAttributes())
-          .filter(Predicates.not(Predicates.in(constructorArgumentsSet)))
-          .toList();
-    }
-    return constructorOmmited;
+    return FluentIterable.from(getImplementedAttributes())
+        .filter(Predicates.not(Predicates.in(getConstructorArguments())))
+        .toList();
   }
 
   private enum NonAuxiliary implements Predicate<ValueAttribute> {
@@ -792,8 +810,14 @@ ImmutableSet<DeclaredType> implementedInterfaces() {
     return hierarchiCollector.implementedInterfaces();
   }
 
+  @Nullable
+  private Boolean generateBuilderFrom;
+
   public boolean isGenerateBuilderFrom() {
-    return !isUseStrictBuilder() && noAttributeInitializerIsNamedAsFrom();
+    if (generateBuilderFrom == null) {
+      generateBuilderFrom = !isUseStrictBuilder() && noAttributeInitializerIsNamedAsFrom();
+    }
+    return generateBuilderFrom;
   }
 
   private boolean noAttributeInitializerIsNamedAsFrom() {