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

mergeFrom() on supertypes and their builders #220

Closed
wants to merge 3 commits into from
+743 −17
Closed

mergeFrom() on supertypes and their builders #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa6f182
mergeFrom() added for super types and super type builders
ph4r05 Dec 8, 2016
b1c924e
mergeFrom() super type builder works across the packages
ph4r05 Dec 8, 2016
f592eb8
property factory fixes (mergeFromSuperBuilder)
ph4r05 Dec 8, 2016
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
82 changes: 82 additions & 0 deletions src/main/java/org/inferred/freebuilder/processor/Analyser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import static org.inferred.freebuilder.processor.util.ModelUtils.getReturnType;
import static org.inferred.freebuilder.processor.util.ModelUtils.maybeAsTypeElement;
import static org.inferred.freebuilder.processor.util.ModelUtils.maybeType;
import static org.inferred.freebuilder.processor.util.ModelUtils.findAnnotationMirror;

import com.google.common.base.Optional;
import com.google.common.base.Predicate;
Expand All @@ -43,6 +44,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;

import org.inferred.freebuilder.FreeBuilder;
import org.inferred.freebuilder.processor.Metadata.Property;
import org.inferred.freebuilder.processor.Metadata.StandardMethod;
import org.inferred.freebuilder.processor.Metadata.UnderrideLevel;
Expand All @@ -53,6 +55,8 @@

import java.io.Serializable;
import java.util.LinkedHashMap;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -166,10 +170,88 @@ Metadata analyse(TypeElement type) throws CannotGenerateCodeException {
metadataBuilder
.clearProperties()
.addAllProperties(codeGenerators(properties, baseMetadata, builder.get()));

metadataBuilder.addAllSuperBuilderTypes(superBuilders(type));
metadataBuilder.putAllSuperTypeProperties(
processSuperTypeProperties(type, baseMetadata, builder));
}
return metadataBuilder.build();
}

