From e1c39bbc601408decb0ae147d181708a5af41307 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Tue, 18 Jun 2013 04:23:15 +0200 Subject: javac builder implementation. Passes all tests. Added toString() impl for builders in both eclipse and javac. Added all documentation, though it'll need some reviewing. --- website/features/experimental/index.html | 2 ++ 1 file changed, 2 insertions(+) (limited to 'website/features/experimental/index.html') diff --git a/website/features/experimental/index.html b/website/features/experimental/index.html index 24fbb541..d0a086a0 100644 --- a/website/features/experimental/index.html +++ b/website/features/experimental/index.html @@ -22,6 +22,8 @@ Features that receive positive community feedback and which seem to produce clean, flexible code will eventually become accepted as a core feature and move out of the experimental package.

@Builder: It's like drinking tea with an extended pinky while wearing a monocle: No-hassle fancy-pants APIs for object creation!
@Accessors: A more fluent API for getters and setters.
@ExtensionMethod: New default field modifiers for the 21st century.
@Wither: Immutable 'setters' - methods that create a clone but with one changed field.
@Value: Immutable classes made very easy.
onMethod= / onConstructor= / onParam: Sup dawg, we heard you like annotations, so we put annotations in your annotations so you can annotate while you're annotating.

Putting the "Ex" in "Experimental": promoted or deleted experimental features.

@Value: Promoted: @Value has proven its value and has been moved to the main package. +

diff --git a/website/features/experimental/onX.html b/website/features/experimental/onX.html index 99365deb..66b0164f 100644 --- a/website/features/experimental/onX.html +++ b/website/features/experimental/onX.html @@ -69,7 +69,7 @@

diff --git a/website/features/index.html b/website/features/index.html index 8b3765c1..f9b8cdfa 100644 --- a/website/features/index.html +++ b/website/features/index.html @@ -30,6 +30,8 @@

@Data

All together now: A shortcut for @ToString, @EqualsAndHashCode, @Getter on all fields, and @Setter on all non-final fields, and @RequiredArgsConstructor!

@Value

Immutable classes made very easy.

@SneakyThrows

To boldly throw checked exceptions where no one has thrown them before!

@Synchronized

