From 2fd46a50b6125fd9f0e8177ffad5db8ee934fe34 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Thu, 1 Nov 2018 16:38:25 +0100 Subject: [feature] FieldNameConstants now works like Builder: Make whatever bits it does by hand and lombok fills in whatever is missing. --- doc/changelog.markdown | 2 +- .../eclipse/handlers/EclipseHandlerUtil.java | 10 +++ .../lombok/eclipse/handlers/HandleBuilder.java | 10 --- .../eclipse/handlers/HandleFieldNameConstants.java | 82 +++++++++++++--------- src/core/lombok/javac/handlers/HandleBuilder.java | 9 --- .../javac/handlers/HandleFieldNameConstants.java | 46 ++++++++---- .../lombok/javac/handlers/JavacHandlerUtil.java | 17 ++++- .../after-delombok/FieldNameConstantsBasic.java | 2 +- .../FieldNameConstantsHandrolled.java | 30 ++++++++ .../after-ecj/FieldNameConstantsBasic.java | 2 +- .../after-ecj/FieldNameConstantsHandrolled.java | 59 ++++++++++++++++ .../before/FieldNameConstantsHandrolled.java | 33 +++++++++ .../features/experimental/FieldNameConstants.html | 3 + 13 files changed, 237 insertions(+), 68 deletions(-) create mode 100644 test/transform/resource/after-delombok/FieldNameConstantsHandrolled.java create mode 100644 test/transform/resource/after-ecj/FieldNameConstantsHandrolled.java create mode 100644 test/transform/resource/before/FieldNameConstantsHandrolled.java diff --git a/doc/changelog.markdown b/doc/changelog.markdown index 57112d76..4760b5f9 100644 --- a/doc/changelog.markdown +++ b/doc/changelog.markdown @@ -3,7 +3,7 @@ Lombok Changelog ### v1.18.5 "Edgy Guinea Pig" * BUGFIX: Since version 1.18.4, the delombok ant task didn't work and errored with a `NoClassDefFoundError`. [Issue #1932](https://github.com/rzwitserloot/lombok/issues/1932) - +* FEATURE: The `@FieldNameConstants` feature now allows you to write the inner type by hand and add whatever you like to it; lombok will add the constants to this class. See the updated [FieldNameConstants feature](https://projectlombok.org/features/experimental/FieldNameConstants) page. ### v1.18.4 (October 30th, 2018) * PLATFORM: Support for Eclipse Photon. [Issue #1831](https://github.com/rzwitserloot/lombok/issues/1831) * PLATFORM: Angular IDE is now recognized by the installer [Issue #1830](https://github.com/rzwitserloot/lombok/issues/1830) diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java index c69b3d0f..026c2383 100644 --- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java +++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java @@ -672,6 +672,16 @@ public class EclipseHandlerUtil { } } + public static EclipseNode findInnerClass(EclipseNode parent, String name) { + char[] c = name.toCharArray(); + for (EclipseNode child : parent.down()) { + if (child.getKind() != Kind.TYPE) continue; + TypeDeclaration td = (TypeDeclaration) child.get(); + if (Arrays.equals(td.name, c)) return child; + } + return null; + } + public static EclipseNode findAnnotation(Class type, EclipseNode node) { if (node == null) return null; if (type == null) return null; diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java index 0ce1436d..fc11aff2 100644 --- a/src/core/lombok/eclipse/handlers/HandleBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java @@ -802,16 +802,6 @@ public class HandleBuilder extends EclipseAnnotationHandler { injectMethod(builderType, setter); } - public EclipseNode findInnerClass(EclipseNode parent, String name) { - char[] c = name.toCharArray(); - for (EclipseNode child : parent.down()) { - if (child.getKind() != Kind.TYPE) continue; - TypeDeclaration td = (TypeDeclaration) child.get(); - if (Arrays.equals(td.name, c)) return child; - } - return null; - } - public EclipseNode makeBuilderClass(boolean isStatic, EclipseNode tdParent, String builderClassName, TypeParameter[] typeParams, ASTNode source) { TypeDeclaration parent = (TypeDeclaration) tdParent.get(); TypeDeclaration builder = new TypeDeclaration(parent.compilationResult); diff --git a/src/core/lombok/eclipse/handlers/HandleFieldNameConstants.java b/src/core/lombok/eclipse/handlers/HandleFieldNameConstants.java index 15650490..91b7771b 100644 --- a/src/core/lombok/eclipse/handlers/HandleFieldNameConstants.java +++ b/src/core/lombok/eclipse/handlers/HandleFieldNameConstants.java @@ -48,7 +48,6 @@ import org.eclipse.jdt.internal.compiler.ast.Statement; import org.eclipse.jdt.internal.compiler.ast.StringLiteral; import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; -import org.eclipse.jdt.internal.compiler.lookup.ClassScope; import org.eclipse.jdt.internal.compiler.lookup.TypeConstants; import org.mangosdk.spi.ProviderFor; @@ -75,7 +74,7 @@ public class HandleFieldNameConstants extends EclipseAnnotationHandler fields, boolean asEnum, String innerTypeName) { + private void createInnerTypeFieldNameConstants(EclipseNode typeNode, EclipseNode errorNode, ASTNode source, AccessLevel level, List fields, boolean asEnum, String innerTypeName) { if (fields.isEmpty()) return; TypeDeclaration parent = (TypeDeclaration) typeNode.get(); - TypeDeclaration innerType = new TypeDeclaration(parent.compilationResult); - innerType.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; - innerType.modifiers = toEclipseModifier(level) | (asEnum ? ClassFileConstants.AccEnum : ClassFileConstants.AccStatic | ClassFileConstants.AccFinal); + EclipseNode fieldsType = findInnerClass(typeNode, innerTypeName); + boolean genConstr = false, genClinit = false; char[] name = innerTypeName.toCharArray(); - innerType.name = name; - innerType.traverse(new SetGeneratedByVisitor(source), (ClassScope) null); - EclipseNode innerNode = injectType(typeNode, innerType); - - ConstructorDeclaration constructor = new ConstructorDeclaration(parent.compilationResult); - constructor.selector = name; - constructor.declarationSourceStart = constructor.sourceStart = source.sourceStart; - constructor.declarationSourceEnd = constructor.sourceEnd = source.sourceEnd; - constructor.modifiers = ClassFileConstants.AccPrivate; - ExplicitConstructorCall superCall = new ExplicitConstructorCall(0); - superCall.sourceStart = source.sourceStart; - superCall.sourceEnd = source.sourceEnd; - superCall.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; - constructor.constructorCall = superCall; - if (!asEnum) constructor.statements = new Statement[0]; - - injectMethod(innerNode, constructor); + TypeDeclaration generatedInnerType = null; + if (fieldsType == null) { + generatedInnerType = new TypeDeclaration(parent.compilationResult); + generatedInnerType.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; + generatedInnerType.modifiers = toEclipseModifier(level) | (asEnum ? ClassFileConstants.AccEnum : ClassFileConstants.AccStatic | ClassFileConstants.AccFinal); + generatedInnerType.name = name; + fieldsType = injectType(typeNode, generatedInnerType); + genConstr = true; + genClinit = asEnum; + } else { + TypeDeclaration builderTypeDeclaration = (TypeDeclaration) fieldsType.get(); + if ((builderTypeDeclaration.modifiers & (ClassFileConstants.AccEnum | ClassFileConstants.AccStatic)) == 0) { + errorNode.addError("Existing " + innerTypeName + " must be declared as an 'enum' or a 'static class'."); + return; + } + genConstr = constructorExists(fieldsType) == MemberExistsResult.NOT_EXISTS; + } - if (asEnum) injectMethod(innerNode, new Clinit(parent.compilationResult)); + if (genConstr) { + ConstructorDeclaration constructor = new ConstructorDeclaration(parent.compilationResult); + constructor.selector = name; + constructor.declarationSourceStart = constructor.sourceStart = source.sourceStart; + constructor.declarationSourceEnd = constructor.sourceEnd = source.sourceEnd; + constructor.modifiers = ClassFileConstants.AccPrivate; + ExplicitConstructorCall superCall = new ExplicitConstructorCall(0); + superCall.sourceStart = source.sourceStart; + superCall.sourceEnd = source.sourceEnd; + superCall.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; + constructor.constructorCall = superCall; + if (!asEnum) constructor.statements = new Statement[0]; + injectMethod(fieldsType, constructor); + } + if (genClinit) { + Clinit cli = new Clinit(parent.compilationResult); + injectMethod(fieldsType, cli); + } + for (EclipseNode fieldNode : fields) { FieldDeclaration field = (FieldDeclaration) fieldNode.get(); char[] fName = field.name; + if (fieldExists(new String(fName), fieldsType) != MemberExistsResult.NOT_EXISTS) continue; int pS = source.sourceStart, pE = source.sourceEnd; long p = (long) pS << 32 | pE; - FieldDeclaration fieldConstant = new FieldDeclaration(fName, pS, pE); - fieldConstant.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; - fieldConstant.modifiers = asEnum ? 0 : ClassFileConstants.AccPublic | ClassFileConstants.AccStatic | ClassFileConstants.AccFinal; - fieldConstant.type = asEnum ? null : new QualifiedTypeReference(TypeConstants.JAVA_LANG_STRING, new long[] {p, p, p}); + FieldDeclaration constantField = new FieldDeclaration(fName, pS, pE); + constantField.bits |= Eclipse.ECLIPSE_DO_NOT_TOUCH_FLAG; + constantField.modifiers = asEnum ? 0 : ClassFileConstants.AccPublic | ClassFileConstants.AccStatic | ClassFileConstants.AccFinal; + constantField.type = asEnum ? null : new QualifiedTypeReference(TypeConstants.JAVA_LANG_STRING, new long[] {p, p, p}); if (asEnum) { AllocationExpression ac = new AllocationExpression(); - ac.enumConstant = fieldConstant; + ac.enumConstant = constantField; ac.sourceStart = source.sourceStart; ac.sourceEnd = source.sourceEnd; - fieldConstant.initialization = ac; + constantField.initialization = ac; } else { - fieldConstant.initialization = new StringLiteral(field.name, pS, pE, 0); + constantField.initialization = new StringLiteral(field.name, pS, pE, 0); } - injectField(innerNode, fieldConstant); + constantField.traverse(new SetGeneratedByVisitor(source), null); + injectField(fieldsType, constantField); } } } diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index f4eabbe8..83a44cca 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -726,15 +726,6 @@ public class HandleBuilder extends JavacAnnotationHandler { injectMethod(builderType, newMethod); } - public JavacNode findInnerClass(JavacNode parent, String name) { - for (JavacNode child : parent.down()) { - if (child.getKind() != Kind.TYPE) continue; - JCClassDecl td = (JCClassDecl) child.get(); - if (td.name.contentEquals(name)) return child; - } - return null; - } - public JavacNode makeBuilderClass(boolean isStatic, JavacNode source, JavacNode tdParent, String builderClassName, List typeParams, JCAnnotation ast) { JavacTreeMaker maker = tdParent.getTreeMaker(); int modifiers = Flags.PUBLIC; diff --git a/src/core/lombok/javac/handlers/HandleFieldNameConstants.java b/src/core/lombok/javac/handlers/HandleFieldNameConstants.java index 5b120948..163a82b3 100644 --- a/src/core/lombok/javac/handlers/HandleFieldNameConstants.java +++ b/src/core/lombok/javac/handlers/HandleFieldNameConstants.java @@ -48,7 +48,6 @@ import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.tree.JCTree.JCStatement; import com.sun.tools.javac.tree.JCTree.JCTypeParameter; import com.sun.tools.javac.tree.JCTree.JCVariableDecl; -import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; import com.sun.tools.javac.util.List; import com.sun.tools.javac.util.Name; @@ -75,7 +74,7 @@ public class HandleFieldNameConstants extends JavacAnnotationHandler fields, boolean asEnum, String innerTypeName) { + private void createInnerTypeFieldNameConstants(JavacNode typeNode, JavacNode errorNode, JCTree pos, AccessLevel level, java.util.List fields, boolean asEnum, String innerTypeName) { if (fields.isEmpty()) return; JavacTreeMaker maker = typeNode.getTreeMaker(); JCModifiers mods = maker.Modifiers(toJavacModifier(level) | (asEnum ? Flags.ENUM : Flags.STATIC | Flags.FINAL)); Name fieldsName = typeNode.toName(innerTypeName); - JCClassDecl innerType = maker.ClassDef(mods, fieldsName, List.nil(), null, List.nil(), List.nil()); - JavacNode innerNode = injectType(typeNode, innerType); - JCModifiers genConstrMods = maker.Modifiers(Flags.GENERATEDCONSTR | (asEnum ? 0L : Flags.PRIVATE)); - JCBlock genConstrBody = maker.Block(0L, List.of(maker.Exec(maker.Apply(List.nil(), maker.Ident(typeNode.toName("super")), List.nil())))); - JCMethodDecl genConstr = maker.MethodDef(genConstrMods, typeNode.toName(""), null, List.nil(), List.nil(), List.nil(), genConstrBody, null); + JavacNode fieldsType = findInnerClass(typeNode, innerTypeName); + boolean genConstr = false; + if (fieldsType == null) { + JCClassDecl innerType = maker.ClassDef(mods, fieldsName, List.nil(), null, List.nil(), List.nil()); + fieldsType = injectType(typeNode, innerType); + recursiveSetGeneratedBy(innerType, pos, typeNode.getContext()); + genConstr = true; + } else { + JCClassDecl builderTypeDeclaration = (JCClassDecl) fieldsType.get(); + long f = builderTypeDeclaration.getModifiers().flags; + if ((f & (Flags.STATIC | Flags.ENUM)) == 0) { + errorNode.addError("Existing " + innerTypeName + " must be declared as an 'enum' or a 'static class'."); + return; + } + genConstr = constructorExists(fieldsType) == MemberExistsResult.NOT_EXISTS; + } - injectMethod(innerNode, genConstr); + if (genConstr) { + JCModifiers genConstrMods = maker.Modifiers(Flags.GENERATEDCONSTR | (asEnum ? 0L : Flags.PRIVATE)); + JCBlock genConstrBody = maker.Block(0L, List.of(maker.Exec(maker.Apply(List.nil(), maker.Ident(typeNode.toName("super")), List.nil())))); + JCMethodDecl c = maker.MethodDef(genConstrMods, typeNode.toName(""), null, List.nil(), List.nil(), List.nil(), genConstrBody, null); + recursiveSetGeneratedBy(c, pos, typeNode.getContext()); + injectMethod(fieldsType, c); + } + java.util.List generated = new ArrayList(); for (JavacNode field : fields) { - JCModifiers enumValueMods = maker.Modifiers(Flags.PUBLIC | Flags.STATIC | Flags.FINAL | (asEnum ? Flags.ENUM : 0L)); + Name fName = ((JCVariableDecl) field.get()).name; + if (fieldExists(fName.toString(), fieldsType) != MemberExistsResult.NOT_EXISTS) continue; + JCModifiers constantValueMods = maker.Modifiers(Flags.PUBLIC | Flags.STATIC | Flags.FINAL | (asEnum ? Flags.ENUM : 0L)); JCExpression returnType; JCExpression init; if (asEnum) { @@ -149,8 +168,11 @@ public class HandleFieldNameConstants extends JavacAnnotationHandler type, JavacNode node) { return findAnnotation(type, node, false); } @@ -1001,7 +1010,11 @@ public class JavacHandlerUtil { return injectField(typeNode, field, false); } - private static JavacNode injectField(JavacNode typeNode, JCVariableDecl field, boolean addGenerated) { + public static JavacNode injectField(JavacNode typeNode, JCVariableDecl field, boolean addGenerated) { + return injectField(typeNode, field, addGenerated, false); + } + + public static JavacNode injectField(JavacNode typeNode, JCVariableDecl field, boolean addGenerated, boolean specialEnumHandling) { JCClassDecl type = (JCClassDecl) typeNode.get(); if (addGenerated) { @@ -1015,7 +1028,7 @@ public class JavacHandlerUtil { boolean skip = false; if (insertBefore.head instanceof JCVariableDecl) { JCVariableDecl f = (JCVariableDecl) insertBefore.head; - if (isEnumConstant(f) || isGenerated(f)) skip = true; + if ((!specialEnumHandling && isEnumConstant(f)) || isGenerated(f)) skip = true; } else if (insertBefore.head instanceof JCMethodDecl) { if ((((JCMethodDecl) insertBefore.head).mods.flags & GENERATEDCONSTR) != 0) skip = true; } diff --git a/test/transform/resource/after-delombok/FieldNameConstantsBasic.java b/test/transform/resource/after-delombok/FieldNameConstantsBasic.java index 8be45d2b..0504327a 100644 --- a/test/transform/resource/after-delombok/FieldNameConstantsBasic.java +++ b/test/transform/resource/after-delombok/FieldNameConstantsBasic.java @@ -6,7 +6,7 @@ public class FieldNameConstantsBasic { String butPrintMePlease; @java.lang.SuppressWarnings("all") static final class Fields { - public static final java.lang.String butPrintMePlease = "butPrintMePlease"; public static final java.lang.String iAmADvdPlayer = "iAmADvdPlayer"; + public static final java.lang.String butPrintMePlease = "butPrintMePlease"; } } diff --git a/test/transform/resource/after-delombok/FieldNameConstantsHandrolled.java b/test/transform/resource/after-delombok/FieldNameConstantsHandrolled.java new file mode 100644 index 00000000..2677cfa0 --- /dev/null +++ b/test/transform/resource/after-delombok/FieldNameConstantsHandrolled.java @@ -0,0 +1,30 @@ +class FieldNameConstantsHandrolled1 { + int field1; + int alsoAField; + int thirdField; + public enum TypeTest { + alsoAField, thirdField, field1; + } +} +class FieldNameConstantsHandrolled2 { + int field1; + int alsoAField; + int thirdField; + public enum TypeTest { + alsoAField, thirdField, field1; + public String foo() { + return name(); + } + } +} + +class FieldNameConstantsHandrolled3 { + int field1; + int alsoAField; + int thirdField; + static class Fields { + public static final java.lang.String field1 = "field1"; + public static final java.lang.String thirdField = "thirdField"; + public static final int alsoAField = 5; + } +} \ No newline at end of file diff --git a/test/transform/resource/after-ecj/FieldNameConstantsBasic.java b/test/transform/resource/after-ecj/FieldNameConstantsBasic.java index 674dd602..5b58e7d7 100644 --- a/test/transform/resource/after-ecj/FieldNameConstantsBasic.java +++ b/test/transform/resource/after-ecj/FieldNameConstantsBasic.java @@ -2,8 +2,8 @@ import lombok.experimental.FieldNameConstants; import lombok.AccessLevel; public @FieldNameConstants(level = AccessLevel.PACKAGE) class FieldNameConstantsBasic { static final @java.lang.SuppressWarnings("all") class Fields { - public static final java.lang.String butPrintMePlease = "butPrintMePlease"; public static final java.lang.String iAmADvdPlayer = "iAmADvdPlayer"; + public static final java.lang.String butPrintMePlease = "butPrintMePlease"; () { } private @java.lang.SuppressWarnings("all") Fields() { diff --git a/test/transform/resource/after-ecj/FieldNameConstantsHandrolled.java b/test/transform/resource/after-ecj/FieldNameConstantsHandrolled.java new file mode 100644 index 00000000..6762fdd8 --- /dev/null +++ b/test/transform/resource/after-ecj/FieldNameConstantsHandrolled.java @@ -0,0 +1,59 @@ +import lombok.experimental.FieldNameConstants; +import lombok.AccessLevel; +@FieldNameConstants(asEnum = true,innerTypeName = "TypeTest") class FieldNameConstantsHandrolled1 { + public enum TypeTest { + field1(), + alsoAField(), + thirdField(), + () { + } + private @java.lang.SuppressWarnings("all") TypeTest() { + super(); + } + } + int field1; + int alsoAField; + int thirdField; + FieldNameConstantsHandrolled1() { + super(); + } +} +@FieldNameConstants(asEnum = true,innerTypeName = "TypeTest") class FieldNameConstantsHandrolled2 { + public enum TypeTest { + field1(), + alsoAField(), + thirdField(), + () { + } + public String foo() { + return name(); + } + private @java.lang.SuppressWarnings("all") TypeTest() { + super(); + } + } + int field1; + int alsoAField; + int thirdField; + FieldNameConstantsHandrolled2() { + super(); + } +} +@FieldNameConstants class FieldNameConstantsHandrolled3 { + static class Fields { + public static final java.lang.String field1 = "field1"; + public static final java.lang.String thirdField = "thirdField"; + public static final int alsoAField = 5; + () { + } + private @java.lang.SuppressWarnings("all") Fields() { + super(); + } + } + int field1; + int alsoAField; + int thirdField; + FieldNameConstantsHandrolled3() { + super(); + } +} diff --git a/test/transform/resource/before/FieldNameConstantsHandrolled.java b/test/transform/resource/before/FieldNameConstantsHandrolled.java new file mode 100644 index 00000000..b6765fb2 --- /dev/null +++ b/test/transform/resource/before/FieldNameConstantsHandrolled.java @@ -0,0 +1,33 @@ +import lombok.experimental.FieldNameConstants; +import lombok.AccessLevel; + +@FieldNameConstants(asEnum = true, innerTypeName = "TypeTest") +class FieldNameConstantsHandrolled1 { + int field1, alsoAField, thirdField; + + public enum TypeTest { + field1 + } +} + +@FieldNameConstants(asEnum = true, innerTypeName = "TypeTest") +class FieldNameConstantsHandrolled2 { + int field1, alsoAField, thirdField; + + public enum TypeTest { + field1; + + public String foo() { + return name(); + } + } +} + +@FieldNameConstants +class FieldNameConstantsHandrolled3 { + int field1, alsoAField, thirdField; + + static class Fields { + public static final int alsoAField = 5; + } +} diff --git a/website/templates/features/experimental/FieldNameConstants.html b/website/templates/features/experimental/FieldNameConstants.html index a6fac99d..283ebe55 100644 --- a/website/templates/features/experimental/FieldNameConstants.html +++ b/website/templates/features/experimental/FieldNameConstants.html @@ -44,6 +44,9 @@ <@f.smallPrint>

+ Starting with lombok v1.18.5, lombok will silently skip generating anything that already exists. You can define the inner Fields enum/class yourself, + in which case lombok will add all the enum constants / public static final fields you haven't written yourself. +

From lombok v1.16.22 to lombok v1.18.2, this feature generated constants inside the type directly; the name of these fields would for example turn field exampleFieldName into public static final String FIELD_EXAMPLE_FIELD_NAME = "exampleFieldName";. The prefix and suffix (here, FIELD_, and the empty string) were configurable. Starting with lombok v1.18.4 this feature has been redesigned into generating an inner type as described above.

Any parameters of lombok annotations that take strings need to be supplied actual string literals; you cannot have references to constants like those generated by @FieldNameConstants. If you'd like to use @FieldNameConstants to for example fill in the of and/or exclude parameters of @ToString and similar lombok annotations, use the @ToString.Include / @ToString.Exclude etc system instead; these are described at the feature pages for those features. -- cgit