private ImmutableMap<ParameterizedType, ImmutableList<Property>> processSuperTypeProperties(
TypeElement type,
Metadata baseMetadata,
Optional<TypeElement> builder) throws CannotGenerateCodeException {
Map<ParameterizedType, ImmutableList<Property>> toRet =
new HashMap<ParameterizedType, ImmutableList<Property>>();

final ImmutableSet<TypeElement> superTypes = MethodFinder.getSupertypes(type);
for (TypeElement superType : superTypes) {
if (superType.equals(type)) {
continue;
}

final ImmutableSet<ExecutableElement> superMethods = methodsOn(superType, elements);
final Map<ExecutableElement, Property> superPropertiesRet =
findProperties(superType, superMethods);
if (superPropertiesRet.isEmpty()) {
continue;
}

ParameterizedType pType = QualifiedName.of(superType).withParameters(
superType.getTypeParameters());

// Code builder dance
if (builder.isPresent()) {
final Metadata metadataSuperType = analyse(superType);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've already figured out what properties are implied by what getter methods; we should just check to see which of those methods are declared abstract on this supertype, rather than regenerating everything.

Also, you can't call analyse on non-FreeBuilder-annotated types, as they shouldn't be expected to follow FreeBuilder's rules about abstract methods.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and covariant return types are going to cause issues. You can either identify those occasions where runtime type checking is going to be safe (not forgetting optionals, collections, etc. can typecheck members), or bail out entirely if the return type doesn't exactly match what you'd expect. I'd recommend the latter, especially as a first PR.

final Metadata.Builder metadataBld = Metadata.Builder.from(metadataSuperType);
metadataBld.setBuilderFactory(Optional.<BuilderFactory>absent());

for (Map.Entry<ExecutableElement, Property> entry : superPropertiesRet.entrySet()) {
Config config = new ConfigImpl(
builder.get(),
metadataBld.build(),
entry.getValue(),
entry.getKey(),
ImmutableSet.<String>of());

entry.setValue(new Property.Builder()
.mergeFrom(entry.getValue())
.setCodeGenerator(createCodeGenerator(config))
.build());
}
}

toRet.put(pType, ImmutableList.copyOf(superPropertiesRet.values()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to generate a merge method for every single supertype, only those which declare a getter method.

}

return ImmutableMap.copyOf(toRet);
}

private ImmutableSet<ParameterizedType> superBuilders(TypeElement type)
throws CannotGenerateCodeException {
Set<ParameterizedType> toRet = new HashSet<ParameterizedType>();
PackageElement pkg = elements.getPackageOf(type);

final ImmutableSet<TypeElement> superTypes = MethodFinder.getSupertypes(type);
for (TypeElement superType : superTypes) {
if (superType.equals(type)) {
continue;
}

final Optional<AnnotationMirror> freeBuilderMirror =
findAnnotationMirror(superType, FreeBuilder.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work: the FreeBuilder annotation is source-only. If your two types are not compiled together, or if your IDE decides to compile the two classes separately (e.g. you change your subtype in Eclipse), you'll be given the class definition from the compiled JAR. Check out the logic in BuildablePropertyFactory, for example. In this case, however, you'll need to check for the presence of all the getter methods too, since a user might have generated a broken builder by hand.


if (freeBuilderMirror.isPresent()) {
ParameterizedType pType = QualifiedName.of(superType).withParameters(
superType.getTypeParameters());
toRet.add(pType);
}
}

return ImmutableSet.copyOf(toRet);
}

private static Set<QualifiedName> visibleTypesIn(TypeElement type) {
ImmutableSet.Builder<QualifiedName> visibleTypes = ImmutableSet.builder();
for (TypeElement nestedType : typesIn(type.getEnclosedElements())) {
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/inferred/freebuilder/processor/BuildablePropertyFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,11 @@ public void addMergeFromValue(Block code, String value) {
code.addLine("%s.mergeFrom(%s.%s());", propertyName, value, property.getGetterName());
}

@Override
public void addMergeFromSuperValue(Block code, String value) {
addMergeFromValue(code, value);
}

@Override
public void addMergeFromBuilder(Block code, String builder) {
String propertyName = property.getName();
Expand All @@ -285,6 +290,11 @@ public void addMergeFromBuilder(Block code, String builder) {
code.add(");\n");
}

@Override
public void addMergeFromSuperBuilder(Block code, String builder) {
addMergeFromBuilder(code, builder);
}

@Override
public void addSetFromResult(SourceBuilder code, String builder, String variable) {
code.addLine("%s.%s(%s);", builder, setter(property), variable);
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/inferred/freebuilder/processor/BuilderMethods.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,9 @@ public static String clearMethod(Property property) {
return "clear" + property.getCapitalizedName();
}

public static String isPropertySetMethod(Property property) {
return "isProperty" + property.getCapitalizedName() + "Set";
}

private BuilderMethods() {}
}
75 changes: 75 additions & 0 deletions src/main/java/org/inferred/freebuilder/processor/CodeGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.collect.Iterables.any;
import static com.google.common.collect.Iterables.getLast;
import static com.google.common.collect.Iterables.getOnlyElement;
import static org.inferred.freebuilder.processor.BuilderMethods.isPropertySetMethod;
import static org.inferred.freebuilder.processor.BuilderFactory.TypeInference.EXPLICIT_TYPES;
import static org.inferred.freebuilder.processor.Metadata.GET_CODE_GENERATOR;
import static org.inferred.freebuilder.processor.Metadata.UnderrideLevel.ABSENT;
Expand All @@ -43,11 +44,14 @@
import org.inferred.freebuilder.processor.util.Excerpts;
import org.inferred.freebuilder.processor.util.PreconditionExcerpts;
import org.inferred.freebuilder.processor.util.SourceBuilder;
import org.inferred.freebuilder.processor.util.ParameterizedType;
import org.inferred.freebuilder.processor.util.QualifiedName;

import java.io.Serializable;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
import java.util.TreeSet;

Expand Down Expand Up @@ -76,7 +80,9 @@ void writeBuilderSource(SourceBuilder code, Metadata metadata) {
addAccessors(metadata, code);
addMergeFromValueMethod(code, metadata);
addMergeFromBuilderMethod(code, metadata);
addMergeFromSuperTypes(code, metadata);
addClearMethod(code, metadata);
addPropertiesSetMethods(code, metadata);
addBuildMethod(code, metadata);
addBuildPartialMethod(code, metadata);

Expand Down Expand Up @@ -203,6 +209,51 @@ private static void addMergeFromBuilderMethod(SourceBuilder code, Metadata metad
.addLine("}");
}

private static void addMergeFromSuperTypes(SourceBuilder code, Metadata metadata) {
for (Map.Entry<ParameterizedType, ImmutableList<Property>> e
: metadata.getSuperTypeProperties().entrySet()) {
final ParameterizedType type = e.getKey();
final ImmutableList<Property> properties = e.getValue();

// mergeFrom - value
code.addLine("")
.addLine("/**")
.addLine(" * Sets all property values using the given {@code %s} as a template.",
type.getQualifiedName())
.addLine(" */")
.addLine("public %s mergeFrom(%s value) {",
metadata.getBuilder(), type.getQualifiedName());
Block body = new Block(code);
for (Property property : properties) {
property.getCodeGenerator().addMergeFromSuperValue(body, "value");
}
code.add(body)
.addLine(" return (%s) this;", metadata.getBuilder())
.addLine("}");

// has builder ?
if (!metadata.getSuperBuilderTypes().contains(type)) {
continue;
}

// mergeFrom - builder
final QualifiedName builder = type.getQualifiedName().nestedType("Builder");
code.addLine("")
.addLine("/**")
.addLine(" * Copies values from the given {@code %s}.", builder.getSimpleName())
.addLine(" * Does not affect any properties not set on the input.")
.addLine(" */")
.addLine("public %s mergeFrom(%s template) {", metadata.getBuilder(), builder);
Block fromBuilderBody = new Block(code);
for (Property property : properties) {
property.getCodeGenerator().addMergeFromSuperBuilder(fromBuilderBody, "template");
}
code.add(fromBuilderBody)
.addLine(" return (%s) this;", metadata.getBuilder())
.addLine("}");
}
}

private static void addClearMethod(SourceBuilder code, Metadata metadata) {
code.addLine("")
.addLine("/**")
Expand All @@ -227,6 +278,30 @@ private static void addClearMethod(SourceBuilder code, Metadata metadata) {
.addLine("}");
}

private static void addPropertiesSetMethods(SourceBuilder code, Metadata metadata) {
if (!any(metadata.getProperties(), IS_REQUIRED)) {
return;

}

for (Property property : metadata.getProperties()) {
if (!IS_REQUIRED.apply(property)) {
continue;
}

code.addLine("")
.addLine("/**")
.addLine(" * Returns true if the required property corresponding to")
.addLine(" * %s is set. ", metadata.getType().javadocNoArgMethodLink(
property.getGetterName()))
.addLine(" */")
.addLine("public boolean %s() {", isPropertySetMethod(property))
.addLine(" return _unsetProperties.contains(%s.%s);",
metadata.getPropertyEnum(), property.getAllCapsName())
.addLine("}");
}
}

private static void addBuildPartialMethod(SourceBuilder code, Metadata metadata) {
code.addLine("")
.addLine("/**")
Expand Down
41 changes: 33 additions & 8 deletions src/main/java/org/inferred/freebuilder/processor/DefaultPropertyFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.inferred.freebuilder.processor.BuilderMethods.getter;
import static org.inferred.freebuilder.processor.BuilderMethods.mapper;
import static org.inferred.freebuilder.processor.BuilderMethods.setter;
import static org.inferred.freebuilder.processor.BuilderMethods.isPropertySetMethod;
import static org.inferred.freebuilder.processor.util.PreconditionExcerpts.checkNotNullInline;
import static org.inferred.freebuilder.processor.util.PreconditionExcerpts.checkNotNullPreamble;
import static org.inferred.freebuilder.processor.util.feature.FunctionPackage.FUNCTION_PACKAGE;
Expand Down Expand Up @@ -191,18 +192,42 @@ public void addMergeFromValue(Block code, String value) {
}
}

@Override
public void addMergeFromSuperValue(Block code, String value) {
addMergeFromValue(code, value);
}

@Override
public void addMergeFromBuilder(Block code, String builder) {
Excerpt base =
hasDefault ? null : Declarations.upcastToGeneratedBuilder(code, metadata, builder);
addMergeFromBuilder(code, builder, false);
}

@Override
public void addMergeFromSuperBuilder(Block code, String builder) {
addMergeFromBuilder(code, builder, true);
}

public void addMergeFromBuilder(Block code, String builder, boolean fromSuper) {
Excerpt base = hasDefault || fromSuper ? null : Declarations.upcastToGeneratedBuilder(
code, metadata, builder);
Excerpt defaults = Declarations.freshBuilder(code, metadata).orNull();
Block unsetContains = new Block(code);
String isPropertySetPositive = fromSuper ? "" : "!";
String isPropertySetNegative = fromSuper ? "!" : "";
if (fromSuper) {
unsetContains.add("%s()", isPropertySetMethod(property));
base = new Block(code).add(builder);
} else {
unsetContains.add("_unsetProperties.contains(%s.%s)",
metadata.getPropertyEnum(), property.getAllCapsName());
}
if (defaults != null) {
code.add("if (");
if (!hasDefault) {
code.add("!%s._unsetProperties.contains(%s.%s) && ",
base, metadata.getPropertyEnum(), property.getAllCapsName())
.add("(%s._unsetProperties.contains(%s.%s) ||",
defaults, metadata.getPropertyEnum(), property.getAllCapsName());
code.add("%s%s.%s && ",
isPropertySetPositive, base, unsetContains)
.add("(%s%s.%s ||",
isPropertySetNegative, defaults, unsetContains);
}
if (isPrimitive) {
code.add("%1$s.%2$s() != %3$s.%2$s()", builder, getter(property), defaults);
Expand All @@ -214,8 +239,8 @@ public void addMergeFromBuilder(Block code, String builder) {
}
code.add(") {%n");
} else if (!hasDefault) {
code.addLine("if (!%s._unsetProperties.contains(%s.%s)) {",
base, metadata.getPropertyEnum(), property.getAllCapsName());
code.addLine("if (%s%s.%s) {",
isPropertySetPositive, base, unsetContains);
}
code.addLine(" %s(%s.%s());", setter(property), builder, getter(property));
if (defaults != null || !hasDefault) {
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/org/inferred/freebuilder/processor/ListMultimapPropertyFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,11 @@ public void addMergeFromValue(Block code, String value) {
code.addLine("%s(%s.%s());", putAllMethod(property), value, property.getGetterName());
}

@Override
public void addMergeFromSuperValue(Block code, String value) {
addMergeFromValue(code, value);
}

@Override
public void addMergeFromBuilder(Block code, String builder) {
code.addLine("%s(((%s) %s).%s);",
Expand All @@ -382,6 +387,14 @@ public void addMergeFromBuilder(Block code, String builder) {
property.getName());
}

@Override
public void addMergeFromSuperBuilder(Block code, String builder) {
code.addLine("%s(%s.%s());",
putAllMethod(property),
builder,
getter(property));
}

@Override
public void addSetFromResult(SourceBuilder code, String builder, String variable) {
code.addLine("%s.%s(%s);", builder, putAllMethod(property), variable);
Expand Down
Loading