[Fixes #1656] Lombok would silently do the wrong thing when using references to `public static final String` fields, instead of actual string literals, there where you can specify strings in lombok annotation parameters, such as `@ToString(of = MyClass.CONSTANT_FIELD)`. We can’t really fix it, but at least now lombok will error when you do that and describe in detail what’s going wrong.

9 files changed, 87 insertions, 40 deletions

diff --git a/src/core/lombok/core/AnnotationValues.java b/src/core/lombok/core/AnnotationValues.java
index df426a65..8db7c857 100644
--- a/src/core/lombok/core/AnnotationValues.java
+++ b/src/core/lombok/core/AnnotationValues.java
@@ -43,6 +43,30 @@ public class AnnotationValues<A extends Annotation> {
 	private final Map<String, AnnotationValue> values;
 	private final LombokNode<?, ?, ?> ast;
 	
+	public static class ClassLiteral {
+		private final String className;
+		
+		public ClassLiteral(String className) {
+			this.className = className;
+		}
+		
+		public String getClassName() {
+			return className;
+		}
+	}
+	
+	public static class FieldSelect {
+		private final String finalPart;
+		
+		public FieldSelect(String finalPart) {
+			this.finalPart = finalPart;
+		}
+		
+		public String getFinalPart() {
+			return finalPart;
+		}
+	}
+	
 	/**
 	 * Represents a single method on the annotation class. For example, the value() method on the Getter annotation.
 	 */
@@ -50,8 +74,7 @@ public class AnnotationValues<A extends Annotation> {
 		/** A list of the raw expressions. List is size 1 unless an array is provided. */
 		public final List<String> raws;
 		
-		/** Guesses for each raw expression. If the raw expression is a literal expression, the guess will
-		 * likely be right. If not, it'll be wrong. */
+		/** Guesses for each raw expression. It's 'primitive' (String or primitive), an AV.ClassLiteral, an AV.FieldSelect, or an array of one of those. */
 		public final List<Object> valueGuesses;
 		
 		/** A list of the actual expressions. List is size 1 unless an array is provided. */
@@ -190,7 +213,7 @@ public class AnnotationValues<A extends Annotation> {
 				
 				if (!isArray && v.valueGuesses.size() > 1) {
 					throw new AnnotationValueDecodeFail(v, 
-							"Expected a single value, but " + method.getName() + " has an array of values", -1);
+						"Expected a single value, but " + method.getName() + " has an array of values", -1);
 				}
 				
 				if (v.valueGuesses.size() == 0 && !isArray) {
@@ -217,7 +240,7 @@ public class AnnotationValues<A extends Annotation> {
 							return defaultValue;
 						} 
 						throw new AnnotationValueDecodeFail(v, 
-								"I can't make sense of this annotation value. Try using a fully qualified literal.", idx);
+							"I can't make sense of this annotation value. Try using a fully qualified literal.", idx);
 					}
 					Array.set(array, idx++, result);
 				}
@@ -232,46 +255,46 @@ public class AnnotationValues<A extends Annotation> {
 	private Object guessToType(Object guess, Class<?> expected, AnnotationValue v, int pos) {
 		if (expected == int.class) {
 			if (guess instanceof Integer || guess instanceof Short || guess instanceof Byte) {
-				return ((Number)guess).intValue();
+				return ((Number) guess).intValue();
 			}
 		}
 		
 		if (expected == long.class) {
 			if (guess instanceof Long || guess instanceof Integer || guess instanceof Short || guess instanceof Byte) {
-				return ((Number)guess).longValue();
+				return ((Number) guess).longValue();
 			}
 		}
 		
 		if (expected == short.class) {
 			if (guess instanceof Integer || guess instanceof Short || guess instanceof Byte) {
-				int intVal = ((Number)guess).intValue();
-				int shortVal = ((Number)guess).shortValue();
+				int intVal = ((Number) guess).intValue();
+				int shortVal = ((Number) guess).shortValue();
 				if (shortVal == intVal) return shortVal;
 			}
 		}
 		
 		if (expected == byte.class) {
 			if (guess instanceof Integer || guess instanceof Short || guess instanceof Byte) {
-				int intVal = ((Number)guess).intValue();
-				int byteVal = ((Number)guess).byteValue();
+				int intVal = ((Number) guess).intValue();
+				int byteVal = ((Number) guess).byteValue();
 				if (byteVal == intVal) return byteVal;
 			}
 		}
 		
 		if (expected == double.class) {
-			if (guess instanceof Number) return ((Number)guess).doubleValue();
+			if (guess instanceof Number) return ((Number) guess).doubleValue();
 		}
 		
 		if (expected == float.class) {
-			if (guess instanceof Number) return ((Number)guess).floatValue();
+			if (guess instanceof Number) return ((Number) guess).floatValue();
 		}
 		
 		if (expected == boolean.class) {
-			if (guess instanceof Boolean) return ((Boolean)guess).booleanValue();
+			if (guess instanceof Boolean) return ((Boolean) guess).booleanValue();
 		}
 		
 		if (expected == char.class) {
-			if (guess instanceof Character) return ((Character)guess).charValue();
+			if (guess instanceof Character) return ((Character) guess).charValue();
 		}
 		
 		if (expected == String.class) {
@@ -279,31 +302,36 @@ public class AnnotationValues<A extends Annotation> {
 		}
 		
 		if (Enum.class.isAssignableFrom(expected) ) {
-			if (guess instanceof String) {
+			if (guess instanceof FieldSelect) {
+				String fieldSel = ((FieldSelect) guess).getFinalPart();
 				for (Object enumConstant : expected.getEnumConstants()) {
-					String target = ((Enum<?>)enumConstant).name();
-					if (target.equals(guess)) return enumConstant;
+					String target = ((Enum<?>) enumConstant).name();
+					if (target.equals(fieldSel)) return enumConstant;
 				}
 				throw new AnnotationValueDecodeFail(v,
-						"Can't translate " + guess + " to an enum of type " + expected, pos);
+					"Can't translate " + fieldSel + " to an enum of type " + expected, pos);
 			}
 		}
 		
 		if (Class.class == expected) {
-			if (guess instanceof String) try {
-				return Class.forName(toFQ((String)guess));
+			if (guess instanceof ClassLiteral) try {
+				String classLit = ((ClassLiteral) guess).getClassName();
+				return Class.forName(toFQ(classLit));
 			} catch (ClassNotFoundException e) {
 				throw new AnnotationValueDecodeFail(v,
-						"Can't translate " + guess + " to a class object.", pos);
+					"Can't translate " + guess + " to a class object.", pos);
 			}
 		}
 		
 		if (guess instanceof AnnotationValues) {
-			return ((AnnotationValues) guess).getInstance();
+			return ((AnnotationValues<?>) guess).getInstance();
 		}
 		
+		if (guess instanceof FieldSelect) throw new AnnotationValueDecodeFail(v,
+			"You must use constant literals in lombok annotations; they cannot be references to (static) fields.", pos);
+		
 		throw new AnnotationValueDecodeFail(v,
-				"Can't translate a " + guess.getClass() + " to the expected " + expected, pos);
+			"Can't translate a " + guess.getClass() + " to the expected " + expected, pos);
 	}
 	
 	/**
diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
index c49107e5..2e402c7e 100644
--- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
+++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
@@ -746,7 +746,7 @@ public class EclipseHandlerUtil {
 	 * Provides AnnotationValues with the data it needs to do its thing.
 	 */
 	public static <A extends java.lang.annotation.Annotation> AnnotationValues<A>
-			createAnnotation(Class<A> type, final EclipseNode annotationNode) {
+		createAnnotation(Class<A> type, final EclipseNode annotationNode) {
 		
 		final Annotation annotation = (Annotation) annotationNode.get();
 		Map<String, AnnotationValue> values = new HashMap<String, AnnotationValue>();
@@ -763,7 +763,7 @@ public class EclipseHandlerUtil {
 			String mName = (n == null || n.length == 0) ? "value" : new String(pair.name);
 			final Expression rhs = pair.value;
 			if (rhs instanceof ArrayInitializer) {
-				expressions = ((ArrayInitializer)rhs).expressions;
+				expressions = ((ArrayInitializer) rhs).expressions;
 			} else if (rhs != null) {
 				expressions = new Expression[] { rhs };
 			}
diff --git a/src/core/lombok/javac/JavacAST.java b/src/core/lombok/javac/JavacAST.java
index 12d919af..4ca2c050 100644
--- a/src/core/lombok/javac/JavacAST.java
+++ b/src/core/lombok/javac/JavacAST.java
@@ -396,7 +396,7 @@ public class JavacAST extends AST<JavacAST, JavacNode, JCTree> {
 			oldSource = log.useSource(newSource);
 			if (pos == null) pos = astObject.pos();
 		}
-		if (pos != null && attemptToRemoveErrorsInRange) {
+		if (pos != null && node != null && attemptToRemoveErrorsInRange) {
 			removeFromDeferredDiagnostics(pos.getStartPosition(), node.getEndPosition(pos));
 		}
 		try {
diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
index fda1a5d2..1976548c 100644
--- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java
+++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
@@ -360,7 +360,10 @@ public class JavacHandlerUtil {
 					expressions.add(inner);
 					if (inner instanceof JCAnnotation) {
 						try {
-							guesses.add(createAnnotation((Class<A>) Class.forName(inner.type.toString()), (JCAnnotation) inner, node));
+							@SuppressWarnings("unchecked")
+							Class<A> innerClass = (Class<A>) Class.forName(inner.type.toString());
+							
+							guesses.add(createAnnotation(innerClass, (JCAnnotation) inner, node));
 						} catch (ClassNotFoundException ex) {
 							throw new IllegalStateException(ex);
 						}
diff --git a/src/utils/lombok/eclipse/Eclipse.java b/src/utils/lombok/eclipse/Eclipse.java
index 18b22256..296c0a19 100644
--- a/src/utils/lombok/eclipse/Eclipse.java
+++ b/src/utils/lombok/eclipse/Eclipse.java
@@ -44,6 +44,8 @@ import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
 
+import lombok.core.AnnotationValues;
+
 public class Eclipse {
 	private static final Annotation[] EMPTY_ANNOTATIONS_ARRAY = new Annotation[0];
 	/**
@@ -161,7 +163,7 @@ public class Eclipse {
 	 */
 	public static Object calculateValue(Expression e) {
 		if (e instanceof Literal) {
-			((Literal)e).computeConstant();
+			((Literal) e).computeConstant();
 			switch (e.constant.typeID()) {
 			case TypeIds.T_int: return e.constant.intValue();
 			case TypeIds.T_byte: return e.constant.byteValue();
@@ -175,13 +177,13 @@ public class Eclipse {
 			default: return null;
 			}
 		} else if (e instanceof ClassLiteralAccess) {
-			return Eclipse.toQualifiedName(((ClassLiteralAccess)e).type.getTypeName());
+			return new AnnotationValues.ClassLiteral(Eclipse.toQualifiedName(((ClassLiteralAccess)e).type.getTypeName()));
 		} else if (e instanceof SingleNameReference) {
-			return new String(((SingleNameReference)e).token);
+			return new AnnotationValues.FieldSelect(new String(((SingleNameReference)e).token));
 		} else if (e instanceof QualifiedNameReference) {
 			String qName = Eclipse.toQualifiedName(((QualifiedNameReference)e).tokens);
 			int idx = qName.lastIndexOf('.');
-			return idx == -1 ? qName : qName.substring(idx+1);
+			return new AnnotationValues.FieldSelect(idx == -1 ? qName : qName.substring(idx+1));
 		}
 		
 		return null;
diff --git a/src/utils/lombok/javac/Javac.java b/src/utils/lombok/javac/Javac.java
index 9ff4d22f..2a66baba 100644
--- a/src/utils/lombok/javac/Javac.java
+++ b/src/utils/lombok/javac/Javac.java
@@ -36,6 +36,7 @@ import javax.lang.model.type.NoType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeVisitor;
 
+import lombok.core.AnnotationValues;
 import lombok.javac.JavacTreeMaker.TreeTag;
 import lombok.javac.JavacTreeMaker.TypeTag;
 
@@ -143,16 +144,17 @@ public class Javac {
 				return ((Number) lit.value).intValue() == 0 ? false : true;
 			}
 			return lit.value;
-		} else if (expr instanceof JCIdent || expr instanceof JCFieldAccess) {
+		}
+		
+		if (expr instanceof JCIdent || expr instanceof JCFieldAccess) {
 			String x = expr.toString();
-			if (x.endsWith(".class")) x = x.substring(0, x.length() - 6);
-			else {
-				int idx = x.lastIndexOf('.');
-				if (idx > -1) x = x.substring(idx + 1);
-			}
-			return x;
-		} else
-			return null;
+			if (x.endsWith(".class")) return new AnnotationValues.ClassLiteral(x.substring(0, x.length() - 6));
+			int idx = x.lastIndexOf('.');
+			if (idx > -1) x = x.substring(idx + 1);
+			return new AnnotationValues.FieldSelect(x);
+		}
+		
+		return null;
 	}
 	
 	public static final TypeTag CTC_BOOLEAN = typeTag("BOOLEAN");
diff --git a/test/transform/resource/before/ToStringWithConstantRefInOf.java b/test/transform/resource/before/ToStringWithConstantRefInOf.java
new file mode 100644
index 00000000..6246dcaf
--- /dev/null
+++ b/test/transform/resource/before/ToStringWithConstantRefInOf.java
@@ -0,0 +1,10 @@
+//skip compare contents
+import lombok.ToString;
+
+@ToString(of = ToStringWithConstantRefInOf.FIELD_NAME)
+public class ToStringWithConstantRefInOf {
+	static final String FIELD_NAME = "id";
+	private String id;
+	private int whatever;
+}
+
diff --git a/test/transform/resource/messages-delombok/ToStringWithConstantRefInOf.java.messages b/test/transform/resource/messages-delombok/ToStringWithConstantRefInOf.java.messages
new file mode 100644
index 00000000..d88e2754
--- /dev/null
+++ b/test/transform/resource/messages-delombok/ToStringWithConstantRefInOf.java.messages
@@ -0,0 +1 @@
+4 You must use constant literals in lombok annotations; they cannot be references to (static) fields.
diff --git a/test/transform/resource/messages-ecj/ToStringWithConstantRefInOf.java.messages b/test/transform/resource/messages-ecj/ToStringWithConstantRefInOf.java.messages
new file mode 100644
index 00000000..d88e2754
--- /dev/null
+++ b/test/transform/resource/messages-ecj/ToStringWithConstantRefInOf.java.messages
@@ -0,0 +1 @@
+4 You must use constant literals in lombok annotations; they cannot be references to (static) fields.