You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/08/04 04:09:15 UTC
[groovy] branch master updated: GROOVY-9669: Enhance immutability
check (closes #1335)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 51b269b GROOVY-9669: Enhance immutability check (closes #1335)
51b269b is described below
commit 51b269bdf001a7d165e45f52ef5d9e13dd10bcdb
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 2 17:57:32 2020 +0800
GROOVY-9669: Enhance immutability check (closes #1335)
---
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..ec1c714 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 final class JcipImmutableValue {
+ private final String v = '234'
+ String toString() { v }
+ }
+
}