Skip to content

Commit

Permalink
[fixes #1981][fixes #1961] Hardcoded some knowledge about how to copy…
Browse files Browse the repository at this point in the history
… jackson’s `@JsonProperty`.
  • Loading branch information
rzwitserloot committed May 6, 2019
1 parent e69a991 commit 42f36e6
Show file tree
Hide file tree
Showing 15 changed files with 209 additions and 18 deletions.
8 changes: 8 additions & 0 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ lombok.launch.AnnotationProcessorHider$ClaimingProcessor,isolating</echo>
<srcdir dir="test/core/src" test="true" />
<srcdir dir="test/bytecode/src" test="true" />
<srcdir dir="test/configuration/src" test="true" />
<srcdir dir="test/stubs" test="true" />
</module>
<settings>
<url url="https://projectlombok.org/downloads/lombok.intellij.settings" />
Expand Down Expand Up @@ -432,6 +433,7 @@ lombok.launch.AnnotationProcessorHider$ClaimingProcessor,isolating</echo>
<srcdir dir="test/core/src" />
<srcdir dir="test/bytecode/src" />
<srcdir dir="test/configuration/src" />
<srcdir dir="test/stubs" />
<conf name="${eclipse.build.configname}" sources="contrib" />
<conf name="test" sources="contrib" />
<local org="org.projectlombok" name="lombok.patcher" dir="../lombok.patcher" />
Expand Down Expand Up @@ -551,6 +553,10 @@ ${sourceWarning}</echo>
<src path="test/bytecode/src" />
<src path="test/configuration/src" />
</ivy:compile>
<mkdir dir="build/teststubs" />
<ivy:compile destdir="build/teststubs" source="1.6" target="1.6" ecj="true" nowarn="true">
<src path="test/stubs" />
</ivy:compile>
</target>

<target name="test-ecj" depends="dist, contrib, setupJavaOracle8TestEnvironment" unless="tests.skip">
Expand Down Expand Up @@ -728,6 +734,7 @@ You can also create your own by writing a 'testenvironment.properties' file. The
<classpath path="${test.location.javac}" />
<classpath path="build/lombok" />
<classpath path="build/tests" />
<classpath path="build/teststubs" />
<batchtest>
<fileset dir="test/core/src">
<include name="lombok/RunAllTests.java" />
Expand All @@ -747,6 +754,7 @@ You can also create your own by writing a 'testenvironment.properties' file. The
<classpath path="${test.location.javac}" />
<classpath path="build/lombok" />
<classpath path="build/tests" />
<classpath path="build/teststubs" />
<batchtest>
<fileset dir="test/core/src">
<include name="lombok/RunAllTests.java" />
Expand Down
1 change: 1 addition & 0 deletions doc/changelog.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Lombok Changelog
* FEATURE: You can now suppress generation of the `builder` method when using `@Builder`; usually because you're only interested in the `toBuilder` method. As a convenience we won't emit warnings about missing `@Builder.Default` annotations when you do this. [Issue #2046](https://github.com/rzwitserloot/lombok/issues/2046)
* FEATURE: You can now change the access modifier of generated builder classes. [Issue #2083](https://github.com/rzwitserloot/lombok/issues/2083).
* FEATURE: When using `@NonNull`, or any other annotation that would result in a null-check, you can configure to generate an assert statement instead. [Issue #2078](https://github.com/rzwitserloot/lombok/issues/2078).
* FEATURE: Lombok now knows exactly how to treat `@com.fasterxml.jackson.annotation.JsonProperty` and will copy it to the right places for example when making builders. [Issue #1961](https://github.com/rzwitserloot/lombok/issues/1961) [Issue #1981](https://github.com/rzwitserloot/lombok/issues/1981).
* PLATFORM: A few lombok features (most notably delombok) failed on JDK12. [Issue #2082](https://github.com/rzwitserloot/lombok/issues/2082)
* BUGFIX: var/val on methods that return an intersection type would now work in Eclipse. [Issue #1986](https://github.com/rzwitserloot/lombok/issues/1986)
* BUGFIX: Fix for java6 regression if a field has javadoc. [Issue #2066](https://github.com/rzwitserloot/lombok/issues/2066)
Expand Down
9 changes: 6 additions & 3 deletions src/core/lombok/core/handlers/HandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static int primeForNull() {
return 43;
}

public static final List<String> NONNULL_ANNOTATIONS, BASE_COPYABLE_ANNOTATIONS;
public static final List<String> NONNULL_ANNOTATIONS, BASE_COPYABLE_ANNOTATIONS, COPY_TO_SETTER_ANNOTATIONS;
static {
NONNULL_ANNOTATIONS = Collections.unmodifiableList(Arrays.asList(new String[] {
"android.annotation.NonNull",
Expand All @@ -92,7 +92,7 @@ public static int primeForNull() {
"org.jetbrains.annotations.NotNull",
"org.jmlspecs.annotation.NonNull",
"org.netbeans.api.annotations.common.NonNull",
"org.springframework.lang.NonNull"
"org.springframework.lang.NonNull",
}));
BASE_COPYABLE_ANNOTATIONS = Collections.unmodifiableList(Arrays.asList(new String[] {
"android.support.annotation.NonNull",
Expand Down Expand Up @@ -298,8 +298,11 @@ public static int primeForNull() {
"org.jetbrains.annotations.NotNull",
"org.jetbrains.annotations.Nullable",
"org.springframework.lang.NonNull",
"org.springframework.lang.Nullable"
"org.springframework.lang.Nullable",
}));
COPY_TO_SETTER_ANNOTATIONS = Collections.unmodifiableList(Arrays.asList(new String[] {
"com.fasterxml.jackson.annotation.JsonProperty",
}));
}

/** Checks if the given name is a valid identifier.
Expand Down
29 changes: 29 additions & 0 deletions src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,15 @@ public static Annotation[] copyAnnotations(ASTNode source, Annotation[]... allAn
return result == null ? null : result.toArray(new Annotation[0]);
}

public static Annotation[] mergeAnnotations(Annotation[] a, Annotation[] b) {
if (a == null || a.length == 0) return (b == null || b.length == 0) ? null : b;
if (b == null || b.length == 0) return a.length == 0 ? null : a;
Annotation[] c = new Annotation[a.length + b.length];
System.arraycopy(a, 0, c, 0, a.length);
System.arraycopy(b, 0, c, a.length, b.length);
return c;
}

public static boolean hasAnnotation(Class<? extends java.lang.annotation.Annotation> type, EclipseNode node) {
if (node == null) return false;
if (type == null) return false;
Expand Down Expand Up @@ -765,6 +774,26 @@ public static Annotation[] findCopyableAnnotations(EclipseNode node) {
return result.toArray(EMPTY_ANNOTATIONS_ARRAY);
}

/**
* Searches the given field node for annotations that are specifically intentioned to be copied to the setter.
*/
public static Annotation[] findCopyableToSetterAnnotations(EclipseNode node) {
AbstractVariableDeclaration avd = (AbstractVariableDeclaration) node.get();
if (avd.annotations == null) return EMPTY_ANNOTATIONS_ARRAY;
List<Annotation> result = new ArrayList<Annotation>();

for (Annotation annotation : avd.annotations) {
TypeReference typeRef = annotation.type;
if (typeRef != null && typeRef.getTypeName() != null) {
for (String bn : COPY_TO_SETTER_ANNOTATIONS) if (typeMatches(bn, node, typeRef)) {
result.add(annotation);
break;
}
}
}
return result.toArray(EMPTY_ANNOTATIONS_ARRAY);
}

/**
* Checks if the provided annotation type is likely to be the intended type for the given annotation node.
*
Expand Down
13 changes: 8 additions & 5 deletions src/core/lombok/eclipse/handlers/HandleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ private static final char[] prefixWith(char[] prefix, char[] name) {
}

for (BuilderFieldData bfd : builderFields) {
makeSetterMethodsForBuilder(builderType, bfd, annotationNode, fluent, chain, accessForInners);
makeSetterMethodsForBuilder(builderType, bfd, annotationNode, fluent, chain, accessForInners, bfd.originalFieldNode);
}

{
Expand Down Expand Up @@ -802,16 +802,16 @@ public void generateBuilderFields(EclipseNode builderType, List<BuilderFieldData

private static final AbstractMethodDeclaration[] EMPTY = {};

public void makeSetterMethodsForBuilder(EclipseNode builderType, BuilderFieldData bfd, EclipseNode sourceNode, boolean fluent, boolean chain, AccessLevel access) {
public void makeSetterMethodsForBuilder(EclipseNode builderType, BuilderFieldData bfd, EclipseNode sourceNode, boolean fluent, boolean chain, AccessLevel access, EclipseNode originalFieldNode) {
boolean deprecate = isFieldDeprecated(bfd.originalFieldNode);
if (bfd.singularData == null || bfd.singularData.getSingularizer() == null) {
makeSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.nameOfSetFlag, sourceNode, fluent, chain, bfd.annotations, access);
makeSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.nameOfSetFlag, sourceNode, fluent, chain, bfd.annotations, access, originalFieldNode);
} else {
bfd.singularData.getSingularizer().generateMethods(bfd.singularData, deprecate, builderType, fluent, chain, access);
}
}

private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] nameOfSetFlag, EclipseNode sourceNode, boolean fluent, boolean chain, Annotation[] annotations, AccessLevel access) {
private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] nameOfSetFlag, EclipseNode sourceNode, boolean fluent, boolean chain, Annotation[] annotations, AccessLevel access, EclipseNode originalFieldNode) {
TypeDeclaration td = (TypeDeclaration) builderType.get();
AbstractMethodDeclaration[] existing = td.methods;
if (existing == null) existing = EMPTY;
Expand All @@ -827,8 +827,11 @@ private void makeSimpleSetterMethodForBuilder(EclipseNode builderType, boolean d

String setterName = fluent ? fieldNode.getName() : HandlerUtil.buildAccessorName("set", fieldNode.getName());

List<Annotation> methodAnnsList = Collections.<Annotation>emptyList();
Annotation[] methodAnns = EclipseHandlerUtil.findCopyableToSetterAnnotations(originalFieldNode);
if (methodAnns != null && methodAnns.length > 0) methodAnnsList = Arrays.asList(methodAnns);
MethodDeclaration setter = HandleSetter.createSetter(td, deprecate, fieldNode, setterName, nameOfSetFlag, chain, toEclipseModifier(access),
sourceNode, Collections.<Annotation>emptyList(), annotations != null ? Arrays.asList(copyAnnotations(sourceNode.get(), annotations)) : Collections.<Annotation>emptyList());
sourceNode, methodAnnsList, annotations != null ? Arrays.asList(copyAnnotations(sourceNode.get(), annotations)) : Collections.<Annotation>emptyList());
injectMethod(builderType, setter);
}

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 @@ -220,7 +220,7 @@ static MethodDeclaration createSetter(TypeDeclaration parent, boolean deprecate,
if (isFieldDeprecated(fieldNode) || deprecate) {
deprecated = new Annotation[] { generateDeprecatedAnnotation(source) };
}
method.annotations = copyAnnotations(source, onMethod.toArray(new Annotation[0]), deprecated);
method.annotations = mergeAnnotations(copyAnnotations(source, onMethod.toArray(new Annotation[0]), deprecated), findCopyableToSetterAnnotations(fieldNode));
Argument param = new Argument(field.name, p, copyType(field.type, source), Modifier.FINAL);
param.sourceStart = pS; param.sourceEnd = pE;
method.arguments = new Argument[] { param };
Expand Down
9 changes: 6 additions & 3 deletions src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -886,13 +886,13 @@ private void generateSetterMethodsForBuilder(EclipseNode builderType, BuilderFie
};

if (bfd.singularData == null || bfd.singularData.getSingularizer() == null) {
generateSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.nameOfSetFlag, returnTypeMaker.make(), returnStatementMaker.make(), sourceNode, bfd.annotations);
generateSimpleSetterMethodForBuilder(builderType, deprecate, bfd.createdFields.get(0), bfd.nameOfSetFlag, returnTypeMaker.make(), returnStatementMaker.make(), sourceNode, bfd.annotations, bfd.originalFieldNode);
} else {
bfd.singularData.getSingularizer().generateMethods(bfd.singularData, deprecate, builderType, true, returnTypeMaker, returnStatementMaker, AccessLevel.PUBLIC);
}
}

private void generateSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] nameOfSetFlag, TypeReference returnType, Statement returnStatement, EclipseNode sourceNode, Annotation[] annosOnParam) {
private void generateSimpleSetterMethodForBuilder(EclipseNode builderType, boolean deprecate, EclipseNode fieldNode, char[] nameOfSetFlag, TypeReference returnType, Statement returnStatement, EclipseNode sourceNode, Annotation[] annosOnParam, EclipseNode originalFieldNode) {
TypeDeclaration td = (TypeDeclaration) builderType.get();
AbstractMethodDeclaration[] existing = td.methods;
if (existing == null) existing = EMPTY_METHODS;
Expand All @@ -908,8 +908,11 @@ private void generateSimpleSetterMethodForBuilder(EclipseNode builderType, boole

String setterName = fieldNode.getName();

List<Annotation> methodAnnsList = Collections.<Annotation>emptyList();
Annotation[] methodAnns = EclipseHandlerUtil.findCopyableToSetterAnnotations(originalFieldNode);
if (methodAnns != null && methodAnns.length > 0) methodAnnsList = Arrays.asList(methodAnns);
MethodDeclaration setter = HandleSetter.createSetter(td, deprecate, fieldNode, setterName, nameOfSetFlag, returnType, returnStatement, ClassFileConstants.AccPublic,
sourceNode, Collections.<Annotation>emptyList(), annosOnParam != null ? Arrays.asList(copyAnnotations(sourceNode.get(), annosOnParam)) : Collections.<Annotation>emptyList());
sourceNode, methodAnnsList, annosOnParam != null ? Arrays.asList(copyAnnotations(sourceNode.get(), annosOnParam)) : Collections.<Annotation>emptyList());
injectMethod(builderType, setter);
}

Expand Down
5 changes: 3 additions & 2 deletions src/core/lombok/javac/handlers/HandleBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -750,10 +750,11 @@ private void makeSimpleSetterMethodForBuilder(JavacNode builderType, boolean dep

JavacTreeMaker maker = fieldNode.getTreeMaker();

JCMethodDecl newMethod = HandleSetter.createSetter(toJavacModifier(access), deprecate, fieldNode, maker, setterName, nameOfSetFlag, chain, source, List.<JCAnnotation>nil(), annosOnParam);
List<JCAnnotation> methodAnns = JavacHandlerUtil.findCopyableToSetterAnnotations(originalFieldNode);
JCMethodDecl newMethod = HandleSetter.createSetter(toJavacModifier(access), deprecate, fieldNode, maker, setterName, nameOfSetFlag, chain, source, methodAnns, annosOnParam);
recursiveSetGeneratedBy(newMethod, source.get(), builderType.getContext());
copyJavadoc(originalFieldNode, newMethod, CopyJavadoc.SETTER);

injectMethod(builderType, newMethod);
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/lombok/javac/handlers/HandleSetter.java
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public static JCMethodDecl createSetter(long access, boolean deprecate, JavacNod
List<JCExpression> throwsClauses = List.nil();
JCExpression annotationMethodDefaultValue = null;

List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod);
List<JCAnnotation> annsOnMethod = mergeAnnotations(copyAnnotations(onMethod), findCopyableToSetterAnnotations(field));
if (isFieldDeprecated(field) || deprecate) {
annsOnMethod = annsOnMethod.prepend(treeMaker.Annotation(genJavaLangTypeRef(field, "Deprecated"), List.<JCExpression>nil()));
}
Expand Down
7 changes: 4 additions & 3 deletions src/core/lombok/javac/handlers/HandleSuperBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -861,13 +861,13 @@ private void generateSetterMethodsForBuilder(final JavacNode builderType, Builde
}};

if (fieldNode.singularData == null || fieldNode.singularData.getSingularizer() == null) {
generateSimpleSetterMethodForBuilder(builderType, deprecate, fieldNode.createdFields.get(0), fieldNode.nameOfSetFlag, source, true, returnTypeMaker.make(), returnStatementMaker.make(), fieldNode.annotations);
generateSimpleSetterMethodForBuilder(builderType, deprecate, fieldNode.createdFields.get(0), fieldNode.nameOfSetFlag, source, true, returnTypeMaker.make(), returnStatementMaker.make(), fieldNode.annotations, fieldNode.originalFieldNode);
} else {
fieldNode.singularData.getSingularizer().generateMethods(fieldNode.singularData, deprecate, builderType, source.get(), true, returnTypeMaker, returnStatementMaker, AccessLevel.PUBLIC);
}
}

private void generateSimpleSetterMethodForBuilder(JavacNode builderType, boolean deprecate, JavacNode fieldNode, Name nameOfSetFlag, JavacNode source, boolean fluent, JCExpression returnType, JCStatement returnStatement, List<JCAnnotation> annosOnParam) {
private void generateSimpleSetterMethodForBuilder(JavacNode builderType, boolean deprecate, JavacNode fieldNode, Name nameOfSetFlag, JavacNode source, boolean fluent, JCExpression returnType, JCStatement returnStatement, List<JCAnnotation> annosOnParam, JavacNode originalFieldNode) {
Name fieldName = ((JCVariableDecl) fieldNode.get()).name;

for (JavacNode child : builderType.down()) {
Expand All @@ -881,7 +881,8 @@ private void generateSimpleSetterMethodForBuilder(JavacNode builderType, boolean

JavacTreeMaker maker = fieldNode.getTreeMaker();

JCMethodDecl newMethod = HandleSetter.createSetter(Flags.PUBLIC, deprecate, fieldNode, maker, setterName, nameOfSetFlag, returnType, returnStatement, source, List.<JCAnnotation>nil(), annosOnParam);
List<JCAnnotation> methodAnns = JavacHandlerUtil.findCopyableToSetterAnnotations(originalFieldNode);
JCMethodDecl newMethod = HandleSetter.createSetter(Flags.PUBLIC, deprecate, fieldNode, maker, setterName, nameOfSetFlag, returnType, returnStatement, source, methodAnns, annosOnParam);

injectMethod(builderType, newMethod);
}
Expand Down
47 changes: 47 additions & 0 deletions src/core/lombok/javac/handlers/JavacHandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,44 @@ public static List<JCAnnotation> findCopyableAnnotations(JavacNode node) {
return result.toList();
}

/**
* Searches the given field node for annotations that are specifically intentioned to be copied to the setter.
*/
public static List<JCAnnotation> findCopyableToSetterAnnotations(JavacNode node) {
JCAnnotation anno = null;
String annoName = null;
for (JavacNode child : node.down()) {
if (child.getKind() == Kind.ANNOTATION) {
if (anno != null) {
annoName = "";
break;
}
JCAnnotation annotation = (JCAnnotation) child.get();
annoName = annotation.annotationType.toString();
anno = annotation;
}
}

if (annoName == null) return List.nil();

if (!annoName.isEmpty()) {
for (String bn : COPY_TO_SETTER_ANNOTATIONS) if (typeMatches(bn, node, anno.annotationType)) return List.of(anno);
}

ListBuffer<JCAnnotation> result = new ListBuffer<JCAnnotation>();
for (JavacNode child : node.down()) {
if (child.getKind() == Kind.ANNOTATION) {
JCAnnotation annotation = (JCAnnotation) child.get();
boolean match = false;
if (!match) for (String bn : COPY_TO_SETTER_ANNOTATIONS) if (typeMatches(bn, node, annotation.annotationType)) {
result.append(annotation);
break;
}
}
}
return result.toList();
}

/**
* Generates a new statement that checks if the given variable is null, and if so, throws a configured exception with the
* variable name as message.
Expand Down Expand Up @@ -1713,6 +1751,15 @@ static List<JCAnnotation> copyAnnotations(List<? extends JCExpression> in) {
return out.toList();
}

static List<JCAnnotation> mergeAnnotations(List<JCAnnotation> a, List<JCAnnotation> b) {
if (a == null || a.isEmpty()) return b;
if (b == null || b.isEmpty()) return a;
ListBuffer<JCAnnotation> out = new ListBuffer<JCAnnotation>();
for (JCAnnotation ann : a) out.append(ann);
for (JCAnnotation ann : b) out.append(ann);
return out.toList();
}

static boolean isClass(JavacNode typeNode) {
return isClassAndDoesNotHaveFlags(typeNode, Flags.INTERFACE | Flags.ENUM | Flags.ANNOTATION);
}
Expand Down
12 changes: 12 additions & 0 deletions test/stubs/com/fasterxml/jackson/annotation/JsonProperty.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.fasterxml.jackson.annotation;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target({ElementType.ANNOTATION_TYPE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
public @interface JsonProperty {
String value();
}

0 comments on commit 42f36e6

Please sign in to comment.