[fixes #2019] Lombok now properly deals with `@NonNull` specifically on the ‘type use’ of a parameter (and, in case of arrays, on the outermost dimension which is actually the first one listed. Weird corner case of the JLS).

6 files changed, 161 insertions, 23 deletions

diff --git a/src/core/lombok/eclipse/EclipseAST.java b/src/core/lombok/eclipse/EclipseAST.java
index 1ba26338..e724fb50 100644
--- a/src/core/lombok/eclipse/EclipseAST.java
+++ b/src/core/lombok/eclipse/EclipseAST.java
@@ -22,6 +22,7 @@
 package lombok.eclipse;
 
 import java.io.File;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URI;
@@ -49,8 +50,12 @@ import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.ImportReference;
 import org.eclipse.jdt.internal.compiler.ast.Initializer;
 import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.ParameterizedQualifiedTypeReference;
+import org.eclipse.jdt.internal.compiler.ast.ParameterizedSingleTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
+import org.eclipse.jdt.internal.compiler.ast.Wildcard;
 
 /**
  * Wraps around Eclipse's internal AST view to add useful features as well as the ability to visit parents from children,
@@ -292,7 +297,7 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 			throw Lombok.sneakyThrow(e);
 		} catch (NullPointerException e) {
 			if (!"false".equals(System.getProperty("lombok.debug.reflection", "false"))) {
-				e.initCause(EcjReflectionCheck.problem);
+				e.initCause(EcjReflectionCheck.problemAddProblemToCompilationResult);
 				throw e;
 			}
 			//ignore, we don't have access to the correct ECJ classes, so lombok can't possibly
@@ -300,6 +305,25 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 		}
 	}
 	
+	public static Annotation[] getTopLevelTypeReferenceAnnotations(TypeReference tr) {
+		Method m = EcjReflectionCheck.typeReferenceGetAnnotationsOnDimensions;
+		if (m == null) return null;
+		Annotation[][] annss = null;
+		try {
+			annss = (Annotation[][]) m.invoke(tr);
+			if (annss != null) return annss[0];
+		} catch (Throwable ignore) {}
+		
+		try {
+			Field f = EcjReflectionCheck.typeReferenceAnnotations;
+			if (f == null) return null;
+			annss = (Annotation[][]) f.get(tr);
+			return annss[annss.length - 1];
+		} catch (Throwable t) {
+			return null;
+		}
+	}
+	
 	private final CompilationUnitDeclaration compilationUnitDeclaration;
 	private boolean completeParse;
 	
@@ -350,6 +374,8 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 			return buildStatement((Statement) node);
 		case ANNOTATION:
 			return buildAnnotation((Annotation) node, false);
+		case TYPE_USE:
+			return buildTypeUse((TypeReference) node);
 		default:
 			throw new AssertionError("Did not expect to arrive here: " + kind);
 		}
@@ -397,6 +423,7 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 		if (field instanceof Initializer) return buildInitializer((Initializer)field);
 		if (setAndGetAsHandled(field)) return null;
 		List<EclipseNode> childNodes = new ArrayList<EclipseNode>();
+		addIfNotNull(childNodes, buildTypeUse(field.type));
 		addIfNotNull(childNodes, buildStatement(field.initialization));
 		childNodes.addAll(buildAnnotations(field.annotations, true));
 		return putInMap(new EclipseNode(this, field, childNodes, Kind.FIELD));
@@ -438,11 +465,40 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 	private EclipseNode buildLocal(LocalDeclaration local, Kind kind) {
 		if (setAndGetAsHandled(local)) return null;
 		List<EclipseNode> childNodes = new ArrayList<EclipseNode>();
+		addIfNotNull(childNodes, buildTypeUse(local.type));
 		addIfNotNull(childNodes, buildStatement(local.initialization));
 		childNodes.addAll(buildAnnotations(local.annotations, true));
 		return putInMap(new EclipseNode(this, local, childNodes, kind));
 	}
 	
+	private EclipseNode buildTypeUse(TypeReference tr) {
+		if (setAndGetAsHandled(tr)) return null;
+		if (tr == null) return null;
+		
+		List<EclipseNode> childNodes = new ArrayList<EclipseNode>();
+		Annotation[] anns = getTopLevelTypeReferenceAnnotations(tr);
+		if (anns != null) for (Annotation ann : anns) addIfNotNull(childNodes, buildAnnotation(ann, false));
+		
+		if (tr instanceof ParameterizedQualifiedTypeReference) {
+			ParameterizedQualifiedTypeReference pqtr = (ParameterizedQualifiedTypeReference) tr;
+			int len = pqtr.tokens.length;
+			for (int i = 0; i < len; i++) {
+				TypeReference[] typeArgs = pqtr.typeArguments[i];
+				if (typeArgs != null) for (TypeReference tArg : typeArgs) addIfNotNull(childNodes, buildTypeUse(tArg));
+			}
+		} else if (tr instanceof ParameterizedSingleTypeReference) {
+			ParameterizedSingleTypeReference pstr = (ParameterizedSingleTypeReference) tr;
+			if (pstr.typeArguments != null) for (TypeReference tArg : pstr.typeArguments) {
+				addIfNotNull(childNodes, buildTypeUse(tArg));
+			}
+		} else if (tr instanceof Wildcard) {
+			TypeReference bound = ((Wildcard) tr).bound;
+			if (bound != null) addIfNotNull(childNodes, buildTypeUse(bound));
+		}
+		
+		return putInMap(new EclipseNode(this, tr, childNodes, Kind.TYPE_USE));
+	}
+	
 	private Collection<EclipseNode> buildAnnotations(Annotation[] annotations, boolean varDecl) {
 		List<EclipseNode> elements = new ArrayList<EclipseNode>();
 		if (annotations != null) for (Annotation an : annotations) addIfNotNull(elements, buildAnnotation(an, varDecl));
@@ -467,9 +523,9 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 	
 	private EclipseNode buildStatement(Statement child) {
 		if (child == null) return null;
-		if (child instanceof TypeDeclaration) return buildType((TypeDeclaration)child);
+		if (child instanceof TypeDeclaration) return buildType((TypeDeclaration) child);
 		
-		if (child instanceof LocalDeclaration) return buildLocal((LocalDeclaration)child, Kind.LOCAL);
+		if (child instanceof LocalDeclaration) return buildLocal((LocalDeclaration) child, Kind.LOCAL);
 		
 		if (setAndGetAsHandled(child)) return null;
 		
@@ -491,21 +547,35 @@ public class EclipseAST extends AST<EclipseAST, EclipseNode, ASTNode> {
 	private static class EcjReflectionCheck {
 		private static final String COMPILATIONRESULT_TYPE = "org.eclipse.jdt.internal.compiler.CompilationResult";
 		
-		public static Method addProblemToCompilationResult;
-		public static final Throwable problem;
-		
+		public static final Method addProblemToCompilationResult;
+		public static final Throwable problemAddProblemToCompilationResult;
+		public static final Method typeReferenceGetAnnotationsOnDimensions;
+		public static final Field typeReferenceAnnotations;
 		static {
 			Throwable problem_ = null;
-			Method m = null;
+			Method m1 = null, m2;
+			Field f;
 			try {
-				m = Permit.getMethod(EclipseAstProblemView.class, "addProblemToCompilationResult", char[].class, Class.forName(COMPILATIONRESULT_TYPE), boolean.class, String.class, int.class, int.class);
+				m1 = Permit.getMethod(EclipseAstProblemView.class, "addProblemToCompilationResult", char[].class, Class.forName(COMPILATIONRESULT_TYPE), boolean.class, String.class, int.class, int.class);
 			} catch (Throwable t) {
 				// That's problematic, but as long as no local classes are used we don't actually need it.
 				// Better fail on local classes than crash altogether.
 				problem_ = t;
 			}
-			addProblemToCompilationResult = m;
-			problem = problem_;
+			try {
+				m2 = Permit.getMethod(TypeReference.class, "getAnnotationsOnDimensions");
+			} catch (Throwable t) {
+				m2 = null;
+			}
+			try {
+				f = Permit.getField(TypeReference.class, "annotations");
+			} catch (Throwable t) {
+				f = null;
+			}
+			addProblemToCompilationResult = m1;
+			problemAddProblemToCompilationResult = problem_;
+			typeReferenceGetAnnotationsOnDimensions = m2;
+			typeReferenceAnnotations = f;
 		}
 	}
 }
diff --git a/src/core/lombok/eclipse/EclipseASTAdapter.java b/src/core/lombok/eclipse/EclipseASTAdapter.java
index 61807fff..c6ad059d 100644
--- a/src/core/lombok/eclipse/EclipseASTAdapter.java
+++ b/src/core/lombok/eclipse/EclipseASTAdapter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -30,6 +30,7 @@ import org.eclipse.jdt.internal.compiler.ast.Initializer;
 import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 
 /**
  * Standard adapter for the {@link EclipseASTVisitor} interface. Every method on that interface
@@ -97,6 +98,15 @@ public abstract class EclipseASTAdapter implements EclipseASTVisitor {
 	public void endVisitLocal(EclipseNode localNode, LocalDeclaration local) {}
 	
 	/** {@inheritDoc} */
+	@Override public void visitTypeUse(EclipseNode typeUseNode, TypeReference typeUse) {}
+	
+	/** {@inheritDoc} */
+	public void visitAnnotationOnTypeUse(TypeReference typeUse, EclipseNode annotationNode, Annotation annotation) {}
+	
+	/** {@inheritDoc} */
+	@Override public void endVisitTypeUse(EclipseNode typeUseNode, TypeReference typeUse) {}
+	
+	/** {@inheritDoc} */
 	public void visitStatement(EclipseNode statementNode, Statement statement) {}
 	
 	/** {@inheritDoc} */
diff --git a/src/core/lombok/eclipse/EclipseASTVisitor.java b/src/core/lombok/eclipse/EclipseASTVisitor.java
index 37bda5e3..0bd668bc 100644
--- a/src/core/lombok/eclipse/EclipseASTVisitor.java
+++ b/src/core/lombok/eclipse/EclipseASTVisitor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2012 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -107,6 +107,13 @@ public interface EclipseASTVisitor {
 	void endVisitLocal(EclipseNode localNode, LocalDeclaration local);
 	
 	/**
+	 * Visits a node that represents a type reference. Anything from {@code int} to {@code T} to {@code foo,.pkg.Bar<T>.Baz<?> @Ann []}.
+	 */
+	void visitTypeUse(EclipseNode typeUseNode, TypeReference typeUse);
+	void visitAnnotationOnTypeUse(TypeReference typeUse, EclipseNode annotationNode, Annotation annotation);
+	void endVisitTypeUse(EclipseNode typeUseNode, TypeReference typeUse);
+	
+	/**
 	 * Visits a statement that isn't any of the other visit methods (e.g. TypeDeclaration).
 	 */
 	void visitStatement(EclipseNode statementNode, Statement statement);
@@ -412,6 +419,21 @@ public interface EclipseASTVisitor {
 			print("</LOCAL %s %s>", str(local.type), str(local.name));
 		}
 		
+		@Override public void visitTypeUse(EclipseNode typeUseNode, TypeReference typeUse) {
+			print("<TYPE %s>", typeUse.getClass());
+			indent++;
+			print("%s", typeUse);
+		}
+		
+		@Override public void visitAnnotationOnTypeUse(TypeReference typeUse, EclipseNode annotationNode, Annotation annotation) {
+			print("<ANNOTATION%s: %s />", isGenerated(annotation) ? " (GENERATED)" : "", annotation);
+		}
+		
+		@Override public void endVisitTypeUse(EclipseNode typeUseNode, TypeReference typeUse) {
+			indent--;
+			print("</TYPE %s>", typeUse.getClass());
+		}
+		
 		public void visitStatement(EclipseNode node, Statement statement) {
 			print("<%s%s%s>", statement.getClass(), isGenerated(statement) ? " (GENERATED)" : "", position(node));
 			if (statement instanceof AllocationExpression) {
diff --git a/src/core/lombok/eclipse/EclipseNode.java b/src/core/lombok/eclipse/EclipseNode.java
index a0580c51..4c7f4eac 100644
--- a/src/core/lombok/eclipse/EclipseNode.java
+++ b/src/core/lombok/eclipse/EclipseNode.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2018 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -38,6 +38,7 @@ import org.eclipse.jdt.internal.compiler.ast.Initializer;
 import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 
 /**
@@ -119,10 +120,18 @@ public class EclipseNode extends lombok.core.LombokNode<EclipseAST, EclipseNode,
 			case LOCAL:
 				visitor.visitAnnotationOnLocal((LocalDeclaration) parent.get(), this, (Annotation) get());
 				break;
+			case TYPE_USE:
+				visitor.visitAnnotationOnTypeUse((TypeReference) parent.get(), this, (Annotation) get());
+				break;
 			default:
 				throw new AssertionError("Annotation not expected as child of a " + up().getKind());
 			}
 			break;
+		case TYPE_USE:
+			visitor.visitTypeUse(this, (TypeReference) get());
+			ast.traverseChildren(visitor, this);
+			visitor.endVisitTypeUse(this, (TypeReference) get());
+			break;
 		case STATEMENT:
 			visitor.visitStatement(this, (Statement) get());
 			ast.traverseChildren(visitor, this);
diff --git a/src/core/lombok/eclipse/TransformEclipseAST.java b/src/core/lombok/eclipse/TransformEclipseAST.java
index e5edba64..6fcde937 100644
--- a/src/core/lombok/eclipse/TransformEclipseAST.java
+++ b/src/core/lombok/eclipse/TransformEclipseAST.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2018 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -39,6 +39,7 @@ import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.eclipse.jdt.internal.compiler.parser.Parser;
 
 /**
@@ -236,5 +237,10 @@ public class TransformEclipseAST {
 			CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
 			nextPriority = Math.min(nextPriority, handlers.handleAnnotation(top, annotationNode, annotation, priority));
 		}
+		
+		@Override public void visitAnnotationOnTypeUse(TypeReference typeUse, EclipseNode annotationNode, Annotation annotation) {
+			CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
+			nextPriority = Math.min(nextPriority, handlers.handleAnnotation(top, annotationNode, annotation, priority));
+		}
 	}
 }
diff --git a/src/core/lombok/eclipse/handlers/HandleNonNull.java b/src/core/lombok/eclipse/handlers/HandleNonNull.java
index ebc62909..1672618d 100644
--- a/src/core/lombok/eclipse/handlers/HandleNonNull.java
+++ b/src/core/lombok/eclipse/handlers/HandleNonNull.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2014 The Project Lombok Authors.
+ * Copyright (C) 2013-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -33,6 +33,7 @@ import lombok.core.AST.Kind;
 import lombok.core.AnnotationValues;
 import lombok.core.HandlerPriority;
 import lombok.eclipse.DeferUntilPostDiet;
+import lombok.eclipse.EclipseAST;
 import lombok.eclipse.EclipseAnnotationHandler;
 import lombok.eclipse.EclipseNode;
 
@@ -52,6 +53,7 @@ import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.SynchronizedStatement;
 import org.eclipse.jdt.internal.compiler.ast.ThrowStatement;
 import org.eclipse.jdt.internal.compiler.ast.TryStatement;
+import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.mangosdk.spi.ProviderFor;
 
 @DeferUntilPostDiet
@@ -95,14 +97,33 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 			return;
 		}
 		
-		if (annotationNode.up().getKind() != Kind.ARGUMENT) return;
-		
-		Argument arg;
+		Argument param;
+		EclipseNode paramNode;
 		AbstractMethodDeclaration declaration;
 		
+		switch (annotationNode.up().getKind()) {
+		case ARGUMENT:
+			paramNode = annotationNode.up();
+			break;
+		case TYPE_USE:
+			EclipseNode typeNode = annotationNode.directUp();
+			boolean ok = false;
+			ASTNode astNode = typeNode.get();
+			if (astNode instanceof TypeReference) {
+				Annotation[] anns = EclipseAST.getTopLevelTypeReferenceAnnotations((TypeReference) astNode);
+				if (anns == null) return;
+				for (Annotation ann : anns) if (ast == ann) ok = true;
+			}
+			if (!ok) return;
+			paramNode = typeNode.directUp();
+			break;
+		default:
+			return;
+		}
+		
 		try {
-			arg = (Argument) annotationNode.up().get();
-			declaration = (AbstractMethodDeclaration) annotationNode.up().up().get();
+			param = (Argument) paramNode.get();
+			declaration = (AbstractMethodDeclaration) paramNode.up().get();
 		} catch (Exception e) {
 			return;
 		}
@@ -118,7 +139,7 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 		// and if they exist, create a new method in the class: 'private static <T> T lombok$nullCheck(T expr, String msg) {if (expr == null) throw NPE; return expr;}' and
 		// wrap all references to it in the super/this to a call to this method.
 		
-		Statement nullCheck = generateNullCheck(arg, annotationNode);
+		Statement nullCheck = generateNullCheck(param, annotationNode);
 		
 		if (nullCheck == null) {
 			// @NonNull applied to a primitive. Kinda pointless. Let's generate a warning.
@@ -129,7 +150,7 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 		if (declaration.statements == null) {
 			declaration.statements = new Statement[] {nullCheck};
 		} else {
-			char[] expectedName = arg.name;
+			char[] expectedName = param.name;
 			/* Abort if the null check is already there, delving into try and synchronized statements */ {
 				Statement[] stats = declaration.statements;
 				int idx = 0;
@@ -162,7 +183,7 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 			newStatements[skipOver] = nullCheck;
 			declaration.statements = newStatements;
 		}
-		annotationNode.up().up().rebuild();
+		paramNode.up().rebuild();
 	}
 	
 	public boolean isNullCheck(Statement stat) {