tiennm99/lombok
1
0
Fork 0
mirror of https://github.com/tiennm99/lombok.git synced 2026-09-05 08:17:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

[fixes #678] @Synchronize an instance method on static variable no longer emits a warning.

Browse Source
This commit is contained in:
Reinier Zwitserloot
2020-02-14 01:21:24 +01:00
parent 15b09ee274
commit 89f98da78d
16 changed files with 156 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/changelog.markdown
+1
View File
@@ -4,6 +4,7 @@ Lombok Changelog
### v.18.13 "Edgy Guinea Pig"
* BREAKING CHANGE: mapstruct users should not add a dependency to lombok-mapstruct-binding. This solves compiling modules with lombok (and mapstruct).
* FEATURE: Similar to `@Builder`, you can now configure a `@SuperBuilder`'s 'setter' prefixes via `@SuperBuilder(setterPrefix = "set")` for example. We still discourage doing this. [Pull Request #2357](https://github.com/rzwitserloot/lombok/pull/2357).
* FEATURE: If using `@Synchronized("lockVar")`, if `lockVar` is referring to a static field, the code lombok generates no longer causes a warning about accessing a static entity incorrectly. [Issue #678](https://github.com/rzwitserloot/lombok/issues/678)
* BUGFIX: Using `@SuperBuilder` on a class that has some fairly convoluted generics usage would fail with 'Wrong number of type arguments'. [Issue #2359](https://github.com/rzwitserloot/lombok/issues/2359) [Pull Request #2362](https://github.com/rzwitserloot/lombok/pull/2362)
### v1.18.12 (February 1st, 2020)
src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
+5 -3
View File
@@ -992,7 +992,7 @@ public class EclipseHandlerUtil {
* Given for example {@code class Outer { class Inner {} }} this would generate {@code char[][] { "Outer", "Inner" }}.
* For method local and top level types, this generates a size-1 char[][] where the only char[] element is {@code name} itself.
*/
private static char[][] getQualifiedInnerName(EclipseNode parent, char[] name) {
public static char[][] getQualifiedInnerName(EclipseNode parent, char[] name) {
int count = 0;
EclipseNode n = parent;
@@ -1692,12 +1692,14 @@ public class EclipseHandlerUtil {
*/
public static MemberExistsResult fieldExists(String fieldName, EclipseNode node) {
node = upToTypeNode(node);
char[] fieldNameChars = null;
if (node != null && node.get() instanceof TypeDeclaration) {
TypeDeclaration typeDecl = (TypeDeclaration)node.get();
TypeDeclaration typeDecl = (TypeDeclaration) node.get();
if (typeDecl.fields != null) for (FieldDeclaration def : typeDecl.fields) {
char[] fName = def.name;
if (fName == null) continue;
if (fieldName.equals(new String(fName))) {
if (fieldNameChars == null) fieldNameChars = fieldName.toCharArray();
if (Arrays.equals(fName, fieldNameChars)) {
return getGeneratedBy(def) == null ? MemberExistsResult.EXISTS_BY_USER : MemberExistsResult.EXISTS_BY_LOMBOK;
}
}
src/core/lombok/eclipse/handlers/HandleSynchronized.java
+45 -16
View File
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2009-2014 The Project Lombok Authors.
* Copyright (C) 2009-2020 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
@@ -25,6 +25,7 @@ import static lombok.core.handlers.HandlerUtil.*;
import static lombok.eclipse.handlers.EclipseHandlerUtil.*;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import lombok.ConfigurationKeys;
import lombok.Synchronized;
@@ -34,6 +35,7 @@ import lombok.core.AST.Kind;
import lombok.eclipse.DeferUntilPostDiet;
import lombok.eclipse.EclipseAnnotationHandler;
import lombok.eclipse.EclipseNode;
import lombok.eclipse.handlers.EclipseHandlerUtil.MemberExistsResult;
import org.eclipse.jdt.internal.compiler.ast.Annotation;
import org.eclipse.jdt.internal.compiler.ast.ArrayAllocationExpression;
@@ -47,6 +49,7 @@ import org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference;
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.TypeDeclaration;
import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
import org.mangosdk.spi.ProviderFor;
@@ -63,31 +66,52 @@ public class HandleSynchronized extends EclipseAnnotationHandler<Synchronized> {
@Override public void preHandle(AnnotationValues<Synchronized> annotation, Annotation source, EclipseNode annotationNode) {
EclipseNode methodNode = annotationNode.up();
if (methodNode == null || methodNode.getKind() != Kind.METHOD || !(methodNode.get() instanceof MethodDeclaration)) return;
MethodDeclaration method = (MethodDeclaration)methodNode.get();
MethodDeclaration method = (MethodDeclaration) methodNode.get();
if (method.isAbstract()) return;
createLockField(annotation, annotationNode, method.isStatic(), false);
createLockField(annotation, annotationNode, new boolean[] {method.isStatic()}, false);
}
public char[] createLockField(AnnotationValues<Synchronized> annotation, EclipseNode annotationNode, boolean isStatic, boolean reportErrors) {
public char[] createLockField(AnnotationValues<Synchronized> annotation, EclipseNode annotationNode, boolean[] isStatic, boolean reportErrors) {
char[] lockName = annotation.getInstance().value().toCharArray();
Annotation source = (Annotation) annotationNode.get();
boolean autoMake = false;
if (lockName.length == 0) {
autoMake = true;
lockName = isStatic ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME;
lockName = isStatic[0] ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME;
}
if (fieldExists(new String(lockName), annotationNode) == MemberExistsResult.NOT_EXISTS) {
EclipseNode typeNode = upToTypeNode(annotationNode);
MemberExistsResult exists = MemberExistsResult.NOT_EXISTS;
if (typeNode != null && typeNode.get() instanceof TypeDeclaration) {
TypeDeclaration typeDecl = (TypeDeclaration) typeNode.get();
if (typeDecl.fields != null) for (FieldDeclaration def : typeDecl.fields) {
char[] fName = def.name;
if (fName == null) continue;
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]) {
if (reportErrors) annotationNode.addError(String.format("The field %s is non-static and thus cannot be used on this static method", new String(lockName)));
return null;
}
isStatic[0] = st;
break;
}
}
}
if (exists == MemberExistsResult.NOT_EXISTS) {
if (!autoMake) {
if (reportErrors) annotationNode.addError(String.format("The field %s does not exist.", new String(lockName)));
if (reportErrors) annotationNode.addError(String.format("The field %s does not exist", new String(lockName)));
return null;
}
FieldDeclaration fieldDecl = new FieldDeclaration(lockName, 0, -1);
setGeneratedBy(fieldDecl, source);
fieldDecl.declarationSourceEnd = -1;
fieldDecl.modifiers = (isStatic ? Modifier.STATIC : 0) | Modifier.FINAL | Modifier.PRIVATE;
fieldDecl.modifiers = (isStatic[0] ? Modifier.STATIC : 0) | Modifier.FINAL | Modifier.PRIVATE;
//We use 'new Object[0];' because unlike 'new Object();', empty arrays *ARE* serializable!
ArrayAllocationExpression arrayAlloc = new ArrayAllocationExpression();
@@ -111,20 +135,21 @@ public class HandleSynchronized extends EclipseAnnotationHandler<Synchronized> {
int p1 = source.sourceStart -1;
int p2 = source.sourceStart -2;
long pos = (((long)p1) << 32) | p2;
long pos = (((long) p1) << 32) | p2;
EclipseNode methodNode = annotationNode.up();
if (methodNode == null || methodNode.getKind() != Kind.METHOD || !(methodNode.get() instanceof MethodDeclaration)) {
annotationNode.addError("@Synchronized is legal only on methods.");
return;
}
MethodDeclaration method = (MethodDeclaration)methodNode.get();
MethodDeclaration method = (MethodDeclaration) methodNode.get();
if (method.isAbstract()) {
annotationNode.addError("@Synchronized is legal only on concrete methods.");
return;
}
char[] lockName = createLockField(annotation, annotationNode, method.isStatic(), true);
boolean[] isStatic = { method.isStatic() };
char[] lockName = createLockField(annotation, annotationNode, isStatic, true);
if (lockName == null) return;
if (method.statements == null) return;
@@ -137,18 +162,22 @@ public class HandleSynchronized extends EclipseAnnotationHandler<Synchronized> {
block.sourceStart = method.bodyStart;
Expression lockVariable;
if (method.isStatic()) lockVariable = new QualifiedNameReference(new char[][] {
methodNode.up().getName().toCharArray(), lockName }, new long[] { pos, pos }, p1, p2);
else {
if (isStatic[0]) {
EclipseNode typeNode = upToTypeNode(annotationNode);
char[][] n = getQualifiedInnerName(typeNode, lockName);
long[] ps = new long[n.length];
Arrays.fill(ps, pos);
lockVariable = new QualifiedNameReference(n, ps, p1, p2);
} else {
lockVariable = new FieldReference(lockName, pos);
ThisReference thisReference = new ThisReference(p1, p2);
setGeneratedBy(thisReference, source);
((FieldReference)lockVariable).receiver = thisReference;
((FieldReference) lockVariable).receiver = thisReference;
}
setGeneratedBy(lockVariable, source);
method.statements = new Statement[] {
new SynchronizedStatement(lockVariable, block, 0, 0)
new SynchronizedStatement(lockVariable, block, 0, 0)
};
// Positions for in-method generated nodes are special
src/core/lombok/javac/handlers/HandleSynchronized.java
+33 -11
View File
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2009-2014 The Project Lombok Authors.
* Copyright (C) 2009-2020 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
@@ -37,11 +37,14 @@ import lombok.javac.handlers.JavacHandlerUtil.MemberExistsResult;
import org.mangosdk.spi.ProviderFor;
import com.sun.tools.javac.code.Flags;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCAnnotation;
import com.sun.tools.javac.tree.JCTree.JCClassDecl;
import com.sun.tools.javac.tree.JCTree.JCExpression;
import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
import com.sun.tools.javac.tree.JCTree.JCNewArray;
import com.sun.tools.javac.tree.JCTree.JCStatement;
import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.List;
@@ -65,7 +68,6 @@ public class HandleSynchronized extends JavacAnnotationHandler<Synchronized> {
if (methodNode == null || methodNode.getKind() != Kind.METHOD || !(methodNode.get() instanceof JCMethodDecl)) {
annotationNode.addError("@Synchronized is legal only on methods.");
return;
}
@@ -73,21 +75,41 @@ public class HandleSynchronized extends JavacAnnotationHandler<Synchronized> {
if ((method.mods.flags & Flags.ABSTRACT) != 0) {
annotationNode.addError("@Synchronized is legal only on concrete methods.");
return;
}
boolean isStatic = (method.mods.flags & Flags.STATIC) != 0;
boolean[] isStatic = new boolean[] {(method.mods.flags & Flags.STATIC) != 0};
String lockName = annotation.getInstance().value();
boolean autoMake = false;
if (lockName.length() == 0) {
autoMake = true;
lockName = isStatic ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME;
lockName = isStatic[0] ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME;
}
JavacTreeMaker maker = methodNode.getTreeMaker().at(ast.pos);
Context context = methodNode.getContext();
if (fieldExists(lockName, methodNode) == MemberExistsResult.NOT_EXISTS) {
JavacNode typeNode = upToTypeNode(annotationNode);
MemberExistsResult exists = MemberExistsResult.NOT_EXISTS;
if (typeNode != null && typeNode.get() instanceof JCClassDecl) {
for (JCTree def : ((JCClassDecl) typeNode.get()).defs) {
if (def instanceof JCVariableDecl) {
if (((JCVariableDecl) def).name.contentEquals(lockName)) {
exists = getGeneratedBy(def) == null ? MemberExistsResult.EXISTS_BY_USER : MemberExistsResult.EXISTS_BY_LOMBOK;
boolean st = ((((JCVariableDecl) def).mods.flags) & Flags.STATIC) != 0;
if (isStatic[0] && !st) {
annotationNode.addError("The field " + lockName + " is non-static and this cannot be used on this static method");
return;
}
isStatic[0] = st;
}
}
}
}
if (exists == MemberExistsResult.NOT_EXISTS) {
if (!autoMake) {
annotationNode.addError("The field " + lockName + " does not exist.");
return;
@@ -95,18 +117,18 @@ public class HandleSynchronized extends JavacAnnotationHandler<Synchronized> {
JCExpression objectType = genJavaLangTypeRef(methodNode, ast.pos, "Object");
//We use 'new Object[0];' because unlike 'new Object();', empty arrays *ARE* serializable!
JCNewArray newObjectArray = maker.NewArray(genJavaLangTypeRef(methodNode, ast.pos, "Object"),
List.<JCExpression>of(maker.Literal(CTC_INT, 0)), null);
List.<JCExpression>of(maker.Literal(CTC_INT, 0)), null);
JCVariableDecl fieldDecl = recursiveSetGeneratedBy(maker.VarDef(
maker.Modifiers(Flags.PRIVATE | Flags.FINAL | (isStatic ? Flags.STATIC : 0)),
methodNode.toName(lockName), objectType, newObjectArray), ast, context);
maker.Modifiers(Flags.PRIVATE | Flags.FINAL | (isStatic[0] ? Flags.STATIC : 0)),
methodNode.toName(lockName), objectType, newObjectArray), ast, context);
injectFieldAndMarkGenerated(methodNode.up(), fieldDecl);
}
if (method.body == null) return;
JCExpression lockNode;
if (isStatic) {
lockNode = chainDots(methodNode, ast.pos, methodNode.up().getName(), lockName);
if (isStatic[0]) {
lockNode = namePlusTypeParamsToTypeReference(maker, typeNode, methodNode.toName(lockName), false, List.<JCTypeParameter>nil());
} else {
lockNode = maker.Select(maker.Ident(methodNode.toName("this")), methodNode.toName(lockName));
}
test/transform/resource/after-delombok/SynchronizedName.java
+1 -1
View File
@@ -8,7 +8,7 @@ class SynchronizedName {
}
}
void test4() {
synchronized (this.READ) {
synchronized (SynchronizedName.READ) {
System.out.println("four");
}
}
test/transform/resource/after-delombok/SynchronizedNameStaticToInstanceRef.java
+1 -3
View File
@@ -2,8 +2,6 @@ class SynchronizedNameStaticToInstanceRef {
private Object read = new Object();
private static Object READ = new Object();
static void test3() {
synchronized (SynchronizedNameStaticToInstanceRef.read) {
System.out.println("three");
}
System.out.println("three");
}
}
test/transform/resource/after-delombok/SynchronizedOnStatic.java
+18
View File
@@ -0,0 +1,18 @@
class SynchronizedOnStatic<Z> {
static class Inner {
private static Object LCK = new Object[0];
public void foo() {
synchronized (SynchronizedOnStatic.Inner.LCK) {
System.out.println();
}
}
}
class Inner2 {
private Object LCK = new Object[0];
public void foo() {
synchronized (this.LCK) {
System.out.println();
}
}
}
}
test/transform/resource/after-ecj/SynchronizedName.java
+1 -1
View File
@@ -13,7 +13,7 @@ class SynchronizedName {
}
}
@lombok.Synchronized("READ") void test4() {
synchronized (this.READ)
synchronized (SynchronizedName.READ)
{
System.out.println("four");
}
test/transform/resource/after-ecj/SynchronizedNameStaticToInstanceRef.java
+1 -4
View File
@@ -7,9 +7,6 @@ class SynchronizedNameStaticToInstanceRef {
super();
}
static @lombok.Synchronized("read") void test3() {
synchronized (SynchronizedNameStaticToInstanceRef.read)
{
System.out.println("three");
}
System.out.println("three");
}
}
test/transform/resource/after-ecj/SynchronizedOnStatic.java
+31
View File
@@ -0,0 +1,31 @@
class SynchronizedOnStatic<Z> {
static class Inner {
private static Object LCK = new Object[0];
<clinit>() {
}
Inner() {
super();
}
public @lombok.Synchronized("LCK") void foo() {
synchronized (SynchronizedOnStatic.Inner.LCK)
{
System.out.println();
}
}
}
class Inner2 {
private Object LCK = new Object[0];
Inner2() {
super();
}
public @lombok.Synchronized("LCK") void foo() {
synchronized (this.LCK)
{
System.out.println();
}
}
}
SynchronizedOnStatic() {
super();
}
}
test/transform/resource/before/SynchronizedOnStatic.java
+16
View File
@@ -0,0 +1,16 @@
class SynchronizedOnStatic<Z> {
static class Inner {
private static Object LCK = new Object[0];
@lombok.Synchronized("LCK")
public void foo() {
System.out.println();
}
}
class Inner2 {
private Object LCK = new Object[0];
@lombok.Synchronized("LCK")
public void foo() {
System.out.println();
}
}
}
test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages
+1 -1
View File
@@ -1 +1 @@
5 non-static variable read cannot be referenced from a static context
5 The field read is non-static and this cannot be used on this static method
test/transform/resource/messages-ecj/SynchronizedName.java.messages
-1
View File
@@ -1 +0,0 @@
8 The static field SynchronizedName.READ should be accessed in a static way
test/transform/resource/messages-ecj/SynchronizedNameNoSuchField.java.messages
+1 -1
View File
@@ -1 +1 @@
5 The field write does not exist.
5 The field write does not exist
test/transform/resource/messages-ecj/SynchronizedNameStaticToInstanceRef.java.messages
+1 -1
View File
@@ -1 +1 @@
5 Cannot make a static reference to the non-static field SynchronizedNameStaticToInstanceRef.read
5 The field read is non-static and thus cannot be used on this static method
test/transform/resource/messages-idempotent/SynchronizedNameStaticToInstanceRef.java.messages
-1
View File
@@ -1 +0,0 @@
5 non-static variable read cannot be referenced from a static context