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 <[email protected]> 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 } + } + }
