From f540335ef972d84f02efba6dcaf608aec0e19129 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Tue, 17 Apr 2018 04:19:37 +0200 Subject: [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. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/lombok/core/AnnotationValues.java | 74 +++++++++++++++------- .../eclipse/handlers/EclipseHandlerUtil.java | 4 +- src/core/lombok/javac/JavacAST.java | 2 +- .../lombok/javac/handlers/JavacHandlerUtil.java | 5 +- 4 files changed, 58 insertions(+), 27 deletions(-) (limited to 'src/core/lombok') 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 { private final Map 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 list of the raw expressions. List is size 1 unless an array is provided. */ public final List 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 valueGuesses; /** A list of the actual expressions. List is size 1 unless an array is provided. */ @@ -190,7 +213,7 @@ public class AnnotationValues { 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 { 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 { 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 { } 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 AnnotationValues - createAnnotation(Class type, final EclipseNode annotationNode) { + createAnnotation(Class type, final EclipseNode annotationNode) { final Annotation annotation = (Annotation) annotationNode.get(); Map values = new HashMap(); @@ -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 { 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) Class.forName(inner.type.toString()), (JCAnnotation) inner, node)); + @SuppressWarnings("unchecked") + Class innerClass = (Class) Class.forName(inner.type.toString()); + + guesses.add(createAnnotation(innerClass, (JCAnnotation) inner, node)); } catch (ClassNotFoundException ex) { throw new IllegalStateException(ex); } -- cgit