Skip to content

Commit

Permalink
* Fixed issues with @FieldDefaults and @value (you can NOT override
Browse files Browse the repository at this point in the history
@value's final-by-default and private-by-default with it; now
appropriate warnings are emitted)
* Builder now errors out on presence of most lombok annotations on an
explicit builder class.
* Builder now takes @FieldDefaults/@value into account.
* Builder on type now generates the constructor as package private
instead of private to avoid synthetic accessor constructors.
* added a bunch of test cases.
* added a test case feature: If the expected file is omitted entirely
but there are expected messages, the differences in the output itself
are ignored.
* streamlined checking for boolean-ness (removed some duplicate code)
* added 'fluent' and 'chain' to @builder.
  • Loading branch information
rzwitserloot committed Jul 15, 2013
1 parent ec0cc43 commit 7af9add
Show file tree
Hide file tree
Showing 23 changed files with 290 additions and 48 deletions.
25 changes: 22 additions & 3 deletions src/core/lombok/core/TransformationsUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,25 @@
package lombok.core;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.RequiredArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import lombok.Value;
import lombok.experimental.Accessors;
import lombok.experimental.FieldDefaults;
import lombok.experimental.Wither;

/**
* Container for static utility methods useful for some of the standard lombok transformations, regardless of
Expand All @@ -39,6 +51,13 @@ private TransformationsUtil() {
//Prevent instantiation
}

@SuppressWarnings({"all", "unchecked", "deprecation"})
public static final List<Class<? extends java.lang.annotation.Annotation>> INVALID_ON_BUILDERS = Collections.unmodifiableList(
Arrays.<Class<? extends java.lang.annotation.Annotation>>asList(
Getter.class, Setter.class, Wither.class, ToString.class, EqualsAndHashCode.class,
RequiredArgsConstructor.class, AllArgsConstructor.class, NoArgsConstructor.class,
Data.class, Value.class, lombok.experimental.Value.class, FieldDefaults.class));

/**
* Given the name of a field, return the 'base name' of that field. For example, {@code fFoobar} becomes {@code foobar} if {@code f} is in the prefix list.
* For prefixes that end in a letter character, the next character must be a non-lowercase character (i.e. {@code hashCode} is not {@code ashCode} even if
Expand Down Expand Up @@ -159,12 +178,12 @@ private static String toAccessorName(AnnotationValues<Accessors> accessors, Char

if (fieldName.length() == 0) return null;

Accessors ac = accessors.getInstance();
fieldName = removePrefix(fieldName, ac.prefix());
Accessors ac = accessors == null ? null : accessors.getInstance();
fieldName = removePrefix(fieldName, ac == null ? new String[0] : ac.prefix());
if (fieldName == null) return null;

String fName = fieldName.toString();
if (adhereToFluent && ac.fluent()) return fName;
if (adhereToFluent && ac != null && ac.fluent()) return fName;

if (isBoolean && fName.startsWith("is") && fieldName.length() > 2 && !Character.isLowerCase(fieldName.charAt(2))) {
// The field is for example named 'isRunning'.
Expand Down
35 changes: 32 additions & 3 deletions src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package lombok.eclipse.handlers;

import static lombok.eclipse.Eclipse.*;
import static lombok.core.TransformationsUtil.*;

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -296,6 +297,29 @@ public static boolean typeMatches(Class<?> type, EclipseNode node, TypeReference

}

public static void sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(EclipseNode typeNode, EclipseNode errorNode) {
List<String> disallowed = null;
for (EclipseNode child : typeNode.down()) {
for (Class<? extends java.lang.annotation.Annotation> annType : INVALID_ON_BUILDERS) {
if (annotationTypeMatches(annType, child)) {
if (disallowed == null) disallowed = new ArrayList<String>();
disallowed.add(annType.getSimpleName());
}
}
}

int size = disallowed == null ? 0 : disallowed.size();
if (size == 0) return;
if (size == 1) {
errorNode.addError("@" + disallowed.get(0) + " is not allowed on builder classes.");
return;
}
StringBuilder out = new StringBuilder();
for (String a : disallowed) out.append("@").append(a).append(", ");
out.setLength(out.length() - 2);
errorNode.addError(out.append(" are not allowed on builder classes.").toString());
}

public static Annotation copyAnnotation(Annotation annotation, ASTNode source) {
int pS = source.sourceStart, pE = source.sourceEnd;

Expand Down Expand Up @@ -845,15 +869,20 @@ private static class GetterMethod {
private static final Object MARKER = new Object();

static void registerCreatedLazyGetter(FieldDeclaration field, char[] methodName, TypeReference returnType) {
if (!nameEquals(returnType.getTypeName(), "boolean") || returnType.dimensions() > 0) return;
generatedLazyGettersWithPrimitiveBoolean.put(field, MARKER);
if (isBoolean(returnType)) {
generatedLazyGettersWithPrimitiveBoolean.put(field, MARKER);
}
}

public static boolean isBoolean(TypeReference typeReference) {
return nameEquals(typeReference.getTypeName(), "boolean") && typeReference.dimensions() == 0;
}

private static GetterMethod findGetter(EclipseNode field) {
FieldDeclaration fieldDeclaration = (FieldDeclaration) field.get();
boolean forceBool = generatedLazyGettersWithPrimitiveBoolean.containsKey(fieldDeclaration);
TypeReference fieldType = fieldDeclaration.type;
boolean isBoolean = forceBool || (nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0);
boolean isBoolean = forceBool || isBoolean(fieldType);

EclipseNode typeNode = field.up();
for (String potentialGetterName : toAllGetterNames(field, isBoolean)) {
Expand Down
30 changes: 25 additions & 5 deletions src/core/lombok/eclipse/handlers/HandleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,17 @@
import lombok.AccessLevel;
import lombok.core.AST.Kind;
import lombok.core.AnnotationValues;
import lombok.core.HandlerPriority;
import lombok.core.TransformationsUtil;
import lombok.eclipse.Eclipse;
import lombok.eclipse.EclipseAnnotationHandler;
import lombok.eclipse.EclipseNode;
import lombok.eclipse.handlers.HandleConstructor.SkipIfConstructorExists;
import lombok.experimental.Builder;
import lombok.experimental.NonFinal;

@ProviderFor(EclipseAnnotationHandler.class)
@HandlerPriority(-1024) //-2^10; to ensure we've picked up @FieldDefault's changes (-2048) but @Value hasn't removed itself yet (-512), so that we can error on presence of it on the builder classes.
public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
@Override public void handle(AnnotationValues<Builder> annotation, Annotation ast, EclipseNode annotationNode) {
long p = (long) ast.sourceStart << 32 | ast.sourceEnd;
Expand Down Expand Up @@ -99,14 +103,23 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
if (parent.get() instanceof TypeDeclaration) {
tdParent = parent;
TypeDeclaration td = (TypeDeclaration) tdParent.get();
new HandleConstructor().generateAllArgsConstructor(tdParent, AccessLevel.PRIVATE, null, SkipIfConstructorExists.I_AM_BUILDER, Collections.<Annotation>emptyList(), ast);

List<EclipseNode> fields = new ArrayList<EclipseNode>();
@SuppressWarnings("deprecation")
boolean valuePresent = (hasAnnotation(lombok.Value.class, parent) || hasAnnotation(lombok.experimental.Value.class, parent));
for (EclipseNode fieldNode : HandleConstructor.findAllFields(tdParent)) {
FieldDeclaration fd = (FieldDeclaration) fieldNode.get();
// final fields with an initializer cannot be written to, so they can't be 'builderized'. Unfortunately presence of @Value makes
// non-final fields final, but @Value's handler hasn't done this yet, so we have to do this math ourselves.
// Value will only skip making a field final if it has an explicit @NonFinal annotation, so we check for that.
if (fd.initialization != null && valuePresent && !hasAnnotation(NonFinal.class, fieldNode)) continue;
namesOfParameters.add(fd.name);
typesOfParameters.add(fd.type);
fields.add(fieldNode);
}

new HandleConstructor().generateConstructor(tdParent, AccessLevel.PACKAGE, fields, null, SkipIfConstructorExists.I_AM_BUILDER, true, Collections.<Annotation>emptyList(), ast);

returnType = namePlusTypeParamsToTypeReference(td.name, td.typeParameters, p);
typeParams = td.typeParameters;
thrownExceptions = null;
Expand Down Expand Up @@ -181,11 +194,15 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
}

EclipseNode builderType = findInnerClass(tdParent, builderClassName);
if (builderType == null) builderType = makeBuilderClass(tdParent, builderClassName, typeParams, ast);
if (builderType == null) {
builderType = makeBuilderClass(tdParent, builderClassName, typeParams, ast);
} else {
sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderType, annotationNode);
}
List<EclipseNode> fieldNodes = addFieldsToBuilder(builderType, namesOfParameters, typesOfParameters, ast);
List<AbstractMethodDeclaration> newMethods = new ArrayList<AbstractMethodDeclaration>();
for (EclipseNode fieldNode : fieldNodes) {
MethodDeclaration newMethod = makeSetterMethodForBuilder(builderType, fieldNode, ast);
MethodDeclaration newMethod = makeSetterMethodForBuilder(builderType, fieldNode, ast, builderInstance.fluent(), builderInstance.chain());
if (newMethod != null) newMethods.add(newMethod);
}

Expand Down Expand Up @@ -315,7 +332,7 @@ private List<EclipseNode> addFieldsToBuilder(EclipseNode builderType, List<char[

private static final AbstractMethodDeclaration[] EMPTY = {};

private MethodDeclaration makeSetterMethodForBuilder(EclipseNode builderType, EclipseNode fieldNode, ASTNode source) {
private MethodDeclaration makeSetterMethodForBuilder(EclipseNode builderType, EclipseNode fieldNode, ASTNode source, boolean fluent, boolean chain) {
TypeDeclaration td = (TypeDeclaration) builderType.get();
AbstractMethodDeclaration[] existing = td.methods;
if (existing == null) existing = EMPTY;
Expand All @@ -329,7 +346,10 @@ private MethodDeclaration makeSetterMethodForBuilder(EclipseNode builderType, Ec
if (Arrays.equals(name, existingName)) return null;
}

return HandleSetter.createSetter(td, fieldNode, fieldNode.getName(), true, ClassFileConstants.AccPublic,
boolean isBoolean = isBoolean(fd.type);
String setterName = fluent ? fieldNode.getName() : TransformationsUtil.toSetterName(null, fieldNode.getName(), isBoolean);

return HandleSetter.createSetter(td, fieldNode, setterName, chain, ClassFileConstants.AccPublic,
source, Collections.<Annotation>emptyList(), Collections.<Annotation>emptyList());
}

Expand Down
10 changes: 9 additions & 1 deletion src/core/lombok/eclipse/handlers/HandleFieldDefaults.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* Handles the {@code lombok.FieldDefaults} annotation for eclipse.
*/
@ProviderFor(EclipseAnnotationHandler.class)
@HandlerPriority(-512) //-2^9; to ensure @Setter and such pick up on messing with the fields' 'final' state, run earlier.
@HandlerPriority(-2048) //-2^11; to ensure @Value picks up on messing with the fields' 'final' state, run earlier.
public class HandleFieldDefaults extends EclipseAnnotationHandler<FieldDefaults> {
public boolean generateFieldDefaultsForType(EclipseNode typeNode, EclipseNode pos, AccessLevel level, boolean makeFinal, boolean checkForTypeLevelFieldDefaults) {
if (checkForTypeLevelFieldDefaults) {
Expand Down Expand Up @@ -112,6 +112,14 @@ public void handle(AnnotationValues<FieldDefaults> annotation, Annotation ast, E
return;
}

if (level == AccessLevel.PACKAGE) {
annotationNode.addError("Setting 'level' to PACKAGE does nothing. To force fields as package private, use the @PackagePrivate annotation on the field.");
}

if (!makeFinal && annotation.isExplicit("makeFinal")) {
annotationNode.addError("Setting 'makeFinal' to false does nothing. To force fields to be non-final, use the @NonFinal annotation on the field.");
}

if (node == null) return;

generateFieldDefaultsForType(node, annotationNode, level, makeFinal, false);
Expand Down
2 changes: 1 addition & 1 deletion src/core/lombok/eclipse/handlers/HandleGetter.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ private void createGetterForField(AccessLevel level,
}

TypeReference fieldType = copyType(field.type, source);
boolean isBoolean = nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0;
boolean isBoolean = isBoolean(fieldType);
String getterName = toGetterName(fieldNode, isBoolean);

if (getterName == null) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/lombok/eclipse/handlers/HandleSetter.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private void createSetterForField(

FieldDeclaration field = (FieldDeclaration) fieldNode.get();
TypeReference fieldType = copyType(field.type, source);
boolean isBoolean = nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0;
boolean isBoolean = isBoolean(fieldType);
String setterName = toSetterName(fieldNode, isBoolean);
boolean shouldReturnThis = shouldReturnThis(fieldNode);

Expand Down
2 changes: 1 addition & 1 deletion src/core/lombok/eclipse/handlers/HandleWither.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private void createWitherForField(

FieldDeclaration field = (FieldDeclaration) fieldNode.get();
TypeReference fieldType = copyType(field.type, source);
boolean isBoolean = nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0;
boolean isBoolean = isBoolean(fieldType);
String witherName = toWitherName(fieldNode, isBoolean);

if (witherName == null) {
Expand Down
16 changes: 16 additions & 0 deletions src/core/lombok/experimental/Builder.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,20 @@
* Default for {@code @Builder} on static methods: {@code (ReturnTypeName)Builder}.
*/
String builderClassName() default "";

/**
* Normally the builder's 'set' methods are fluent, meaning, they have the same name as the field. Set this
* to {@code false} to name the setter method for field {@code someField}: {@code setSomeField}.
* <p>
* <strong>Default: true</strong>
*/
boolean fluent() default true;

/**
* Normally the builder's 'set' methods are chaining, meaning, they return the builder so that you can chain
* calls to set methods. Set this to {@code false} to have these 'set' methods return {@code void} instead.
* <p>
* <strong>Default: true</strong>
*/
boolean chain() default true;
}
33 changes: 26 additions & 7 deletions src/core/lombok/javac/handlers/HandleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,19 @@
import lombok.AccessLevel;
import lombok.core.AST.Kind;
import lombok.core.AnnotationValues;
import lombok.core.HandlerPriority;
import lombok.core.TransformationsUtil;
import lombok.experimental.Builder;
import lombok.experimental.NonFinal;
import lombok.javac.JavacAnnotationHandler;
import lombok.javac.JavacNode;
import lombok.javac.handlers.HandleConstructor.SkipIfConstructorExists;

import static lombok.javac.Javac.*;
import static lombok.core.handlers.HandlerUtil.*;
import static lombok.javac.handlers.JavacHandlerUtil.*;

@ProviderFor(JavacAnnotationHandler.class)
@HandlerPriority(-1024) //-2^10; to ensure we've picked up @FieldDefault's changes (-2048) but @Value hasn't removed itself yet (-512), so that we can error on presence of it on the builder classes.
public class HandleBuilder extends JavacAnnotationHandler<Builder> {
@Override public void handle(AnnotationValues<Builder> annotation, JCAnnotation ast, JavacNode annotationNode) {
Builder builderInstance = annotation.getInstance();
Expand Down Expand Up @@ -94,14 +97,22 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
if (parent.get() instanceof JCClassDecl) {
tdParent = parent;
JCClassDecl td = (JCClassDecl) tdParent.get();
new HandleConstructor().generateAllArgsConstructor(tdParent, AccessLevel.PRIVATE, null, SkipIfConstructorExists.I_AM_BUILDER, annotationNode);

ListBuffer<JavacNode> allFields = ListBuffer.lb();
@SuppressWarnings("deprecation")
boolean valuePresent = (hasAnnotation(lombok.Value.class, parent) || hasAnnotation(lombok.experimental.Value.class, parent));
for (JavacNode fieldNode : HandleConstructor.findAllFields(tdParent)) {
JCVariableDecl fd = (JCVariableDecl) fieldNode.get();
// final fields with an initializer cannot be written to, so they can't be 'builderized'. Unfortunately presence of @Value makes
// non-final fields final, but @Value's handler hasn't done this yet, so we have to do this math ourselves.
// Value will only skip making a field final if it has an explicit @NonFinal annotation, so we check for that.
if (fd.init != null && valuePresent && !hasAnnotation(NonFinal.class, fieldNode)) continue;
namesOfParameters.add(fd.name);
typesOfParameters.add(fd.vartype);
allFields.append(fieldNode);
}

new HandleConstructor().generateConstructor(tdParent, AccessLevel.PACKAGE, List.<JCAnnotation>nil(), allFields.toList(), null, SkipIfConstructorExists.I_AM_BUILDER, true, annotationNode);

returnType = namePlusTypeParamsToTypeReference(tdParent.getTreeMaker(), td.name, td.typarams);
typeParams = td.typarams;
thrownExceptions = List.nil();
Expand Down Expand Up @@ -171,11 +182,15 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
}

JavacNode builderType = findInnerClass(tdParent, builderClassName);
if (builderType == null) builderType = makeBuilderClass(tdParent, builderClassName, typeParams, ast);
if (builderType == null) {
builderType = makeBuilderClass(tdParent, builderClassName, typeParams, ast);
} else {
sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderType, annotationNode);
}
java.util.List<JavacNode> fieldNodes = addFieldsToBuilder(builderType, namesOfParameters, typesOfParameters, ast);
java.util.List<JCMethodDecl> newMethods = new ArrayList<JCMethodDecl>();
for (JavacNode fieldNode : fieldNodes) {
JCMethodDecl newMethod = makeSetterMethodForBuilder(builderType, fieldNode, ast);
JCMethodDecl newMethod = makeSetterMethodForBuilder(builderType, fieldNode, ast, builderInstance.fluent(), builderInstance.chain());
if (newMethod != null) newMethods.add(newMethod);
}

Expand Down Expand Up @@ -281,16 +296,20 @@ private java.util.List<JavacNode> addFieldsToBuilder(JavacNode builderType, java
}


private JCMethodDecl makeSetterMethodForBuilder(JavacNode builderType, JavacNode fieldNode, JCTree source) {
private JCMethodDecl makeSetterMethodForBuilder(JavacNode builderType, JavacNode fieldNode, JCTree source, boolean fluent, boolean chain) {
Name fieldName = ((JCVariableDecl) fieldNode.get()).name;

for (JavacNode child : builderType.down()) {
if (child.getKind() != Kind.METHOD) continue;
Name existingName = ((JCMethodDecl) child.get()).name;
if (existingName.equals(fieldName)) return null;
}

boolean isBoolean = isBoolean(fieldNode);
String setterName = fluent ? fieldNode.getName() : TransformationsUtil.toSetterName(null, fieldNode.getName(), isBoolean);

TreeMaker maker = builderType.getTreeMaker();
return HandleSetter.createSetter(Flags.PUBLIC, fieldNode, maker, fieldName.toString(), true, source, List.<JCAnnotation>nil(), List.<JCAnnotation>nil());
return HandleSetter.createSetter(Flags.PUBLIC, fieldNode, maker, setterName, chain, source, List.<JCAnnotation>nil(), List.<JCAnnotation>nil());
}

private JavacNode findInnerClass(JavacNode parent, String name) {
Expand Down
10 changes: 9 additions & 1 deletion src/core/lombok/javac/handlers/HandleFieldDefaults.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
* Handles the {@code lombok.FieldDefaults} annotation for eclipse.
*/
@ProviderFor(JavacAnnotationHandler.class)
@HandlerPriority(-512) //-2^9; to ensure @Setter and such pick up on messing with the fields' 'final' state, run earlier.
@HandlerPriority(-2048) //-2^11; to ensure @Value picks up on messing with the fields' 'final' state, run earlier.
public class HandleFieldDefaults extends JavacAnnotationHandler<FieldDefaults> {
public boolean generateFieldDefaultsForType(JavacNode typeNode, JavacNode errorNode, AccessLevel level, boolean makeFinal, boolean checkForTypeLevelFieldDefaults) {
if (checkForTypeLevelFieldDefaults) {
Expand Down Expand Up @@ -108,6 +108,14 @@ public void setFieldDefaultsForField(JavacNode fieldNode, DiagnosticPosition pos
return;
}

if (level == AccessLevel.PACKAGE) {
annotationNode.addError("Setting 'level' to PACKAGE does nothing. To force fields as package private, use the @PackagePrivate annotation on the field.");
}

if (!makeFinal && annotation.isExplicit("makeFinal")) {
annotationNode.addError("Setting 'makeFinal' to false does nothing. To force fields to be non-final, use the @NonFinal annotation on the field.");
}

if (node == null) return;

generateFieldDefaultsForType(node, annotationNode, level, makeFinal, false);
Expand Down

0 comments on commit 7af9add

Please sign in to comment.