Fixed issue #778: problems with onX if the annotation to be added has…

… named args.

doc/changelog.markdown

@@ -4,6 +4,7 @@ Lombok Changelog
 ### v1.16.15 "Edgy Guinea Pig"
 * v1.16.14 is the latest stable release of Project Lombok.
 * PLATFORM: Improved jdk9 support, work in progress.
+* BUGFIX: The `onX` feature (which lets you add annotations to generated methods) did not work if the annotation you added contained named parameters, and you are compiling with JDK8's javac. We can't fix this (it's a bug in javac), but we have provided an alternate, prettier way to do `onX` on javac8+. [Issue #778](https://github.com/rzwitserloot/lombok/issues/778) [onX documentation](https://projectlombok.org/features/experimental/onX.html)
 
 ### v1.16.14 (February 10th, 2017)
 * FEATURE: Generated classes, methods and fields can now also annotated with `@lombok.Generated` [Issue #1014](https://github.com/rzwitserloot/lombok/issues/1014)<br>

src/core/lombok/AllArgsConstructor.java

@@ -49,7 +49,12 @@
 	String staticName() default "";
 
 	/**
-	 * Any annotations listed here are put on the generated constructor. The syntax for this feature is: {@code @AllArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated constructor.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @AllArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @AllArgsConstructor(onConstructor_={@AnnotationsGohere})} // note the underscore after {@code onConstructor}.
 	 */
 	AnyAnnotation[] onConstructor() default {};
 

src/core/lombok/EqualsAndHashCode.java

@@ -63,8 +63,13 @@
 	boolean doNotUseGetters() default false;
 
 	/**
-	 * Any annotations listed here are put on the generated parameter of {@code equals} and {@code canEqual}. The syntax for this feature is: {@code @EqualsAndHashCode(onParam=@__({@AnnotationsGoHere}))}
-	 * This is useful to add for example a {@code Nullable} annotation.
+	 * Any annotations listed here are put on the generated parameter of {@code equals} and {@code canEqual}.
+	 * This is useful to add for example a {@code Nullable} annotation.<br />
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @EqualsAndHashCode(onParam=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @EqualsAndHashCode(onParam_={@AnnotationsGohere})} // note the underscore after {@code onParam}.
 	 */
 	AnyAnnotation[] onParam() default {};
 

src/core/lombok/Getter.java

@@ -58,9 +58,14 @@
 	lombok.AccessLevel value() default lombok.AccessLevel.PUBLIC;
 
 	/**
-	 * Any annotations listed here are put on the generated method. The syntax for this feature is: {@code @Getter(onMethod=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated method.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @Getter(onMethod=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @Getter(onMethod_={@AnnotationsGohere})} // note the underscore after {@code onMethod}.
 	 */
-	AnyAnnotation[] onMethod() default @AnyAnnotation;
+	AnyAnnotation[] onMethod() default {};
 
 	boolean lazy() default false;
 

src/core/lombok/NoArgsConstructor.java

@@ -51,7 +51,12 @@
 	String staticName() default "";
 
 	/**
-	 * Any annotations listed here are put on the generated constructor. The syntax for this feature is: {@code @NoArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated constructor.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @NoArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @NoArgsConstructor(onConstructor_={@AnnotationsGohere})} // note the underscore after {@code onConstructor}.
 	 */
 	AnyAnnotation[] onConstructor() default {};
 

src/core/lombok/RequiredArgsConstructor.java

@@ -49,7 +49,12 @@
 	String staticName() default "";
 
 	/**
-	 * Any annotations listed here are put on the generated constructor. The syntax for this feature is: {@code @RequiredArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated constructor.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @RequiredArgsConstructor(onConstructor=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @RequiredArgsConstructor(onConstructor_={@AnnotationsGohere})} // note the underscore after {@code onConstructor}.
 	 */
 	AnyAnnotation[] onConstructor() default {};
 

src/core/lombok/Setter.java

@@ -59,12 +59,22 @@
 	lombok.AccessLevel value() default lombok.AccessLevel.PUBLIC;
 
 	/**
-	 * Any annotations listed here are put on the generated method. The syntax for this feature is: {@code @Setter(onMethod=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated method.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @Setter(onMethod=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @Setter(onMethod_={@AnnotationsGohere})} // note the underscore after {@code onMethod}.
 	 */
 	AnyAnnotation[] onMethod() default {};
 
 	/**
-	 * Any annotations listed here are put on the generated method's parameter. The syntax for this feature is: {@code @Setter(onParam=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated method's parameter.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @Setter(onParam=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @Setter(onParam_={@AnnotationsGohere})} // note the underscore after {@code onParam}.
 	 */
 	AnyAnnotation[] onParam() default {};
 

src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java

@@ -1616,6 +1616,14 @@ private static boolean isAllValidOnXCharacters(char[] in) {
 		return true;
 	}
 
+	public static void addError(String errorName, EclipseNode node) {
+		if (node.getLatestJavaSpecSupported() < 8) {
+			node.addError("The correct format is " + errorName + "_={@SomeAnnotation, @SomeOtherAnnotation})");
+		} else {
+			node.addError("The correct format is " + errorName + "=@__({@SomeAnnotation, @SomeOtherAnnotation}))");
+		}
+	}
+
 	public static List<Annotation> unboxAndRemoveAnnotationParameter(Annotation annotation, String annotationName, String errorName, EclipseNode errorNode) {
 		if ("value".equals(annotationName)) {
 			// We can't unbox this, because SingleMemberAnnotation REQUIRES a value, and this method
@@ -1638,51 +1646,70 @@ public static List<Annotation> unboxAndRemoveAnnotationParameter(Annotation anno
 
 		char[] nameAsCharArray = annotationName.toCharArray();
 
+		top:
 		for (int i = 0; i < pairs.length; i++) {
-			if (pairs[i].name == null || !Arrays.equals(nameAsCharArray, pairs[i].name)) continue;
+			boolean allowRaw;
+			char[] name = pairs[i].name;
+			if (name == null) continue;
+			if (name.length < nameAsCharArray.length) continue;
+			for (int j = 0; j < nameAsCharArray.length; j++) {
+				if (name[j] != nameAsCharArray[j]) continue top;
+			}
+			allowRaw = name.length > nameAsCharArray.length;
+			for (int j = nameAsCharArray.length; j < name.length; j++) {
+				if (name[j] != '_') continue top;
+			}
+			// If we're still here it's the targeted annotation param.
 			Expression value = pairs[i].value;
 			MemberValuePair[] newPairs = new MemberValuePair[pairs.length - 1];
 			if (i > 0) System.arraycopy(pairs, 0, newPairs, 0, i);
 			if (i < pairs.length - 1) System.arraycopy(pairs, i + 1, newPairs, i, pairs.length - i - 1);
 			normalAnnotation.memberValuePairs = newPairs;
-			// We have now removed the annotation parameter and stored '@__({... annotations ...})',
-			// which we must now unbox.
-			if (!(value instanceof Annotation)) {
-				errorNode.addError("The correct format is " + errorName + "@__({@SomeAnnotation, @SomeOtherAnnotation}))");
-				return Collections.emptyList();
-			}
-
-			Annotation atDummyIdentifier = (Annotation) value;
-			if (!(atDummyIdentifier.type instanceof SingleTypeReference) ||
-					!isAllValidOnXCharacters(((SingleTypeReference) atDummyIdentifier.type).token)) {
-				errorNode.addError("The correct format is " + errorName + "@__({@SomeAnnotation, @SomeOtherAnnotation}))");
-				return Collections.emptyList();
-			}
-
-			if (atDummyIdentifier instanceof MarkerAnnotation) {
-				// It's @Getter(onMethod=@__). This is weird, but fine.
-				return Collections.emptyList();
-			}
+			// We have now removed the annotation parameter and stored the value,
+			// which we must now unbox. It's either annotations, or @__(annotations).
 
 			Expression content = null;
 
-			if (atDummyIdentifier instanceof NormalAnnotation) {
-				MemberValuePair[] mvps = ((NormalAnnotation) atDummyIdentifier).memberValuePairs;
-				if (mvps == null || mvps.length == 0) {
-					// It's @Getter(onMethod=@__()). This is weird, but fine.
+			if (value instanceof ArrayInitializer) {
+				if (!allowRaw) {
+					addError(errorName, errorNode);
 					return Collections.emptyList();
 				}
-				if (mvps.length == 1 && Arrays.equals("value".toCharArray(), mvps[0].name)) {
-					content = mvps[0].value;
+				content = value;
+			} else if (!(value instanceof Annotation)) {
+				addError(errorName, errorNode);
+				return Collections.emptyList();
+			} else {
+				Annotation atDummyIdentifier = (Annotation) value;
+				if (atDummyIdentifier.type instanceof SingleTypeReference && isAllValidOnXCharacters(((SingleTypeReference) atDummyIdentifier.type).token)) {
+					if (atDummyIdentifier instanceof MarkerAnnotation) {
+						return Collections.emptyList();
+					} else if (atDummyIdentifier instanceof NormalAnnotation) {
+						MemberValuePair[] mvps = ((NormalAnnotation) atDummyIdentifier).memberValuePairs;
+						if (mvps == null || mvps.length == 0) {
+							return Collections.emptyList();
+						}
+						if (mvps.length == 1 && Arrays.equals("value".toCharArray(), mvps[0].name)) {
+							content = mvps[0].value;
+						}
+					} else if (atDummyIdentifier instanceof SingleMemberAnnotation) {
+						content = ((SingleMemberAnnotation) atDummyIdentifier).memberValue;
+					} else {
+						addError(errorName, errorNode);
+						return Collections.emptyList();
+					}
+				} else {
+					if (allowRaw) {
+						content = atDummyIdentifier;
+					} else {
+						addError(errorName, errorNode);
+						return Collections.emptyList();
+					}
 				}
 			}
 
-			if (atDummyIdentifier instanceof SingleMemberAnnotation) {
-				content = ((SingleMemberAnnotation) atDummyIdentifier).memberValue;
-			}
-
 			if (content == null) {
-				errorNode.addError("The correct format is " + errorName + "@__({@SomeAnnotation, @SomeOtherAnnotation}))");
+				addError(errorName, errorNode);
 				return Collections.emptyList();
 			}
 
@@ -1694,13 +1721,13 @@ public static List<Annotation> unboxAndRemoveAnnotationParameter(Annotation anno
 				if (expressions != null) for (Expression ex : expressions) {
 					if (ex instanceof Annotation) result.add((Annotation) ex);
 					else {
-						errorNode.addError("The correct format is " + errorName + "@__({@SomeAnnotation, @SomeOtherAnnotation}))");
+						addError(errorName, errorNode);
 						return Collections.emptyList();
 					}
 				}
 				return result;
 			} else {
-				errorNode.addError("The correct format is " + errorName + "@__({@SomeAnnotation, @SomeOtherAnnotation}))");
+				addError(errorName, errorNode);
 				return Collections.emptyList();
 			}
 		}

src/core/lombok/eclipse/handlers/HandleConstructor.java

@@ -93,7 +93,7 @@ public static class HandleNoArgsConstructor extends EclipseAnnotationHandler<NoA
 			boolean force = ann.force();
 
 			List<EclipseNode> fields = force ? findFinalFields(typeNode) : Collections.<EclipseNode>emptyList();
-			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@NoArgsConstructor(onConstructor=", annotationNode);
+			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@NoArgsConstructor(onConstructor", annotationNode);
 
 			new HandleConstructor().generateConstructor(typeNode, level, fields, force, staticName, SkipIfConstructorExists.NO, null, onConstructor, annotationNode);
 		}
@@ -117,7 +117,7 @@ public static class HandleRequiredArgsConstructor extends EclipseAnnotationHandl
 				suppressConstructorProperties = suppress;
 			}
 
-			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@RequiredArgsConstructor(onConstructor=", annotationNode);
+			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@RequiredArgsConstructor(onConstructor", annotationNode);
 
 			new HandleConstructor().generateConstructor(
 				typeNode, level, findRequiredFields(typeNode), false, staticName, SkipIfConstructorExists.NO,
@@ -179,7 +179,7 @@ public static class HandleAllArgsConstructor extends EclipseAnnotationHandler<Al
 				suppressConstructorProperties = suppress;
 			}
 
-			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@AllArgsConstructor(onConstructor=", annotationNode);
+			List<Annotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@AllArgsConstructor(onConstructor", annotationNode);
 
 			new HandleConstructor().generateConstructor(
 				typeNode, level, findAllFields(typeNode), false, staticName, SkipIfConstructorExists.NO,

src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java

@@ -129,7 +129,7 @@ public void generateEqualsAndHashCodeForType(EclipseNode typeNode, EclipseNode e
 		List<String> includes = Arrays.asList(ann.of());
 		EclipseNode typeNode = annotationNode.up();
 
-		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@EqualsAndHashCode(onParam=", annotationNode);
+		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@EqualsAndHashCode(onParam", annotationNode);
 		checkForBogusFieldNames(typeNode, annotation);
 
 		Boolean callSuper = ann.callSuper();

src/core/lombok/eclipse/handlers/HandleGetter.java

@@ -148,7 +148,7 @@ public void handle(AnnotationValues<Getter> annotation, Annotation ast, EclipseN
 
 		if (node == null) return;
 
-		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Getter(onMethod=", annotationNode);
+		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Getter(onMethod", annotationNode);
 
 		switch (node.getKind()) {
 		case FIELD:

src/core/lombok/eclipse/handlers/HandleSetter.java

@@ -125,8 +125,8 @@ public void handle(AnnotationValues<Setter> annotation, Annotation ast, EclipseN
 		AccessLevel level = annotation.getInstance().value();
 		if (level == AccessLevel.NONE || node == null) return;
 
-		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Setter(onMethod=", annotationNode);
-		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@Setter(onParam=", annotationNode);
+		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Setter(onMethod", annotationNode);
+		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@Setter(onParam", annotationNode);
 
 		switch (node.getKind()) {
 		case FIELD:

src/core/lombok/eclipse/handlers/HandleWither.java

@@ -127,8 +127,8 @@ public void generateWitherForField(EclipseNode fieldNode, EclipseNode sourceNode
 		AccessLevel level = annotation.getInstance().value();
 		if (level == AccessLevel.NONE || node == null) return;
 
-		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Wither(onMethod=", annotationNode);
-		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@Wither(onParam=", annotationNode);
+		List<Annotation> onMethod = unboxAndRemoveAnnotationParameter(ast, "onMethod", "@Wither(onMethod", annotationNode);
+		List<Annotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@Wither(onParam", annotationNode);
 
 		switch (node.getKind()) {
 		case FIELD:

src/core/lombok/experimental/Wither.java

@@ -60,12 +60,22 @@
 	AccessLevel value() default AccessLevel.PUBLIC;
 
 	/**
-	 * Any annotations listed here are put on the generated method. The syntax for this feature is: {@code @Setter(onMethod=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated method.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @Wither(onMethod=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @Wither(onMethod_={@AnnotationsGohere})} // note the underscore after {@code onMethod}.
 	 */
 	AnyAnnotation[] onMethod() default {};
 
 	/**
-	 * Any annotations listed here are put on the generated method's parameter. The syntax for this feature is: {@code @Setter(onParam=@__({@AnnotationsGoHere}))}
+	 * Any annotations listed here are put on the generated method's parameter.
+	 * The syntax for this feature depends on JDK version (nothing we can do about that; it's to work around javac bugs).<br />
+	 * up to JDK7:<br />
+	 *  {@code @Wither(onParam=@__({@AnnotationsGoHere}))}<br />
+	 * from JDK8:<br />
+	 *  {@code @Wither(onParam_={@AnnotationsGohere})} // note the underscore after {@code onParam}.
 	 */
 	AnyAnnotation[] onParam() default {};
 

src/core/lombok/javac/handlers/HandleConstructor.java

@@ -71,7 +71,7 @@ public static class HandleNoArgsConstructor extends JavacAnnotationHandler<NoArg
 			deleteImportFromCompilationUnit(annotationNode, "lombok.AccessLevel");
 			JavacNode typeNode = annotationNode.up();
 			if (!checkLegality(typeNode, annotationNode, NoArgsConstructor.class.getSimpleName())) return;
-			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@NoArgsConstructor(onConstructor=", annotationNode);
+			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@NoArgsConstructor(onConstructor", annotationNode);
 			NoArgsConstructor ann = annotation.getInstance();
 			AccessLevel level = ann.access();
 			if (level == AccessLevel.NONE) return;
@@ -91,7 +91,7 @@ public static class HandleRequiredArgsConstructor extends JavacAnnotationHandler
 			deleteImportFromCompilationUnit(annotationNode, "lombok.AccessLevel");
 			JavacNode typeNode = annotationNode.up();
 			if (!checkLegality(typeNode, annotationNode, RequiredArgsConstructor.class.getSimpleName())) return;
-			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@RequiredArgsConstructor(onConstructor=", annotationNode);
+			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@RequiredArgsConstructor(onConstructor", annotationNode);
 			RequiredArgsConstructor ann = annotation.getInstance();
 			AccessLevel level = ann.access();
 			if (level == AccessLevel.NONE) return;
@@ -141,7 +141,7 @@ public static class HandleAllArgsConstructor extends JavacAnnotationHandler<AllA
 			deleteImportFromCompilationUnit(annotationNode, "lombok.AccessLevel");
 			JavacNode typeNode = annotationNode.up();
 			if (!checkLegality(typeNode, annotationNode, AllArgsConstructor.class.getSimpleName())) return;
-			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@AllArgsConstructor(onConstructor=", annotationNode);
+			List<JCAnnotation> onConstructor = unboxAndRemoveAnnotationParameter(ast, "onConstructor", "@AllArgsConstructor(onConstructor", annotationNode);
 			AllArgsConstructor ann = annotation.getInstance();
 			AccessLevel level = ann.access();
 			if (level == AccessLevel.NONE) return;

src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java

@@ -95,7 +95,7 @@ public void checkForBogusFieldNames(JavacNode type, AnnotationValues<EqualsAndHa
 		List<String> excludes = List.from(ann.exclude());
 		List<String> includes = List.from(ann.of());
 		JavacNode typeNode = annotationNode.up();
-		List<JCAnnotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@EqualsAndHashCode(onParam=", annotationNode);
+		List<JCAnnotation> onParam = unboxAndRemoveAnnotationParameter(ast, "onParam", "@EqualsAndHashCode(onParam", annotationNode);
 		checkForBogusFieldNames(typeNode, annotation);
 
 		Boolean callSuper = ann.callSuper();