[records] [`@NonNull`] eclipse impl onfthe `@NonNull` on record components feature.

All tests passing.

6 files changed, 186 insertions, 39 deletions

diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
index defe8d34..70d98cc6 100644
--- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
+++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
@@ -1889,26 +1889,6 @@ public class EclipseHandlerUtil {
 	}
 	
 	/**
-	 * Checks if there is at least one constructor that is generated by lombok.
-	 * 
-	 * @param node Any node that represents the Type (TypeDeclaration) to look in, or any child node thereof.
-	 */
-	public static boolean lombokConstructorExists(EclipseNode node) {
-		node = upToTypeNode(node);
-		if (node != null && node.get() instanceof TypeDeclaration) {
-			TypeDeclaration typeDecl = (TypeDeclaration) node.get();
-			if (typeDecl.methods != null) for (AbstractMethodDeclaration def : typeDecl.methods) {
-				if (!(def instanceof ConstructorDeclaration)) continue;
-				if ((def.bits & ASTNode.IsDefaultConstructor | IsCanonicalConstructor) != 0) continue;
-				if (isTolerate(node, def)) continue;
-				if (getGeneratedBy(def) != null) return true;
-			}
-		}
-		
-		return false;
-	}
-	
-	/**
 	 * Inserts a field into an existing type. The type must represent a {@code TypeDeclaration}.
 	 * The field carries the @{@link SuppressWarnings}("all") annotation.
 	 */
diff --git a/src/core/lombok/eclipse/handlers/HandleNonNull.java b/src/core/lombok/eclipse/handlers/HandleNonNull.java
index 27e78d32..365eef33 100644
--- a/src/core/lombok/eclipse/handlers/HandleNonNull.java
+++ b/src/core/lombok/eclipse/handlers/HandleNonNull.java
@@ -22,11 +22,12 @@
 package lombok.eclipse.handlers;
 
 import static lombok.core.handlers.HandlerUtil.handleFlagUsage;
-import static lombok.eclipse.Eclipse.isPrimitive;
+import static lombok.eclipse.Eclipse.*;
 import static lombok.eclipse.handlers.EclipseHandlerUtil.*;
 
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
+import java.util.List;
 
 import org.eclipse.jdt.internal.compiler.ast.ASTNode;
 import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
@@ -36,19 +37,28 @@ import org.eclipse.jdt.internal.compiler.ast.Argument;
 import org.eclipse.jdt.internal.compiler.ast.AssertStatement;
 import org.eclipse.jdt.internal.compiler.ast.Assignment;
 import org.eclipse.jdt.internal.compiler.ast.Block;
+import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.EqualExpression;
+import org.eclipse.jdt.internal.compiler.ast.ExplicitConstructorCall;
 import org.eclipse.jdt.internal.compiler.ast.Expression;
+import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.FieldReference;
 import org.eclipse.jdt.internal.compiler.ast.IfStatement;
 import org.eclipse.jdt.internal.compiler.ast.MessageSend;
 import org.eclipse.jdt.internal.compiler.ast.NullLiteral;
+import org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.SingleNameReference;
+import org.eclipse.jdt.internal.compiler.ast.SingleTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.SynchronizedStatement;
+import org.eclipse.jdt.internal.compiler.ast.ThisReference;
 import org.eclipse.jdt.internal.compiler.ast.ThrowStatement;
 import org.eclipse.jdt.internal.compiler.ast.TryStatement;
+import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.TypeReference;
+import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 
-import lombok.AccessLevel;
 import lombok.ConfigurationKeys;
 import lombok.NonNull;
 import lombok.core.AST.Kind;
@@ -58,7 +68,6 @@ import lombok.eclipse.EcjAugments;
 import lombok.eclipse.EclipseAST;
 import lombok.eclipse.EclipseAnnotationHandler;
 import lombok.eclipse.EclipseNode;
-import lombok.eclipse.handlers.HandleConstructor.SkipIfConstructorExists;
 import lombok.spi.Provides;
 
 @Provides
