Repository: groovy Updated Branches: refs/heads/master 6f73fb997 -> c170300cd
check for known immutables before checking for handled cases plus minor refactoring Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/c170300c Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/c170300c Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/c170300c Branch: refs/heads/master Commit: c170300cdc8976ed0fed37bda412c7389e85971d Parents: 6f73fb9 Author: paulk <[email protected]> Authored: Mon Feb 5 11:47:05 2018 +1000 Committer: paulk <[email protected]> Committed: Mon Feb 5 11:47:05 2018 +1000 ---------------------------------------------------------------------- .../transform/ImmutableASTTransformation.java | 116 +++++++++++-------- 1 file changed, 70 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/c170300c/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java index 79c7c46..3e8fd8f 100644 --- a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java @@ -167,10 +167,9 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { "java.time.temporal.ValueRange", "java.time.temporal.WeekFields" )); - private static final Class MY_CLASS = ImmutableBase.class; - private static final Class<? extends Annotation> KNOWN_IMMUTABLE_CLASS = KnownImmutable.class; - private static final Class<? extends Annotation> IMMUTABLE_BASE_CLASS = ImmutableBase.class; - private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(IMMUTABLE_BASE_CLASS, false); + private static final String KNOWN_IMMUTABLE_NAME = KnownImmutable.class.getName(); + private static final Class<? extends Annotation> MY_CLASS = ImmutableBase.class; + private static final ClassNode IMMUTABLE_BASE_TYPE = makeWithoutCaching(MY_CLASS, false); public static final ClassNode MY_TYPE = make(MY_CLASS); static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final String MEMBER_KNOWN_IMMUTABLE_CLASSES = "knownImmutableClasses"; @@ -507,10 +506,10 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { FieldNode fNode = pNode.getField(); final ClassNode fieldType = fNode.getType(); Statement statement; - if (fieldType.isArray() || isOrImplements(fieldType, CLONEABLE_TYPE)) { - statement = createConstructorStatementArrayOrCloneable(fNode, namedArgs); - } else if (isKnownImmutableClass(fieldType, knownImmutableClasses) || isKnownImmutable(pNode.getName(), knownImmutables)) { + if (isKnownImmutableType(fieldType, knownImmutableClasses) || isKnownImmutable(pNode.getName(), knownImmutables)) { statement = createConstructorStatementDefault(fNode, namedArgs); + } else if (fieldType.isArray() || isOrImplements(fieldType, CLONEABLE_TYPE)) { + statement = createConstructorStatementArrayOrCloneable(fNode, namedArgs); } else if (fieldType.isDerivedFrom(DATE_TYPE)) { statement = createConstructorStatementDate(fNode, namedArgs); } else if (isOrImplements(fieldType, COLLECTION_TYPE) || fieldType.isDerivedFrom(COLLECTION_TYPE) || isOrImplements(fieldType, MAP_TYPE) || fieldType.isDerivedFrom(MAP_TYPE)) { @@ -588,33 +587,6 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return assignWithDefault(namedArgs, assignInit, param, assignStmt); } - private static boolean isKnownImmutableClass(ClassNode fieldType, List<String> knownImmutableClasses) { - if (inImmutableList(fieldType.getName()) || knownImmutableClasses.contains(fieldType.getName())) - return true; - if (!fieldType.isResolved()) - return false; - if ("java.util.Optional".equals(fieldType.getName()) && fieldType.getGenericsTypes() != null && fieldType.getGenericsTypes().length == 1) { - GenericsType optionalType = fieldType.getGenericsTypes()[0]; - if (optionalType.isResolved() && !optionalType.isPlaceholder() && !optionalType.isWildcard()) { - String name = optionalType.getType().getName(); - if (inImmutableList(name) || knownImmutableClasses.contains(name)) return true; - if (optionalType.getType().isEnum() || !optionalType.getType().getAnnotations(MY_TYPE).isEmpty()) - return true; - } - } - return fieldType.isEnum() || - ClassHelper.isPrimitiveType(fieldType) || - !fieldType.getAnnotations(MY_TYPE).isEmpty(); - } - - private static boolean isKnownImmutable(String fieldName, List<String> knownImmutables) { - return knownImmutables.contains(fieldName); - } - - private static boolean inImmutableList(String typeName) { - return builtinImmutables.contains(typeName); - } - private static Statement createConstructorStatementArrayOrCloneable(FieldNode fNode, boolean namedArgs) { final Expression fieldExpr = propX(varX("this"), fNode.getName()); final Expression initExpr = fNode.getInitialValueExpression(); @@ -805,7 +777,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { */ @SuppressWarnings("Unchecked") public static Object checkImmutable(String className, String fieldName, Object field) { - if (field == null || field instanceof Enum || inImmutableList(field.getClass().getName())) return field; + if (field == null || field instanceof Enum || isBuiltinImmutable(field.getClass().getName())) return field; if (field instanceof Collection) return DefaultGroovyMethods.asImmutable((Collection) field); if (getAnnotationByName(field, "groovy.transform.Immutable") != null) return field; @@ -823,9 +795,12 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return null; } + /** + * For compatibility with pre 2.5 compiled classes + */ @SuppressWarnings("Unchecked") - public static Object checkImmutable(Class<?> clazz, String fieldName, Object field, List<String> knownImmutableFieldNames, List<Class> knownImmutableClasses) { - if (field == null || field instanceof Enum || knownImmutable(field.getClass()) || knownImmutableFieldNames.contains(fieldName) || knownImmutableClasses.contains(field.getClass())) { + public static Object checkImmutable(Class<?> clazz, String fieldName, Object field) { + if (field == null || field instanceof Enum || builtinOrMarkedImmutableClass(field.getClass())) { return field; } @@ -847,7 +822,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return DefaultGroovyMethods.asImmutable((Collection) field); } // potentially allow Collection coercion for a constructor - if (knownImmutable(fieldType)) { + if (builtinOrMarkedImmutableClass(fieldType)) { return field; } } catch (NoSuchFieldException ignore) { @@ -858,12 +833,9 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { throw new RuntimeException(createErrorMessage(clazz.getName(), fieldName, typeName, "constructing")); } - /* - * For compatibility with pre 2.5 compiled classes - */ @SuppressWarnings("Unchecked") - public static Object checkImmutable(Class<?> clazz, String fieldName, Object field) { - if (field == null || field instanceof Enum || knownImmutable(field.getClass())) { + public static Object checkImmutable(Class<?> clazz, String fieldName, Object field, List<String> knownImmutableFieldNames, List<Class> knownImmutableClasses) { + if (field == null || field instanceof Enum || builtinOrMarkedImmutableClass(field.getClass()) || knownImmutableFieldNames.contains(fieldName) || knownImmutableClasses.contains(field.getClass())) { return field; } @@ -885,7 +857,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return DefaultGroovyMethods.asImmutable((Collection) field); } // potentially allow Collection coercion for a constructor - if (knownImmutable(fieldType)) { + if (builtinOrMarkedImmutableClass(fieldType) || knownImmutableClasses.contains(fieldType)) { return field; } } catch (NoSuchFieldException ignore) { @@ -896,8 +868,60 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { throw new RuntimeException(createErrorMessage(clazz.getName(), fieldName, typeName, "constructing")); } - private static boolean knownImmutable(Class<?> clazz) { - return inImmutableList(clazz.getName()) || clazz.getAnnotation(KNOWN_IMMUTABLE_CLASS) != null; + private static boolean isKnownImmutableType(ClassNode fieldType, List<String> knownImmutableClasses) { + if (builtinOrDeemedType(fieldType, knownImmutableClasses)) + return true; + if (!fieldType.isResolved()) + return false; + if ("java.util.Optional".equals(fieldType.getName()) && fieldType.getGenericsTypes() != null && fieldType.getGenericsTypes().length == 1) { + GenericsType optionalType = fieldType.getGenericsTypes()[0]; + if (optionalType.isResolved() && !optionalType.isPlaceholder() && !optionalType.isWildcard()) { + ClassNode valueType = optionalType.getType(); + if (builtinOrDeemedType(valueType, knownImmutableClasses)) return true; + if (valueType.isEnum()) return true; + } + } + return fieldType.isEnum() || + ClassHelper.isPrimitiveType(fieldType) || + hasImmutableAnnotation(fieldType); + } + + private static boolean builtinOrDeemedType(ClassNode fieldType, List<String> knownImmutableClasses) { + return isBuiltinImmutable(fieldType.getName()) || knownImmutableClasses.contains(fieldType.getName()) || hasImmutableAnnotation(fieldType); + } + + private static boolean hasImmutableAnnotation(ClassNode type) { + List<AnnotationNode> annotations = type.getAnnotations(); + for (AnnotationNode next : annotations) { + String name = next.getClassNode().getName(); + if (matchingMarkerName(name)) return true; + } + return false; + } + + private static boolean hasImmutableAnnotation(Class clazz) { + Annotation[] annotations = clazz.getAnnotations(); + for (Annotation next : annotations) { + String name = next.annotationType().getName(); + if (matchingMarkerName(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 isKnownImmutable(String fieldName, List<String> knownImmutables) { + return knownImmutables.contains(fieldName); + } + + private static boolean isBuiltinImmutable(String typeName) { + return builtinImmutables.contains(typeName); + } + + private static boolean builtinOrMarkedImmutableClass(Class<?> clazz) { + return isBuiltinImmutable(clazz.getName()) || hasImmutableAnnotation(clazz); } public static void checkPropNames(Object instance, Map<String, Object> args) {