-- cgit From 7af9add9996f2efab6cccc50c5503b3457534930 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Tue, 16 Jul 2013 00:51:31 +0200 Subject: * Fixed issues with @FieldDefaults and @Value (you can NOT override @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. --- src/core/lombok/core/TransformationsUtil.java | 25 ++++++++++++++-- .../eclipse/handlers/EclipseHandlerUtil.java | 35 ++++++++++++++++++++-- .../lombok/eclipse/handlers/HandleBuilder.java | 30 +++++++++++++++---- .../eclipse/handlers/HandleFieldDefaults.java | 10 ++++++- src/core/lombok/eclipse/handlers/HandleGetter.java | 2 +- src/core/lombok/eclipse/handlers/HandleSetter.java | 2 +- src/core/lombok/eclipse/handlers/HandleWither.java | 2 +- src/core/lombok/experimental/Builder.java | 16 ++++++++++ src/core/lombok/javac/handlers/HandleBuilder.java | 33 +++++++++++++++----- .../lombok/javac/handlers/HandleFieldDefaults.java | 10 ++++++- .../lombok/javac/handlers/JavacHandlerUtil.java | 29 +++++++++++++++++- test/core/src/lombok/AbstractRunTests.java | 18 ++++++----- .../after-delombok/BuilderChainAndFluent.java | 31 +++++++++++++++++++ .../resource/after-delombok/BuilderSimple.java | 2 +- .../resource/after-ecj/BuilderChainAndFluent.java | 25 ++++++++++++++++ .../resource/after-ecj/BuilderSimple.java | 2 +- .../resource/before/BuilderChainAndFluent.java | 4 +++ .../resource/before/BuilderInvalidUse.java | 18 +++++++++++ .../BuilderInvalidUse.java.messages | 2 ++ .../messages-ecj/BuilderInvalidUse.java.messages | 2 ++ website/features/Value.html | 5 +++- website/features/experimental/Builder.html | 33 ++++++++++++-------- website/features/experimental/index.html | 2 +- 23 files changed, 290 insertions(+), 48 deletions(-) create mode 100644 test/transform/resource/after-delombok/BuilderChainAndFluent.java create mode 100644 test/transform/resource/after-ecj/BuilderChainAndFluent.java create mode 100644 test/transform/resource/before/BuilderChainAndFluent.java create mode 100644 test/transform/resource/before/BuilderInvalidUse.java create mode 100644 test/transform/resource/messages-delombok/BuilderInvalidUse.java.messages create mode 100644 test/transform/resource/messages-ecj/BuilderInvalidUse.java.messages (limited to 'website/features/experimental/index.html') diff --git a/src/core/lombok/core/TransformationsUtil.java b/src/core/lombok/core/TransformationsUtil.java index 921c27d6..8959ad7a 100644 --- a/src/core/lombok/core/TransformationsUtil.java +++ b/src/core/lombok/core/TransformationsUtil.java @@ -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 @@ -39,6 +51,13 @@ public class TransformationsUtil { //Prevent instantiation } + @SuppressWarnings({"all", "unchecked", "deprecation"}) + public static final List> INVALID_ON_BUILDERS = Collections.unmodifiableList( + Arrays.>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 @@ -159,12 +178,12 @@ public class TransformationsUtil { 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'. diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java index 364ce0a5..9bd634f7 100644 --- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java +++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java @@ -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; @@ -296,6 +297,29 @@ public class EclipseHandlerUtil { } + public static void sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(EclipseNode typeNode, EclipseNode errorNode) { + List disallowed = null; + for (EclipseNode child : typeNode.down()) { + for (Class annType : INVALID_ON_BUILDERS) { + if (annotationTypeMatches(annType, child)) { + if (disallowed == null) disallowed = new ArrayList(); + 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; @@ -845,15 +869,20 @@ public class EclipseHandlerUtil { 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)) { diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java index e2bf5fe2..70110a9c 100644 --- a/src/core/lombok/eclipse/handlers/HandleBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java @@ -58,13 +58,17 @@ import org.mangosdk.spi.ProviderFor; 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 { @Override public void handle(AnnotationValues annotation, Annotation ast, EclipseNode annotationNode) { long p = (long) ast.sourceStart << 32 | ast.sourceEnd; @@ -99,14 +103,23 @@ public class HandleBuilder extends EclipseAnnotationHandler { if (parent.get() instanceof TypeDeclaration) { tdParent = parent; TypeDeclaration td = (TypeDeclaration) tdParent.get(); - new HandleConstructor().generateAllArgsConstructor(tdParent, AccessLevel.PRIVATE, null, SkipIfConstructorExists.I_AM_BUILDER, Collections.emptyList(), ast); + List fields = new ArrayList(); + @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.emptyList(), ast); + returnType = namePlusTypeParamsToTypeReference(td.name, td.typeParameters, p); typeParams = td.typeParameters; thrownExceptions = null; @@ -181,11 +194,15 @@ public class HandleBuilder extends EclipseAnnotationHandler { } 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 fieldNodes = addFieldsToBuilder(builderType, namesOfParameters, typesOfParameters, ast); List newMethods = new ArrayList(); 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); } @@ -315,7 +332,7 @@ public class HandleBuilder extends EclipseAnnotationHandler { 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; @@ -329,7 +346,10 @@ public class HandleBuilder extends EclipseAnnotationHandler { 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.emptyList(), Collections.emptyList()); } diff --git a/src/core/lombok/eclipse/handlers/HandleFieldDefaults.java b/src/core/lombok/eclipse/handlers/HandleFieldDefaults.java index 0d21fc08..d6d839cc 100644 --- a/src/core/lombok/eclipse/handlers/HandleFieldDefaults.java +++ b/src/core/lombok/eclipse/handlers/HandleFieldDefaults.java @@ -43,7 +43,7 @@ import org.mangosdk.spi.ProviderFor; * 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 { public boolean generateFieldDefaultsForType(EclipseNode typeNode, EclipseNode pos, AccessLevel level, boolean makeFinal, boolean checkForTypeLevelFieldDefaults) { if (checkForTypeLevelFieldDefaults) { @@ -112,6 +112,14 @@ public class HandleFieldDefaults extends EclipseAnnotationHandler 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); diff --git a/src/core/lombok/eclipse/handlers/HandleGetter.java b/src/core/lombok/eclipse/handlers/HandleGetter.java index 760c595e..787f6f6c 100644 --- a/src/core/lombok/eclipse/handlers/HandleGetter.java +++ b/src/core/lombok/eclipse/handlers/HandleGetter.java @@ -187,7 +187,7 @@ public class HandleGetter extends EclipseAnnotationHandler { } 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) { diff --git a/src/core/lombok/eclipse/handlers/HandleSetter.java b/src/core/lombok/eclipse/handlers/HandleSetter.java index ae846a4e..3bfcc51c 100644 --- a/src/core/lombok/eclipse/handlers/HandleSetter.java +++ b/src/core/lombok/eclipse/handlers/HandleSetter.java @@ -159,7 +159,7 @@ public class HandleSetter extends EclipseAnnotationHandler { 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); diff --git a/src/core/lombok/eclipse/handlers/HandleWither.java b/src/core/lombok/eclipse/handlers/HandleWither.java index 9d74cbd1..27fbc635 100644 --- a/src/core/lombok/eclipse/handlers/HandleWither.java +++ b/src/core/lombok/eclipse/handlers/HandleWither.java @@ -160,7 +160,7 @@ public class HandleWither extends EclipseAnnotationHandler { 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) { diff --git a/src/core/lombok/experimental/Builder.java b/src/core/lombok/experimental/Builder.java index 5f2d1ca6..1300e7d3 100644 --- a/src/core/lombok/experimental/Builder.java +++ b/src/core/lombok/experimental/Builder.java @@ -118,4 +118,20 @@ public @interface Builder { * 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}. + *

+ * Default: true + */ + 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. + *

+ * Default: true + */ + boolean chain() default true; } diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index aa485b26..6422f5ed 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -49,16 +49,19 @@ import com.sun.tools.javac.util.Name; 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 { @Override public void handle(AnnotationValues annotation, JCAnnotation ast, JavacNode annotationNode) { Builder builderInstance = annotation.getInstance(); @@ -94,14 +97,22 @@ public class HandleBuilder extends JavacAnnotationHandler { 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 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.nil(), allFields.toList(), null, SkipIfConstructorExists.I_AM_BUILDER, true, annotationNode); + returnType = namePlusTypeParamsToTypeReference(tdParent.getTreeMaker(), td.name, td.typarams); typeParams = td.typarams; thrownExceptions = List.nil(); @@ -171,11 +182,15 @@ public class HandleBuilder extends JavacAnnotationHandler { } 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 fieldNodes = addFieldsToBuilder(builderType, namesOfParameters, typesOfParameters, ast); java.util.List newMethods = new ArrayList(); 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); } @@ -281,16 +296,20 @@ public class HandleBuilder extends JavacAnnotationHandler { } - 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.nil(), List.nil()); + return HandleSetter.createSetter(Flags.PUBLIC, fieldNode, maker, setterName, chain, source, List.nil(), List.nil()); } private JavacNode findInnerClass(JavacNode parent, String name) { diff --git a/src/core/lombok/javac/handlers/HandleFieldDefaults.java b/src/core/lombok/javac/handlers/HandleFieldDefaults.java index d32446c3..038f3e3f 100644 --- a/src/core/lombok/javac/handlers/HandleFieldDefaults.java +++ b/src/core/lombok/javac/handlers/HandleFieldDefaults.java @@ -44,7 +44,7 @@ import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; * 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 { public boolean generateFieldDefaultsForType(JavacNode typeNode, JavacNode errorNode, AccessLevel level, boolean makeFinal, boolean checkForTypeLevelFieldDefaults) { if (checkForTypeLevelFieldDefaults) { @@ -108,6 +108,14 @@ public class HandleFieldDefaults extends JavacAnnotationHandler { 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); diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java index a24dad7d..d7d29da2 100644 --- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java +++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java @@ -21,6 +21,7 @@ */ package lombok.javac.handlers; +import static lombok.core.TransformationsUtil.INVALID_ON_BUILDERS; import static lombok.javac.Javac.*; import java.lang.annotation.Annotation; @@ -446,8 +447,12 @@ public class JavacHandlerUtil { } } - private static boolean isBoolean(JavacNode field) { + public static boolean isBoolean(JavacNode field) { JCExpression varType = ((JCVariableDecl) field.get()).vartype; + return isBoolean(varType); + } + + public static boolean isBoolean(JCExpression varType) { return varType != null && varType.toString().equals("boolean"); } @@ -1065,6 +1070,28 @@ public class JavacHandlerUtil { return maker.Ident(typeName); } + public static void sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(JavacNode typeNode, JavacNode errorNode) { + List disallowed = List.nil(); + for (JavacNode child : typeNode.down()) { + for (Class annType : INVALID_ON_BUILDERS) { + if (annotationTypeMatches(annType, child)) { + disallowed = disallowed.append(annType.getSimpleName()); + } + } + } + + int size = disallowed.size(); + if (size == 0) return; + if (size == 1) { + errorNode.addError("@" + disallowed.head + " 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()); + } + static List copyAnnotations(List in) { ListBuffer out = ListBuffer.lb(); for (JCExpression expr : in) { diff --git a/test/core/src/lombok/AbstractRunTests.java b/test/core/src/lombok/AbstractRunTests.java index a80c7d8d..2f3f0988 100644 --- a/test/core/src/lombok/AbstractRunTests.java +++ b/test/core/src/lombok/AbstractRunTests.java @@ -68,10 +68,12 @@ public abstract class AbstractRunTests { } } - StringReader r = new StringReader(expectedFile); - BufferedReader br = new BufferedReader(r); - String firstLine = br.readLine(); - if (firstLine != null && (firstLine.startsWith("//ignore") || params.shouldIgnoreBasedOnVersion(firstLine))) return false; + if (expectedFile != null) { + StringReader r = new StringReader(expectedFile); + BufferedReader br = new BufferedReader(r); + String firstLine = br.readLine(); + if (firstLine != null && (firstLine.startsWith("//ignore") || params.shouldIgnoreBasedOnVersion(firstLine))) return false; + } compare( file.getName(), @@ -91,7 +93,7 @@ public abstract class AbstractRunTests { try { reader = new BufferedReader(new FileReader(file)); } catch (FileNotFoundException e) { - return ""; + return null; } StringBuilder result = new StringBuilder(); String line; @@ -104,7 +106,7 @@ public abstract class AbstractRunTests { } private String readFile(File dir, File file, boolean messages) throws IOException { - if (dir == null) return ""; + if (dir == null) return null; return readFile(new File(dir, file.getName() + (messages ? ".messages" : ""))); } @@ -140,7 +142,9 @@ public abstract class AbstractRunTests { } private void compare(String name, String expectedFile, String actualFile, List expectedMessages, LinkedHashSet actualMessages, boolean printErrors) throws Throwable { - try { + if (expectedFile == null && expectedMessages.isEmpty()) expectedFile = ""; + + if (expectedFile != null) try { compareContent(name, expectedFile, actualFile); } catch (Throwable e) { if (printErrors) { diff --git a/test/transform/resource/after-delombok/BuilderChainAndFluent.java b/test/transform/resource/after-delombok/BuilderChainAndFluent.java new file mode 100644 index 00000000..d4975bff --- /dev/null +++ b/test/transform/resource/after-delombok/BuilderChainAndFluent.java @@ -0,0 +1,31 @@ +class BuilderChainAndFluent { + private final int yes; + @java.lang.SuppressWarnings("all") + BuilderChainAndFluent(final int yes) { + this.yes = yes; + } + @java.lang.SuppressWarnings("all") + public static class BuilderChainAndFluentBuilder { + private int yes; + @java.lang.SuppressWarnings("all") + BuilderChainAndFluentBuilder() { + } + @java.lang.SuppressWarnings("all") + public void setYes(final int yes) { + this.yes = yes; + } + @java.lang.SuppressWarnings("all") + public BuilderChainAndFluent build() { + return new BuilderChainAndFluent(yes); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "BuilderChainAndFluent.BuilderChainAndFluentBuilder(yes=" + this.yes + ")"; + } + } + @java.lang.SuppressWarnings("all") + public static BuilderChainAndFluentBuilder builder() { + return new BuilderChainAndFluentBuilder(); + } +} \ No newline at end of file diff --git a/test/transform/resource/after-delombok/BuilderSimple.java b/test/transform/resource/after-delombok/BuilderSimple.java index 24ac369c..11c0e58c 100644 --- a/test/transform/resource/after-delombok/BuilderSimple.java +++ b/test/transform/resource/after-delombok/BuilderSimple.java @@ -5,7 +5,7 @@ class BuilderSimple { private List also; private int $butNotMe; @java.lang.SuppressWarnings("all") - private BuilderSimple(final int yes, final List also) { + BuilderSimple(final int yes, final List also) { this.yes = yes; this.also = also; } diff --git a/test/transform/resource/after-ecj/BuilderChainAndFluent.java b/test/transform/resource/after-ecj/BuilderChainAndFluent.java new file mode 100644 index 00000000..6a307105 --- /dev/null +++ b/test/transform/resource/after-ecj/BuilderChainAndFluent.java @@ -0,0 +1,25 @@ +@lombok.experimental.Builder(fluent = false,chain = false) class BuilderChainAndFluent { + public static @java.lang.SuppressWarnings("all") class BuilderChainAndFluentBuilder { + private int yes; + @java.lang.SuppressWarnings("all") BuilderChainAndFluentBuilder() { + super(); + } + public @java.lang.SuppressWarnings("all") void setYes(final int yes) { + this.yes = yes; + } + public @java.lang.SuppressWarnings("all") BuilderChainAndFluent build() { + return new BuilderChainAndFluent(yes); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (("BuilderChainAndFluent.BuilderChainAndFluentBuilder(yes=" + this.yes) + ")"); + } + } + private final int yes; + @java.lang.SuppressWarnings("all") BuilderChainAndFluent(final int yes) { + super(); + this.yes = yes; + } + public static @java.lang.SuppressWarnings("all") BuilderChainAndFluentBuilder builder() { + return new BuilderChainAndFluentBuilder(); + } +} diff --git a/test/transform/resource/after-ecj/BuilderSimple.java b/test/transform/resource/after-ecj/BuilderSimple.java index 228b1928..85db360d 100644 --- a/test/transform/resource/after-ecj/BuilderSimple.java +++ b/test/transform/resource/after-ecj/BuilderSimple.java @@ -25,7 +25,7 @@ import java.util.List; private final int yes; private List also; private int $butNotMe; - private @java.lang.SuppressWarnings("all") BuilderSimple(final int yes, final List also) { + @java.lang.SuppressWarnings("all") BuilderSimple(final int yes, final List also) { super(); this.yes = yes; this.also = also; diff --git a/test/transform/resource/before/BuilderChainAndFluent.java b/test/transform/resource/before/BuilderChainAndFluent.java new file mode 100644 index 00000000..4d08741b --- /dev/null +++ b/test/transform/resource/before/BuilderChainAndFluent.java @@ -0,0 +1,4 @@ +@lombok.experimental.Builder(fluent = false, chain = false) +class BuilderChainAndFluent { + private final int yes; +} diff --git a/test/transform/resource/before/BuilderInvalidUse.java b/test/transform/resource/before/BuilderInvalidUse.java new file mode 100644 index 00000000..07f37d3d --- /dev/null +++ b/test/transform/resource/before/BuilderInvalidUse.java @@ -0,0 +1,18 @@ +@lombok.experimental.Builder +class BuilderInvalidUse { + private int something; + + @lombok.Getter @lombok.Setter @lombok.experimental.FieldDefaults(makeFinal = true) @lombok.experimental.Wither @lombok.Data @lombok.ToString @lombok.EqualsAndHashCode + @lombok.AllArgsConstructor + public static class BuilderInvalidUseBuilder { + + } +} + +@lombok.experimental.Builder +class AlsoInvalid { + @lombok.Value + public static class AlsoInvalidBuilder { + + } +} \ No newline at end of file diff --git a/test/transform/resource/messages-delombok/BuilderInvalidUse.java.messages b/test/transform/resource/messages-delombok/BuilderInvalidUse.java.messages new file mode 100644 index 00000000..aeeb0c86 --- /dev/null +++ b/test/transform/resource/messages-delombok/BuilderInvalidUse.java.messages @@ -0,0 +1,2 @@ +1:1 @Getter, @Setter, @Wither, @Data, @ToString, @EqualsAndHashCode, @AllArgsConstructor are not allowed on builder classes. +12:1 @Value is not allowed on builder classes. \ No newline at end of file diff --git a/test/transform/resource/messages-ecj/BuilderInvalidUse.java.messages b/test/transform/resource/messages-ecj/BuilderInvalidUse.java.messages new file mode 100644 index 00000000..8ffc6e26 --- /dev/null +++ b/test/transform/resource/messages-ecj/BuilderInvalidUse.java.messages @@ -0,0 +1,2 @@ +1:0 @Getter, @Setter, @FieldDefaults, @Wither, @Data, @ToString, @EqualsAndHashCode, @AllArgsConstructor are not allowed on builder classes. +12:331 @Value is not allowed on builder classes. \ No newline at end of file diff --git a/website/features/Value.html b/website/features/Value.html index 92fcc825..e2cd8600 100644 --- a/website/features/Value.html +++ b/website/features/Value.html @@ -31,7 +31,7 @@

It is possible to override the final-by-default and private-by-default behaviour using either an explicit access level on a field, or by using the @NonFinal or @PackagePrivate annotations.
It is possible to override any default behaviour for any of the 'parts' that make up @Value by explicitly using that annotation. -

@@ -54,6 +54,9 @@

@Value was an experimental feature from v0.11.4 to v0.11.9 (as @lombok.experimental.Value). It has since been moved into the core package. The old annotation is still around (and is an alias). It will eventually be removed in a future version, though. +

+ It is not possible to use @FieldDefaults to 'undo' the private-by-default and final-by-default aspect of fields in the annotated class. Use @NonFinal and @PackagePrivate on the fields in the class to override this behaviour. +

diff --git a/website/features/experimental/Builder.html b/website/features/experimental/Builder.html index 5ba74a27..a43d024b 100644 --- a/website/features/experimental/Builder.html +++ b/website/features/experimental/Builder.html @@ -11,7 +11,7 @@

@Builder

It's like drinking tea with an extended pinky while wearing a monocle: No-hassle fancy-pants APIs for object creation!

... and Bob's your uncle: No-hassle fancy-pants APIs for object creation!

Since

@@ -24,7 +24,6 @@ Experimental because:

New feature - community feedback requested.
This feature will move to the core package soon.

Current status: sure thing - This feature will move to the core package soon.

@@ -43,7 +42,7 @@

An inner static class named FooBuilder, with the same type arguments as the static method (called the builder).

In the builder: One private non-static non-final field for each parameter of the target.

In the builder: A package private no-args empty constructor.

In the builder: A 'setter'-like method for each parmeter of the target: It has the same type as that parameter and the same name. +

In the builder: A 'setter'-like method for each parameter of the target: It has the same type as that parameter and the same name. It returns the builder itself, so that the setter calls can be chained, as in the above example.

In the builder: A build() method which calls the static method, passing in each field. It returns the same type that the target returns.

@@ -52,18 +51,16 @@ Each listed generated element will be silently skipped if that element already exists (disregarding parameter counts and looking only at names). This includes the builder itself: If that class already exists, lombok will simply start injecting fields and methods inside this already existing - class, unless of course the fields / methods to be injected already exist. + class, unless of course the fields / methods to be injected already exist. You may not put any other method (or constructor) generating lombok annotation + on a builder class though; for example, you can not put @EqualsAndHashCode on the builder class.

Now that the "static method" mode is clear, putting a @Builder annotation on a constructor functions similarly; effectively, constructors are just static methods that have a special syntax to invoke them: Their 'return type' is the class they construct, and their type parameters are the same as the type parameters of the class itself.

- Finally, applying @Builder to a class is as if you added @AllArgsConstructor(acces = AccessLevel.PACKAGE) to the class and applied the - @Builder annotation to this all-args-constructor. Note that this constructor is only generated if there is no explicit - constructor present in the so annotated class, but this @AllArgsConstructor has priority over any other implicitly - generated lombok constructor (such as @Data and @Value). If an explicit constructor is present, no constructor is generated, - but the builder will be created with the assumption that this constructor exists. If you've written another constructor, you'll get a compilation error.
- The solution is to either let lombok write this constructor (delete your own), or, annotate your constructor instead. + Finally, applying @Builder to a class is as if you added @AllArgsConstructor(access = AccessLevel.PACKAGE) to the class and applied the + @Builder annotation to this all-args-constructor. This only works if you haven't written any explicit constructors yourself. If you do have an + explicit constructor, put the @Builder annotation on the constructor instead of on the class.

The name of the builder class is FoobarBuilder, where Foobar is the simplified, title-cased form of the return type of the target - that is, the name of your type for @Builder on constructors and types, and the name of the return type for @Builder @@ -71,9 +68,15 @@ FancyListBuilder<T>. If @Builder is applied to a static method that returns void, the builder will be named VoidBuilder.

- The only configurable aspect of builder are the builder's class name (default: return type + 'Builder'), the build() method's name, and the - builder() method's name:
- @Builder(builderClassName = "HelloWorldBuilder", buildMethodName = "execute", builderMethodName = "helloWorld") + The configurable aspects of builder are:

The builder's class name (default: return type + 'Builder')
The build() method's name (default: "build")
The builder() method's name (default: "builder")
The 'fluent' nature of the generated 'setter'-like methods. A fluent 'setter' is named just fieldName(), a non-fluent one is named setFieldName(). (default: true)
The 'chain' nature of the generated 'setter'-like methods. A chainable 'setter' returns the builder object itself, letting you chain 'set' calls. A non-chainable setter returns void. (default: true)

+ Example usage where all options are changed from their defaults:
+

@Builder(builderClassName = "HelloWorldBuilder", buildMethodName = "execute", builderMethodName = "helloWorld", fluent = false, chain = false)

@@ -104,6 +107,10 @@ instances of a builder as keys in a set or map. However, that's not a sensible thing to do. Hence, no hashCode or equals.

Generics are sorted out for you. +

+ If an explicit constructor is present, but @Builder is placed on the class, then the builder will be generated as if an explicit constructor is present with the + same arguments list as what @AllArgsConstructor would produce. If this constructor does not exist, a compile time error will result. Usually you should just either let + lombok make this constructor (delete your constructor from the source), or, move the @Builder annotation to the constructor.

diff --git a/website/features/experimental/index.html b/website/features/experimental/index.html index 31fcd5ad..16d58050 100644 --- a/website/features/experimental/index.html +++ b/website/features/experimental/index.html @@ -23,7 +23,7 @@ as a core feature and move out of the experimental package.

@Builder: It's like drinking tea with an extended pinky while wearing a monocle: No-hassle fancy-pants APIs for object creation!; ... and Bob's your uncle: No-hassle fancy-pants APIs for object creation!
@Accessors: A more fluent API for getters and setters.
@ExtensionMethod

@Value

Since

Overview

With Lombok

Vanilla Java

Small print

@Value

Since

Experimental

Overview

With Lombok

Vanilla Java

Small print

Putting the "Ex" in "Experimental": promoted or deleted experimental features.

@Builder

Since