@@ -68,7 +77,6 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 	private static final char[] CHECK_NOT_NULL = "checkNotNull".toCharArray();
 	
 	public static final HandleNonNull INSTANCE = new HandleNonNull();
-	private HandleConstructor handleConstructor = new HandleConstructor();
 	
 	public void fix(EclipseNode method) {
 		for (EclipseNode m : method.down()) {
@@ -83,14 +91,146 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 		}
 	}
 	
+	private List<FieldDeclaration> getRecordComponents(EclipseNode typeNode) {
+		List<FieldDeclaration> list = new ArrayList<FieldDeclaration>();
+		
+		for (EclipseNode child : typeNode.down()) {
+			if (child.getKind() == Kind.FIELD) {
+				FieldDeclaration fd = (FieldDeclaration) child.get();
+				if ((fd.modifiers & AccRecord) != 0) list.add(fd);
+			}
+		}
+		
+		return list;
+	}
+	
+	private EclipseNode addCompactConstructorIfNeeded(EclipseNode typeNode, EclipseNode annotationNode) {
+		// explicit Compact Constructor has bits set: Bit32, IsCanonicalConstructor (10).
+		// implicit Compact Constructor has bits set: Bit32, IsCanonicalConstructor (10), and IsImplicit (11).
+		// explicit constructor with long-form shows up as a normal constructor (Bit32 set, that's all), but the
+		//   implicit CC is then also present and will presumably be stripped out in some later phase.
+		
+		EclipseNode toRemove = null;
+		EclipseNode existingCompactConstructor = null;
+		List<FieldDeclaration> recordComponents = null;
+		for (EclipseNode child : typeNode.down()) {
+			if (!(child.get() instanceof ConstructorDeclaration)) continue;
+			ConstructorDeclaration cd = (ConstructorDeclaration) child.get();
+			if ((cd.bits & IsCanonicalConstructor) != 0) {
+				if ((cd.bits & IsImplicit) != 0) {
+					toRemove = child;
+				} else {
+					existingCompactConstructor = child;
+				}
+			} else {
+				// If this constructor has exact matching types vs. the record components,
+				// this is the canonical constructor in long form and we should not generate one.
+				
+				if (recordComponents == null) recordComponents = getRecordComponents(typeNode);
+				int argLength = cd.arguments == null ? 0 : cd.arguments.length;
+				int compLength = recordComponents.size();
+				boolean isCanonical = argLength == compLength;
+				if (isCanonical) top: for (int i = 0; i < argLength; i++) {
+					TypeReference a = recordComponents.get(i).type;
+					TypeReference b = cd.arguments[i] == null ? null : cd.arguments[i].type;
+					// technically this won't match e.g. `java.lang.String` to just `String`;
+					// to use this feature you'll need to use the same way to write it, which seems
+					// like a fair requirement.
+					char[][] ta = getRawTypeName(a);
+					char[][] tb = getRawTypeName(b);
+					if (ta == null || tb == null || ta.length != tb.length) {
+						isCanonical = false;
+						break top;
+					}
+					for (int j = 0; j < ta.length; j++) {
+						if (!Arrays.equals(ta[j], tb[j])) {
+							isCanonical = false;
+							break top;
+						}
+					}
+				}
+				if (isCanonical) {
+					return null;
+				}
+			}
+		}
+		if (existingCompactConstructor != null) return existingCompactConstructor;
+		int posToInsert = -1;
+		TypeDeclaration td = (TypeDeclaration) typeNode.get();
+		if (toRemove != null) {
+			int idxToRemove = -1;
+			for (int i = 0; i < td.methods.length; i++) {
+				if (td.methods[i] == toRemove.get()) idxToRemove = i;
+			}
+			if (idxToRemove != -1) {
+				System.arraycopy(td.methods, idxToRemove + 1, td.methods, idxToRemove, td.methods.length - idxToRemove - 1);
+				posToInsert = td.methods.length - 1;
+				typeNode.removeChild(toRemove);
+			}
+		}
+		if (posToInsert == -1) {
+			AbstractMethodDeclaration[] na = new AbstractMethodDeclaration[td.methods.length + 1];
+			posToInsert = td.methods.length;
+			System.arraycopy(td.methods, 0, na, 0, posToInsert);
+			td.methods = na;
+		}
+		
+		ConstructorDeclaration cd = new ConstructorDeclaration(((CompilationUnitDeclaration) typeNode.top().get()).compilationResult);
+		cd.modifiers = ClassFileConstants.AccPublic;
+		cd.bits = ASTNode.Bit32 | ECLIPSE_DO_NOT_TOUCH_FLAG | IsCanonicalConstructor;
+		cd.selector = td.name;
+		cd.constructorCall = new ExplicitConstructorCall(ExplicitConstructorCall.ImplicitSuper);
+		if (recordComponents == null) recordComponents = getRecordComponents(typeNode);
+		cd.arguments = new Argument[recordComponents.size()];
+		cd.statements = new Statement[recordComponents.size()];
+		cd.bits = IsCanonicalConstructor;
+		
+		for (int i = 0; i < cd.arguments.length; i++) {
+			FieldDeclaration cmp = recordComponents.get(i);
+			cd.arguments[i] = new Argument(cmp.name, cmp.sourceStart, cmp.type, 0);
+			cd.arguments[i].bits = ASTNode.IsArgument | ASTNode.IgnoreRawTypeCheck | ASTNode.IsReachable;
+			FieldReference lhs = new FieldReference(cmp.name, 0);
+			lhs.receiver = new ThisReference(0, 0);
+			SingleNameReference rhs = new SingleNameReference(cmp.name, 0);
+			cd.statements[i] = new Assignment(lhs, rhs, cmp.sourceEnd);
+		}
+		
+		setGeneratedBy(cd, annotationNode.get());
+		for (int i = 0; i < cd.arguments.length; i++) {
+			FieldDeclaration cmp = recordComponents.get(i);
+			cd.arguments[i].sourceStart = cmp.sourceStart;
+			cd.arguments[i].sourceEnd = cmp.sourceStart;
+			cd.arguments[i].declarationSourceEnd = cmp.sourceStart;
+			cd.arguments[i].declarationEnd = cmp.sourceStart;
+		}
+		
+		td.methods[posToInsert] = cd;
+		cd.annotations = addSuppressWarningsAll(typeNode, cd, cd.annotations);
+		cd.annotations = addGenerated(typeNode, cd, cd.annotations);
+		return typeNode.add(cd, Kind.METHOD);
+	}
+	
+	private static char[][] getRawTypeName(TypeReference a) {
+		if (a instanceof QualifiedTypeReference) return ((QualifiedTypeReference) a).tokens;
+		if (a instanceof SingleTypeReference) return new char[][] {((SingleTypeReference) a).token};
+		return null;
+	}
+	
 	@Override public void handle(AnnotationValues<NonNull> annotation, Annotation ast, EclipseNode annotationNode) {
 		// Generating new methods is only possible during diet parse but modifying existing methods requires a full parse.
 		// As we need both for @NonNull we reset the handled flag during diet parse.
+		
 		if (!annotationNode.isCompleteParse()) {
-			EclipseNode typeNode = upToTypeNode(annotationNode);
-			if (isRecordField(annotationNode.up()) && !lombokConstructorExists(typeNode)) {
-				handleConstructor.generateAllArgsConstructor(typeNode, AccessLevel.PUBLIC, null, SkipIfConstructorExists.NO, Collections.<Annotation>emptyList(), annotationNode);
+			if (annotationNode.up().getKind() == Kind.FIELD) {
+				//Check if this is a record and we need to generate the compact form constructor.
+				EclipseNode typeNode = annotationNode.up().up();
+				if (typeNode.getKind() == Kind.TYPE) {
+					if (isRecord(typeNode)) {
+						addCompactConstructorIfNeeded(typeNode, annotationNode);
+					}
+				}
 			}
+			
 			EcjAugments.ASTNode_handled.clear(ast);
 			return;
 		}
@@ -98,6 +238,16 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 		handle0(ast, annotationNode, false);
 	}
 	
+	private EclipseNode findCompactConstructor(EclipseNode typeNode) {
+		for (EclipseNode child : typeNode.down()) {
+			if (!(child.get() instanceof ConstructorDeclaration)) continue;
+			ConstructorDeclaration cd = (ConstructorDeclaration) child.get();
+			if ((cd.bits & IsCanonicalConstructor) != 0 && (cd.bits & IsImplicit) == 0) return child;
+		}
+		
+		return null;
+	}
+	
 	private void handle0(Annotation ast, EclipseNode annotationNode, boolean force) {
 		handleFlagUsage(annotationNode, ConfigurationKeys.NON_NULL_FLAG_USAGE, "@NonNull");
 		
@@ -106,13 +256,26 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 			// but in that case those handlers will take care of it. However, we DO check if the annotation is applied to
 			// a primitive, because those handlers trigger on any annotation named @NonNull and we only want the warning
 			// behaviour on _OUR_ 'lombok.NonNull'.
+			EclipseNode fieldNode = annotationNode.up();
+			EclipseNode typeNode = fieldNode.up();
 			
 			try {
 				if (isPrimitive(((AbstractVariableDeclaration) annotationNode.up().get()).type)) {
 					annotationNode.addWarning("@NonNull is meaningless on a primitive.");
+					return;
 				}
 			} catch (Exception ignore) {}
 			
+			if (isRecord(typeNode)) {
+				// well, these kinda double as parameters (of the compact constructor), so we do some work here.
+				// NB:Tthe diet parse run already added an explicit compact constructor if we need to take any actions.
+				EclipseNode compactConstructor = findCompactConstructor(typeNode);
+				
+				if (compactConstructor != null) {
+					addNullCheckIfNeeded((AbstractMethodDeclaration) compactConstructor.get(), (AbstractVariableDeclaration) fieldNode.get(), annotationNode);
+				}
+			}
+			
 			return;
 		}
 		
@@ -154,6 +317,11 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 			return;
 		}
 		
