From 52a31bc4ae2806907194d32567a820c670670357 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Wed, 24 Mar 2021 06:20:02 +0100 Subject: [records] [`@NonNull`] eclipse impl onfthe `@NonNull` on record components feature. All tests passing. --- .../eclipse/handlers/EclipseHandlerUtil.java | 20 --- .../lombok/eclipse/handlers/HandleNonNull.java | 185 ++++++++++++++++++++- src/core/lombok/javac/handlers/HandleNonNull.java | 2 + src/delombok/lombok/delombok/PrettyPrinter.java | 3 - .../lombok/eclipse/agent/PatchVal.java | 5 +- src/utils/lombok/eclipse/Eclipse.java | 10 +- 6 files changed, 186 insertions(+), 39 deletions(-) (limited to 'src') 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 @@ -1888,26 +1888,6 @@ public class EclipseHandlerUtil { return MemberExistsResult.NOT_EXISTS; } - /** - * 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 { 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 { } } + private List getRecordComponents(EclipseNode typeNode) { + List list = new ArrayList(); + + 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 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 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.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 { 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 { // 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 { 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 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 { 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 { 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 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("\\."); -- cgit