From 58b38dcaa516435746a4513d76a32508e714c380 Mon Sep 17 00:00:00 2001 From: Pim van der Loos Date: Tue, 25 Jan 2022 14:24:18 +0100 Subject: [PATCH] Apply feedback - Applied the feedback received on the first version: + Using the handleExperimentalFlagUsage to reflect the fact this is an experimental feature. + Replace the 1-len boolean[] with 1) A regular boolean for javac (no need for mutability), and 2) An AtomicBoolean for ecj (more fitting for mutable boolean). + Replace the typeReferenceNameEquals method with a much cleaner Arrays.deepEquals call. --- .../eclipse/handlers/HandleLockedUtil.java | 36 +++++++------------ .../javac/handlers/HandleLockedUtil.java | 16 ++++----- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/core/lombok/eclipse/handlers/HandleLockedUtil.java b/src/core/lombok/eclipse/handlers/HandleLockedUtil.java index 15d06e57..ceb05709 100644 --- a/src/core/lombok/eclipse/handlers/HandleLockedUtil.java +++ b/src/core/lombok/eclipse/handlers/HandleLockedUtil.java @@ -39,12 +39,12 @@ import org.eclipse.jdt.internal.compiler.ast.Statement; import org.eclipse.jdt.internal.compiler.ast.ThisReference; 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 java.lang.reflect.Modifier; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; -import static lombok.core.handlers.HandlerUtil.handleFlagUsage; +import static lombok.core.handlers.HandlerUtil.handleExperimentalFlagUsage; import static lombok.eclipse.handlers.EclipseHandlerUtil.upToTypeNode; import static lombok.eclipse.handlers.EclipseHandlerUtil.*; @@ -61,31 +61,21 @@ public final class HandleLockedUtil { //Prevent instantiation } - private static boolean typeReferenceNameEquals(TypeReference a, TypeReference b) { - char[][] typeNameA = a.getTypeName(); - char[][] typeNameB = b.getTypeName(); - if (typeNameA.length != typeNameB.length) return false; - for (int idx = 0; idx < typeNameA.length; ++idx) { - if (!Arrays.equals(typeNameA[idx], typeNameB[idx])) return false; - } - return true; - } - public static void preHandle(String annotationValue, char[][] lockClass, EclipseNode annotationNode) { EclipseNode methodNode = annotationNode.up(); if (methodNode == null || methodNode.getKind() != AST.Kind.METHOD || !(methodNode.get() instanceof MethodDeclaration)) return; MethodDeclaration method = (MethodDeclaration) methodNode.get(); if (method.isAbstract()) return; - createLockField(annotationValue, annotationNode, lockClass, new boolean[] { method.isStatic() }, false); + createLockField(annotationValue, annotationNode, lockClass, new AtomicBoolean(method.isStatic()), false); } - private static char[] createLockField(String name, EclipseNode annotationNode, char[][] lockClass, boolean[] isStatic, boolean reportErrors) { + private static char[] createLockField(String name, EclipseNode annotationNode, char[][] lockClass, AtomicBoolean isStatic, boolean reportErrors) { char[] lockName = name.toCharArray(); Annotation source = (Annotation) annotationNode.get(); if (lockName.length == 0) { - lockName = isStatic[0] ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME; + lockName = isStatic.get() ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME; } EclipseNode typeNode = upToTypeNode(annotationNode); @@ -101,13 +91,13 @@ public final class HandleLockedUtil { if (Arrays.equals(fName, lockName)) { exists = getGeneratedBy(def) == null ? MemberExistsResult.EXISTS_BY_USER : MemberExistsResult.EXISTS_BY_LOMBOK; boolean st = def.isStatic(); - if (st != isStatic[0] && exists == MemberExistsResult.EXISTS_BY_LOMBOK) { + if (st != isStatic.get() && exists == MemberExistsResult.EXISTS_BY_LOMBOK) { if (reportErrors) annotationNode.addError(String.format("The generated field %s does not match the static status of this method", new String(lockName))); return null; } - isStatic[0] = st; + isStatic.set(st); - if (exists == MemberExistsResult.EXISTS_BY_LOMBOK && !typeReferenceNameEquals(lockType, def.type)) { + if (exists == MemberExistsResult.EXISTS_BY_LOMBOK && !Arrays.deepEquals(lockType.getTypeName(), def.type.getTypeName())) { annotationNode.addError("Expected field " + new String(lockName) + " to be of type " + lockType + " but got type " + def.type + "! Did you mix @Locked with @Locked.Read/Write on the same generated field?"); return null; @@ -121,7 +111,7 @@ public final class HandleLockedUtil { FieldDeclaration fieldDecl = setGeneratedBy(new FieldDeclaration(lockName, 0, -1), source); fieldDecl.declarationSourceEnd = -1; - fieldDecl.modifiers = (isStatic[0] ? Modifier.STATIC : 0) | Modifier.FINAL | Modifier.PRIVATE; + fieldDecl.modifiers = (isStatic.get() ? Modifier.STATIC : 0) | Modifier.FINAL | Modifier.PRIVATE; AllocationExpression lockAlloc = setGeneratedBy(new AllocationExpression(), source); lockAlloc.type = setGeneratedBy(new QualifiedTypeReference(lockClass, new long[] { 0, 0, 0, 0, 0 }), source); @@ -161,7 +151,7 @@ public final class HandleLockedUtil { */ public static void handle(String annotationValue, Annotation source, EclipseNode annotationNode, String annotationName, char[][] lockClass, char[] lockableMethodName) { - handleFlagUsage(annotationNode, ConfigurationKeys.LOCKED_FLAG_USAGE, annotationName); + handleExperimentalFlagUsage(annotationNode, ConfigurationKeys.LOCKED_FLAG_USAGE, annotationName); int p1 = source.sourceStart -1; int p2 = source.sourceStart -2; @@ -185,7 +175,7 @@ public final class HandleLockedUtil { return; } - boolean[] isStatic = { method.isStatic() }; + AtomicBoolean isStatic = new AtomicBoolean(method.isStatic()); char[] lockName = createLockField(annotationValue, annotationNode, lockClass, isStatic, true); if (lockName == null) return; if (method.statements == null) return; @@ -198,8 +188,8 @@ public final class HandleLockedUtil { block.sourceEnd = method.bodyEnd; block.sourceStart = method.bodyStart; - Statement acquireLock = getLockingStatement(source, typeNode, LOCK_METHOD, lockName, lockableMethodName, isStatic[0], p1, p2, pos); - Statement unLock = getLockingStatement(source, typeNode, UNLOCK_METHOD, lockName, lockableMethodName, isStatic[0], p1, p2, pos); + Statement acquireLock = getLockingStatement(source, typeNode, LOCK_METHOD, lockName, lockableMethodName, isStatic.get(), p1, p2, pos); + Statement unLock = getLockingStatement(source, typeNode, UNLOCK_METHOD, lockName, lockableMethodName, isStatic.get(), p1, p2, pos); TryStatement tryStatement = new TryStatement(); tryStatement.tryBlock = block; diff --git a/src/core/lombok/javac/handlers/HandleLockedUtil.java b/src/core/lombok/javac/handlers/HandleLockedUtil.java index 6675aa30..a001e984 100644 --- a/src/core/lombok/javac/handlers/HandleLockedUtil.java +++ b/src/core/lombok/javac/handlers/HandleLockedUtil.java @@ -33,7 +33,7 @@ import lombok.javac.JavacTreeMaker; import java.lang.annotation.Annotation; import static com.sun.tools.javac.tree.JCTree.*; -import static lombok.core.handlers.HandlerUtil.handleFlagUsage; +import static lombok.core.handlers.HandlerUtil.handleExperimentalFlagUsage; import static lombok.javac.handlers.JavacHandlerUtil.*; /** @@ -82,7 +82,7 @@ public final class HandleLockedUtil { */ public static void handle(String annotationValue, JCTree.JCAnnotation ast, JavacNode annotationNode, Class annotationClass, String annotationName, String[] lockClass, String lockableMethodName) { - handleFlagUsage(annotationNode, ConfigurationKeys.LOCKED_FLAG_USAGE, annotationName); + handleExperimentalFlagUsage(annotationNode, ConfigurationKeys.LOCKED_FLAG_USAGE, annotationName); if (inNetbeansEditor(annotationNode)) return; deleteAnnotationIfNeccessary(annotationNode, annotationClass); @@ -105,11 +105,11 @@ public final class HandleLockedUtil { return; } - boolean[] isStatic = new boolean[]{(method.mods.flags & Flags.STATIC) != 0}; + boolean isStatic = (method.mods.flags & Flags.STATIC) != 0; String lockName = annotationValue; if (lockName.length() == 0) { - lockName = isStatic[0] ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME; + lockName = isStatic ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME; } JavacTreeMaker maker = methodNode.getTreeMaker().at(ast.pos); @@ -126,11 +126,11 @@ public final class HandleLockedUtil { exists = getGeneratedBy(varDeclDef) == null ? MemberExistsResult.EXISTS_BY_USER : MemberExistsResult.EXISTS_BY_LOMBOK; boolean st = ((varDeclDef.mods.flags) & Flags.STATIC) != 0; - if (isStatic[0] != st && exists == MemberExistsResult.EXISTS_BY_LOMBOK) { + if (isStatic != st && exists == MemberExistsResult.EXISTS_BY_LOMBOK) { annotationNode.addError("The generated field " + lockName + " does not match the static status of this method"); return; } - isStatic[0] = st; + isStatic = st; if (exists == MemberExistsResult.EXISTS_BY_LOMBOK && !lockType.toString().equals(varDeclDef.vartype.toString())) { annotationNode.addError("Expected field " + lockName + " to be of type " + lockType + " but got type " + @@ -145,7 +145,7 @@ public final class HandleLockedUtil { if (exists == MemberExistsResult.NOT_EXISTS) { JCNewClass lockInstance = maker.NewClass(null, NIL_EXPRESSION, lockType, NIL_EXPRESSION, null); JCVariableDecl newLockField = recursiveSetGeneratedBy(maker.VarDef( - maker.Modifiers(Flags.PRIVATE | Flags.FINAL | (isStatic[0] ? Flags.STATIC : 0)), + maker.Modifiers(Flags.PRIVATE | Flags.FINAL | (isStatic ? Flags.STATIC : 0)), methodNode.toName(lockName), lockType, lockInstance ), annotationNode); @@ -155,7 +155,7 @@ public final class HandleLockedUtil { if (method.body == null || typeNode == null) return; JCExpression lockNode; - if (isStatic[0]) { + if (isStatic) { lockNode = namePlusTypeParamsToTypeReference(maker, typeNode, methodNode.toName(lockName), false, List.nil()); } else { lockNode = maker.Select(maker.Ident(methodNode.toName("this")), methodNode.toName(lockName));