+		addNullCheckIfNeeded(declaration, param, annotationNode);
+		paramNode.up().rebuild();
+	}
+	
+	private void addNullCheckIfNeeded(AbstractMethodDeclaration declaration, AbstractVariableDeclaration param, EclipseNode annotationNode) {
 		// Possibly, if 'declaration instanceof ConstructorDeclaration', fetch declaration.constructorCall, search it for any references to our parameter,
 		// 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.
@@ -202,7 +370,6 @@ public class HandleNonNull extends EclipseAnnotationHandler<NonNull> {
 			newStatements[skipOver] = nullCheck;
 			declaration.statements = newStatements;
 		}
-		paramNode.up().rebuild();
 	}
 	
 	public boolean isNullCheck(Statement stat) {
diff --git a/src/core/lombok/javac/handlers/HandleNonNull.java b/src/core/lombok/javac/handlers/HandleNonNull.java
index 8e5b0030..786a7659 100644
--- a/src/core/lombok/javac/handlers/HandleNonNull.java
+++ b/src/core/lombok/javac/handlers/HandleNonNull.java
@@ -219,6 +219,8 @@ public class HandleNonNull extends JavacAnnotationHandler<NonNull> {
 			
 			JCVariableDecl fDecl = (JCVariableDecl) annotationNode.up().get();
 			if ((fDecl.mods.flags & RECORD) != 0) {
+				// well, these kinda double as parameters (of the compact constructor), so we do some work here.
+				
 				List<JCMethodDecl> compactConstructors = addCompactConstructorIfNeeded(annotationNode.up().up(), annotationNode);
 				for (JCMethodDecl ctr : compactConstructors) {
 					addNullCheckIfNeeded(ctr, annotationNode.up(), annotationNode);
diff --git a/src/delombok/lombok/delombok/PrettyPrinter.java b/src/delombok/lombok/delombok/PrettyPrinter.java
index 8ce38c33..c46a3298 100644
--- a/src/delombok/lombok/delombok/PrettyPrinter.java
+++ b/src/delombok/lombok/delombok/PrettyPrinter.java
@@ -815,9 +815,6 @@ public class PrettyPrinter extends JCTree.Visitor {
 		boolean argsLessConstructor = false;
 		if (isConstructor && (tree.mods.flags & COMPACT_RECORD_CONSTRUCTOR) != 0) {
 			argsLessConstructor = true;
-			for (JCVariableDecl param : tree.params) {
-				if ((param.mods.flags & GENERATED_MEMBER) == 0) argsLessConstructor = false;
-			}
 		}
 		
 		boolean first = true;
diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchVal.java b/src/eclipseAgent/lombok/eclipse/agent/PatchVal.java
index 9663f364..774e5b40 100644
--- a/src/eclipseAgent/lombok/eclipse/agent/PatchVal.java
+++ b/src/eclipseAgent/lombok/eclipse/agent/PatchVal.java
@@ -120,7 +120,7 @@ public class PatchVal {
 	public static boolean couldBe(ImportReference[] imports, String key, TypeReference ref) {
 		String[] keyParts = key.split("\\.");
 		if (ref instanceof SingleTypeReference) {
-			char[] token = ((SingleTypeReference)ref).token;
+			char[] token = ((SingleTypeReference) ref).token;
 			if (!matches(keyParts[keyParts.length - 1], token)) return false;
 			if (imports == null) return true;
 			top:
@@ -140,7 +140,7 @@ public class PatchVal {
 		}
 		
 		if (ref instanceof QualifiedTypeReference) {
-			char[][] tokens = ((QualifiedTypeReference)ref).tokens;
+			char[][] tokens = ((QualifiedTypeReference) ref).tokens;
 			if (keyParts.length != tokens.length) return false;
 			for(int i = 0; i < tokens.length; ++i) {
 				String part = keyParts[i];
@@ -270,7 +270,6 @@ public class PatchVal {
 		if (val) local.modifiers |= ClassFileConstants.AccFinal;
 		local.annotations = addValAnnotation(local.annotations, local.type, scope);
 		local.type = replacement != null ? replacement : new QualifiedTypeReference(TypeConstants.JAVA_LANG_OBJECT, poss(local.type, 3));
-		
 		return false;
 	}
 	
diff --git a/src/utils/lombok/eclipse/Eclipse.java b/src/utils/lombok/eclipse/Eclipse.java
index ac15f90b..8af481b9 100644
--- a/src/utils/lombok/eclipse/Eclipse.java
+++ b/src/utils/lombok/eclipse/Eclipse.java
@@ -60,10 +60,12 @@ public class Eclipse {
 	 */
 	public static final int ECLIPSE_DO_NOT_TOUCH_FLAG = ASTNode.Bit24;
 	
-	/* This section includes flags that would ordinarily be in ClassFileConstants, but which are 'too new' (we don't compile against older versions of ecj/eclipse for compatibility). */
-	public static final int AccRecord = ASTNode.Bit25;
-	public static final int IsCanonicalConstructor = ASTNode.Bit10; // For record declarations, and presumably later on any constructor matching the destructor.
-	public static final int IsImplicit = ASTNode.Bit11; // the generated statements in the compact constructor of a record.
+	/* This section includes flags that are in ecj files too new vs. the deps we compile against.
+	 * Specifically: org.eclipse.jdt.internal.compiler.ast.ASTNode and org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers
+	 */
+	public static final int AccRecord = ASTNode.Bit25; // ECM.AccRecord
+	public static final int IsCanonicalConstructor = ASTNode.Bit10; // ASTNode.IsCanonicalConstructor
+	public static final int IsImplicit = ASTNode.Bit11; // ASTNode.IsImplicit
 	
 	private static final Pattern SPLIT_AT_DOT = Pattern.compile("\\.");