You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/08/02 09:41:12 UTC
[groovy] branch GROOVY-9669 created (now f539e90)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch GROOVY-9669
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at f539e90 GROOVY-9669: Enhance immutability check
This branch includes the following new commits:
new f539e90 GROOVY-9669: Enhance immutability check
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-9669: Enhance immutability check
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY-9669
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f539e904ce460db9a38e7dbd6ac277ef6a536053
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 2 17:39:15 2020 +0800
GROOVY-9669: Enhance immutability check
---
build.gradle | 2 ++
.../groovy/ast/tools/ImmutablePropertyUtils.java | 17 +++++++++++------
.../org/codehaus/groovy/runtime/GStringImpl.java | 2 +-
src/test/groovy/GStringTest.groovy | 21 +++++++++++++++++++++
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/build.gradle b/build.gradle
index 7f2f1f2..55cc6dd 100644
--- a/build.gradle
+++ b/build.gradle
@@ -153,6 +153,7 @@ ext {
checkstyleVersion = '8.34'
junit5Version = '5.6.2'
junit5PlatformVersion = '1.6.2'
+ jcipAnnotationsVersion = '1.0'
}
dependencies {
@@ -206,6 +207,7 @@ dependencies {
testImplementation project(':groovy-test')
testImplementation project(':groovy-macro')
spotbugsPlugins 'com.h3xstream.findsecbugs:findsecbugs-plugin:1.10.1'
+ testImplementation "net.jcip:jcip-annotations:$jcipAnnotationsVersion"
}
ext.generatedDirectory = "${buildDir}/generated/sources"
diff --git a/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java b/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java
index c89b484..190f259 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java
@@ -19,7 +19,6 @@
package org.apache.groovy.ast.tools;
import groovy.transform.ImmutableOptions;
-import groovy.transform.KnownImmutable;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
@@ -54,7 +53,6 @@ public class ImmutablePropertyUtils {
private static final ClassNode CLONEABLE_TYPE = make(Cloneable.class);
private static final ClassNode DATE_TYPE = make(Date.class);
private static final ClassNode REFLECTION_INVOKER_TYPE = make(ReflectionMethodInvoker.class);
- private static final String KNOWN_IMMUTABLE_NAME = KnownImmutable.class.getName();
private static final Class<? extends Annotation> IMMUTABLE_OPTIONS_CLASS = ImmutableOptions.class;
public static final ClassNode IMMUTABLE_OPTIONS_TYPE = makeWithoutCaching(IMMUTABLE_OPTIONS_CLASS, false);
private static final String MEMBER_KNOWN_IMMUTABLE_CLASSES = "knownImmutableClasses";
@@ -127,6 +125,13 @@ public class ImmutablePropertyUtils {
"java.io.File"
));
+ private static final Set<String> BUILTIN_IMMUTABLE_ANNOTATIONS = new HashSet<String>(Arrays.asList(
+ "groovy.transform.Immutable",
+ "groovy.transform.KnownImmutable",
+// "javax.annotation.concurrent.Immutable", // its RetentionPolicy is CLASS, can not be got via reflection
+ "net.jcip.annotations.Immutable" // supported by Findbugs and IntelliJ IDEA
+ ));
+
private ImmutablePropertyUtils() { }
public static Expression cloneArrayOrCloneableExpr(Expression fieldExpr, ClassNode type) {
@@ -194,13 +199,13 @@ public class ImmutablePropertyUtils {
List<AnnotationNode> annotations = type.getAnnotations();
for (AnnotationNode next : annotations) {
String name = next.getClassNode().getName();
- if (matchingMarkerName(name)) return true;
+ if (matchingImmutableMarkerName(name)) return true;
}
return false;
}
- private static boolean matchingMarkerName(String name) {
- return name.equals("groovy.transform.Immutable") || name.equals(KNOWN_IMMUTABLE_NAME);
+ private static boolean matchingImmutableMarkerName(String name) {
+ return BUILTIN_IMMUTABLE_ANNOTATIONS.contains(name);
}
public static boolean isBuiltinImmutable(String typeName) {
@@ -211,7 +216,7 @@ public class ImmutablePropertyUtils {
Annotation[] annotations = clazz.getAnnotations();
for (Annotation next : annotations) {
String name = next.annotationType().getName();
- if (matchingMarkerName(name)) return true;
+ if (matchingImmutableMarkerName(name)) return true;
}
return false;
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/GStringImpl.java b/src/main/java/org/codehaus/groovy/runtime/GStringImpl.java
index 55bead7..6fb9d98 100644
--- a/src/main/java/org/codehaus/groovy/runtime/GStringImpl.java
+++ b/src/main/java/org/codehaus/groovy/runtime/GStringImpl.java
@@ -154,7 +154,7 @@ public class GStringImpl extends GString {
private static boolean checkValuesImmutable(Object[] values) {
for (Object value : values) {
if (null == value) continue;
- if (!(ImmutablePropertyUtils.isBuiltinImmutable(value.getClass().getName())
+ if (!(ImmutablePropertyUtils.builtinOrMarkedImmutableClass(value.getClass())
|| (value instanceof GStringImpl && ((GStringImpl) value).cacheable))) {
return false;
}
diff --git a/src/test/groovy/GStringTest.groovy b/src/test/groovy/GStringTest.groovy
index 452195e..82f6e29 100644
--- a/src/test/groovy/GStringTest.groovy
+++ b/src/test/groovy/GStringTest.groovy
@@ -631,5 +631,26 @@ class GStringTest extends GroovyTestCase {
def gstr12 = "a${"${123}"}b"
assert 'a123b' == gstr12
assert gstr12.toString() === gstr12.toString()
+
+ def gstr13 = "a${new GroovyImmutableValue()}b"
+ assert 'a123b' == gstr13
+ assert gstr13.toString() === gstr13.toString()
+
+ def gstr14 = "a${new JcipImmutableValue()}b"
+ assert 'a234b' == gstr14
+ assert gstr14.toString() === gstr14.toString()
+ }
+
+ @groovy.transform.Immutable
+ static class GroovyImmutableValue {
+ private final String v = '123'
+ String toString() { v }
}
+
+ @net.jcip.annotations.Immutable
+ static class JcipImmutableValue {
+ private final String v = '234'
+ String toString() { v }
+ }
+
}