groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject groovy git commit: GROOVY-7981: Not public constructors for groovy.transform.Immutable annotated class (closes #453, closes #665)
Date Thu, 22 Feb 2018 07:20:05 GMT
Repository: groovy
Updated Branches:
  refs/heads/master 40d337def -> a98efdb3f


GROOVY-7981: Not public constructors for groovy.transform.Immutable annotated class (closes
#453, closes #665)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/a98efdb3
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/a98efdb3
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/a98efdb3

Branch: refs/heads/master
Commit: a98efdb3f2eb5453e1d541cef9ce7dff9e811331
Parents: 40d337d
Author: paulk <paulk@asert.com.au>
Authored: Thu Feb 22 15:25:07 2018 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Thu Feb 22 17:17:58 2018 +1000

----------------------------------------------------------------------
 .../groovy/groovy/transform/MapConstructor.java | 21 +++++++
 .../groovy/groovy/transform/NamedVariant.java   | 15 ++++-
 .../groovy/transform/TupleConstructor.java      | 21 +++++++
 .../transform/options/PropertyHandler.java      |  5 ++
 .../groovy/transform/options/Visibility.java    | 25 ++++++--
 .../groovy/ast/tools/VisibilityUtils.java       | 40 ++++++-------
 .../groovy/classgen/EnumCompletionVisitor.java  |  7 ++-
 .../MapConstructorASTTransformation.java        | 10 ++--
 .../NamedVariantASTTransformation.java          | 15 +++--
 .../TupleConstructorASTTransformation.java      | 19 +++---
 src/spec/doc/core-metaprogramming.adoc          |  7 +++
 .../test/CodeGenerationASTTransformsTest.groovy |  5 ++
 .../TupleConstructorTransformTest.groovy        | 63 ++++++++++++++++++++
 13 files changed, 200 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/groovy/groovy/transform/MapConstructor.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/MapConstructor.java b/src/main/groovy/groovy/transform/MapConstructor.java
index d6aec1c..8c13284 100644
--- a/src/main/groovy/groovy/transform/MapConstructor.java
+++ b/src/main/groovy/groovy/transform/MapConstructor.java
@@ -67,6 +67,19 @@ import java.lang.annotation.Target;
  * }
  * </pre>
  * <p>
+ * Custom visibility:
+ * <ul>
+ * <li>The {@code @MapConstructor} annotation generates a public constructor unless
an applicable
+ * {@link VisibilityOptions} annotation is also present. It can be useful to change the visibility
+ * if you want to also create a builder or provide your own static factory method for object
creation.
+ * You can make the constructor private and access it from the builder or your factory method.
(Note:
+ * you'll probably want to use {@code @CompileStatic} in conjunction with such an approach
since
+ * dynamic Groovy currently gives the ability to access even private constructors.)</li>
+ * <li>An optional {@code visibilityId} attribute can be specified. If present, it
must match the optional
+ * {@code id} attribute of an applicable {@code VisibilityOptions} annotation. This can be
useful
+ * if multiple {@code VisibilityOptions} annotations are needed.</li>
+ * </ul>
+ * <p>
  * Custom property handling:
  * <ul>
  * <li>The {@code @MapConstructor} annotation supports customization using {@code @PropertyOptions}
@@ -92,6 +105,7 @@ import java.lang.annotation.Target;
  * </ul>
  *
  * @see PropertyOptions
+ * @see VisibilityOptions
  * @since 2.5.0
  */
 @java.lang.annotation.Documented
@@ -179,6 +193,13 @@ public @interface MapConstructor {
     boolean specialNamedArgHandling() default true;
 
     /**
+     * If specified, must match the "id" attribute in a VisibilityOptions annotation to enable
a custom visibility.
+     *
+     * @since 2.5.0
+     */
+    String visibilityId() default Undefined.STRING;
+
+    /**
      * A Closure containing statements which will be prepended to the generated constructor.
The first statement within the Closure may be "super(someArgs)" in which case the no-arg super
constructor won't be called.
      */
     Class pre() default Undefined.CLASS.class;

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/groovy/groovy/transform/NamedVariant.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/NamedVariant.java b/src/main/groovy/groovy/transform/NamedVariant.java
index 2a10470..e3823cf 100644
--- a/src/main/groovy/groovy/transform/NamedVariant.java
+++ b/src/main/groovy/groovy/transform/NamedVariant.java
@@ -52,6 +52,8 @@ import java.lang.annotation.Target;
  *     <li>If no arguments with {@code @NamedParam} or {@code @NamedDelegate} annotations
are found the
  *     first argument is assumed to be an implicit named delegate</li>
  * </ol>
+ * You can also mix and match the {@code @NamedParam} and {@code @NamedDelegate} annotations.
+ *
  * Named arguments will be supplied via the map with their property name (configurable via
  * annotation attributes within {@code @NamedParam}) being the key and value being the argument
value.
  * For named delegates, any properties of the delegate can become map keys. Duplicate keys
across
@@ -75,15 +77,22 @@ import java.lang.annotation.Target;
  * def result = foo(g: 12, b: 42, r: 12)
  * assert result.toString() == 'Color(r:12, g:12, b:42)'
  * </pre>
+ * You could also explicitly annotate the {@code shade} argument with the {@code @NamedDelegate}
annotation if you wanted.
+ *
  * The generated method will be something like this:
  * <pre>
  * String foo(Map args) {
  *     return foo(args as Color)
  * }
  * </pre>
- * The generated method/constructor retains the visibility and return type of the original
- * but the {@code @VisibilityOptions} annotation can be added to the visibility. You could
have the
+ * The generated method/constructor retains the visibility and return type of the original
method/constructor
+ * but the {@link @VisibilityOptions} annotation can be added to customize the visibility.
You could have the
  * annotated method/constructor private for instance but have the generated one be public.
+ *
+ * @see VisibilityOptions
+ * @see NamedParam
+ * @see NamedDelegate
+ * @since 2.5.0
  */
 @Incubating
 @Retention(RetentionPolicy.SOURCE)
@@ -91,7 +100,7 @@ import java.lang.annotation.Target;
 @GroovyASTTransformationClass("org.codehaus.groovy.transform.NamedVariantASTTransformation")
 public @interface NamedVariant {
     /**
-     * If specified, must match the "id" attribute in the VisibilityOptions annotation.
+     * If specified, must match the optional "id" attribute in an applicable {@code VisibilityOptions}
annotation.
      */
     String visibilityId() default Undefined.STRING;
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/groovy/groovy/transform/TupleConstructor.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/TupleConstructor.java b/src/main/groovy/groovy/transform/TupleConstructor.java
index 32af45f..651f2ca 100644
--- a/src/main/groovy/groovy/transform/TupleConstructor.java
+++ b/src/main/groovy/groovy/transform/TupleConstructor.java
@@ -158,6 +158,19 @@ import java.lang.annotation.Target;
  * assert student.courses == ['IT']
  * </pre>
  * <p>
+ * Custom visibility:
+ * <ul>
+ * <li>The {@code @TupleConstructor} annotation generates a public constructor unless
an applicable
+ * {@link VisibilityOptions} annotation is also present. It can be useful to change the visibility
+ * if you want to also create a builder or provide your own static factory method for object
creation.
+ * You can make the constructor private and access it from the builder or your factory method.
(Note:
+ * you'll probably want to use {@code @CompileStatic} in conjunction with such an approach
since
+ * dynamic Groovy currently gives the ability to access even private constructors.)</li>
+ * <li>An optional {@code visibilityId} attribute can be specified. If present, it
must match the optional
+ * {@code id} attribute of an applicable {@code VisibilityOptions} annotation. This can be
useful
+ * if multiple {@code VisibilityOptions} annotations are needed.</li>
+ * </ul>
+ * <p>
  * Custom property handling:
  * <ul>
  * <li>The {@code @TupleConstructor} annotation supports customization using {@code
@PropertyOptions}
@@ -197,6 +210,7 @@ import java.lang.annotation.Target;
  * </ul>
  *
  * @see PropertyOptions
+ * @see VisibilityOptions
  * @since 1.8.0
  */
 @java.lang.annotation.Documented
@@ -307,6 +321,13 @@ public @interface TupleConstructor {
     boolean allProperties() default false;
 
     /**
+     * If specified, must match the "id" attribute in a VisibilityOptions annotation to enable
a custom visibility.
+     *
+     * @since 2.5.0
+     */
+    String visibilityId() default Undefined.STRING;
+
+    /**
      * A Closure containing statements which will be prepended to the generated constructor.
The first statement
      * within the Closure may be {@code super(someArgs)} in which case the no-arg super constructor
won't be called.
      *

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/groovy/groovy/transform/options/PropertyHandler.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/options/PropertyHandler.java b/src/main/groovy/groovy/transform/options/PropertyHandler.java
index 1a4acc0..fc9029a 100644
--- a/src/main/groovy/groovy/transform/options/PropertyHandler.java
+++ b/src/main/groovy/groovy/transform/options/PropertyHandler.java
@@ -35,6 +35,11 @@ import java.util.List;
 
 import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
 
+/**
+ * Used to provide custom property handling when getting, setting or initializing properties.
+ *
+ * @since 2.5.0
+ */
 @Incubating
 public abstract class PropertyHandler {
     private static final Class<? extends Annotation> PROPERTY_OPTIONS_CLASS = PropertyOptions.class;

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/groovy/groovy/transform/options/Visibility.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/options/Visibility.java b/src/main/groovy/groovy/transform/options/Visibility.java
index e0ea899..d95c338 100644
--- a/src/main/groovy/groovy/transform/options/Visibility.java
+++ b/src/main/groovy/groovy/transform/options/Visibility.java
@@ -18,10 +18,25 @@
  */
 package groovy.transform.options;
 
+import java.lang.reflect.Modifier;
+
 public enum Visibility {
-    PUBLIC,
-    PROTECTED,
-    PACKAGE_PRIVATE,
-    PRIVATE,
-    UNDEFINED
+    PUBLIC(Modifier.PUBLIC),
+    PROTECTED(Modifier.PROTECTED),
+    PACKAGE_PRIVATE(0),
+    PRIVATE(Modifier.PRIVATE),
+    UNDEFINED(-1);
+
+    private final int modifier;
+
+    Visibility(int modifier) {
+        this.modifier = modifier;
+    }
+
+    public int getModifier() {
+        if (modifier == -1) {
+            throw new UnsupportedOperationException("getModifier() not supported for UNDEFINED");
+        }
+        return modifier;
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/java/org/apache/groovy/ast/tools/VisibilityUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/groovy/ast/tools/VisibilityUtils.java b/src/main/java/org/apache/groovy/ast/tools/VisibilityUtils.java
index 46fefad..124ad35 100644
--- a/src/main/java/org/apache/groovy/ast/tools/VisibilityUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/VisibilityUtils.java
@@ -44,52 +44,48 @@ public class VisibilityUtils {
     private VisibilityUtils() {
     }
 
-    public static int getVisibility(AnnotationNode anno, AnnotatedNode node, int originalModifiers)
{
-
+    /**
+     * Determine the correct modifiers by looking for a potential @VisibilityOptions annotation.
+     *
+     * @param anno The annotation being processed (if any) which may support a 'visibilityId'
attribute
+     * @param node The node being processed which may also be annotated with @VisibilityOptions
+     * @param clazz The type of node being constructed
+     * @param originalModifiers The modifier value to adjust or return if no applicable @VisibilityOptions
is found
+     * @return the updated modifiers
+     */
+    public static int getVisibility(AnnotationNode anno, AnnotatedNode node, Class<? extends
AnnotatedNode> clazz, int originalModifiers) {
         List<AnnotationNode> annotations = node.getAnnotations(VISIBILITY_OPTIONS_TYPE);
-        if (annotations.isEmpty()) return originalModifiers;
+        if (annotations.isEmpty() || anno == null) return originalModifiers;
 
         String visId = getMemberStringValue(anno, "visibilityId", null);
 
         Visibility vis = null;
         if (visId == null) {
-            vis = getVisForAnnotation(node, annotations.get(0), null);
+            vis = getVisForAnnotation(clazz, annotations.get(0), null);
         } else {
             for (AnnotationNode visAnno : annotations) {
-                vis = getVisForAnnotation(node, visAnno, visId);
+                vis = getVisForAnnotation(clazz, visAnno, visId);
                 if (vis != Visibility.UNDEFINED) break;
             }
         }
         if (vis == null || vis == Visibility.UNDEFINED) return originalModifiers;
 
         int result = originalModifiers & ~(ACC_PUBLIC | ACC_PROTECTED | ACC_PRIVATE);
-        switch (vis) {
-            case PUBLIC:
-                result |= ACC_PUBLIC;
-                break;
-            case PROTECTED:
-                result |= ACC_PROTECTED;
-                break;
-            case PRIVATE:
-                result |= ACC_PRIVATE;
-                break;
-
-        }
-        return result;
+        return result | vis.getModifier();
     }
 
-    private static Visibility getVisForAnnotation(AnnotatedNode node, AnnotationNode visAnno,
String visId) {
+    private static Visibility getVisForAnnotation(Class<? extends AnnotatedNode> clazz,
AnnotationNode visAnno, String visId) {
         Map<String, Expression> visMembers = visAnno.getMembers();
         if (visMembers == null) return Visibility.UNDEFINED;
         String id = getMemberStringValue(visAnno, "id", null);
         if ((id == null && visId != null) || (id != null && !id.equals(visId)))
return Visibility.UNDEFINED;
 
         Visibility vis = null;
-        if (node instanceof ConstructorNode) {
+        if (clazz.equals(ConstructorNode.class)) {
             vis = getVisibility(visMembers.get("constructor"));
-        } else if (node instanceof MethodNode) {
+        } else if (clazz.equals(MethodNode.class)) {
             vis = getVisibility(visMembers.get("method"));
-        } else if (node instanceof ClassNode) {
+        } else if (clazz.equals(ClassNode.class)) {
             vis = getVisibility(visMembers.get("type"));
         }
         if (vis == null || vis == Visibility.UNDEFINED) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
index cba687e..8ef3a04 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
@@ -37,11 +37,12 @@ import org.codehaus.groovy.ast.stmt.Statement;
 import org.codehaus.groovy.control.CompilationUnit;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.transform.TupleConstructorASTTransformation;
-import org.objectweb.asm.Opcodes;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+
 /**
  * Enums have a parent constructor with two arguments from java.lang.Enum.
  * This visitor adds those two arguments into manually created constructors
@@ -85,7 +86,7 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
                 addMapConstructors(enumClass);
             } else {
                 for (ConstructorNode constructorNode : sctors) {
-                    ConstructorNode init = new ConstructorNode(Opcodes.ACC_PUBLIC, constructorNode.getParameters(),
ClassNode.EMPTY_ARRAY, new BlockStatement());
+                    ConstructorNode init = new ConstructorNode(ACC_PUBLIC, constructorNode.getParameters(),
ClassNode.EMPTY_ARRAY, new BlockStatement());
                     enumClass.addConstructor(init);
                 }
             }
@@ -144,7 +145,7 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
     }
 
     private static void addMapConstructors(ClassNode enumClass) {
-        TupleConstructorASTTransformation.addSpecialMapConstructors(enumClass, "One of the
enum constants for enum " + enumClass.getName() +
+        TupleConstructorASTTransformation.addSpecialMapConstructors(ACC_PUBLIC, enumClass,
"One of the enum constants for enum " + enumClass.getName() +
                 " was initialized with null. Please use a non-null value or define your own
constructor.", true);
     }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
index aa995ef..51de450 100644
--- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
@@ -51,6 +51,7 @@ import java.util.Map;
 import java.util.Set;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.hasNoArgConstructor;
+import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
 import static org.codehaus.groovy.ast.ClassHelper.make;
 import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
@@ -173,9 +174,10 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation
i
             ClosureExpression transformed = (ClosureExpression) transformer.transform(post);
             body.addStatement(transformed.getCode());
         }
-        doAddConstructor(cNode, new ConstructorNode(ACC_PUBLIC, params, ClassNode.EMPTY_ARRAY,
body));
+        int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
+        doAddConstructor(cNode, new ConstructorNode(modifiers, params, ClassNode.EMPTY_ARRAY,
body));
         if (noArg && !superList.isEmpty() && !hasNoArgConstructor(cNode)/*
&& !specialNamedArgCase*/) {
-            createNoArgConstructor(cNode);
+            createNoArgConstructor(cNode, modifiers);
         }
     }
 
@@ -220,9 +222,9 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation
i
         }
     }
 
-    private static void createNoArgConstructor(ClassNode cNode) {
+    private static void createNoArgConstructor(ClassNode cNode, int modifiers) {
         Statement body = stmt(ctorX(ClassNode.THIS, args(new MapExpression())));
-        cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY,
body));
+        cNode.addConstructor(new ConstructorNode(modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY,
body));
     }
 
     private static ClassCodeExpressionTransformer makeMapTypedArgsTransformer() {

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
index 70053bd..3a7e3ab 100644
--- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
@@ -75,7 +75,6 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
     private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
     private static final ClassNode NAMED_PARAM_TYPE = makeWithoutCaching(NamedParam.class,
false);
     private static final ClassNode NAMED_DELEGATE_TYPE = makeWithoutCaching(NamedDelegate.class,
false);
-    private static final ClassNode MAP_TYPE = makeWithoutCaching(Map.class, false);
 
     @Override
     public void visit(ASTNode[] nodes, SourceUnit source) {
@@ -121,7 +120,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
                     if (getMemberValue(namedParam, "type") == null) {
                         namedParam.addMember("type", classX(fromParam.getType()));
                     }
-                    if (!checkDuplicates(mNode, propNames, name)) return;
+                    if (hasDuplicates(mNode, propNames, name)) return;
                     // TODO check specified type is assignable from declared param type?
                     // ClassNode type = getMemberClassValue(namedParam, "type");
                     if (required) {
@@ -139,7 +138,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
                     if (!processDelegateParam(mNode, mapParam, args, propNames, fromParam))
return;
                 } else {
                     args.addExpression(varX(fromParam));
-                    if (!checkDuplicates(mNode, propNames, fromParam.getName())) return;
+                    if (hasDuplicates(mNode, propNames, fromParam.getName())) return;
                     genParams.add(fromParam);
                 }
             }
@@ -163,7 +162,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
         }
 
         final BlockStatement body = new BlockStatement();
-        int modifiers = getVisibility(anno, mNode, mNode.getModifiers());
+        int modifiers = getVisibility(anno, mNode, mNode.getClass(), mNode.getModifiers());
         if (mNode instanceof ConstructorNode) {
             body.addStatement(stmt(ctorX(ClassNode.THIS, args)));
             body.addStatement(inner);
@@ -198,7 +197,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
         Set<String> names = new HashSet<String>();
         List<PropertyNode> props = getAllProperties(names, fromParam.getType(), true,
false, false, true, false, true);
         for (String next : names) {
-            if (!checkDuplicates(mNode, propNames, next)) return false;
+            if (hasDuplicates(mNode, propNames, next)) return false;
         }
         List<MapEntryExpression> entries = new ArrayList<MapEntryExpression>();
         for (PropertyNode pNode : props) {
@@ -214,12 +213,12 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation
{
         return true;
     }
 
-    private boolean checkDuplicates(MethodNode mNode, List<String> propNames, String
next) {
+    private boolean hasDuplicates(MethodNode mNode, List<String> propNames, String
next) {
         if (propNames.contains(next)) {
             addError("Error during " + MY_TYPE_NAME + " processing. Duplicate property '"
+ next + "' found.", mNode);
-            return false;
+            return true;
         }
         propNames.add(next);
-        return true;
+        return false;
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index d225dfb..9696d15 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -55,6 +55,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
 import static org.codehaus.groovy.ast.ClassHelper.make;
 import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
@@ -86,7 +87,6 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
     static final ClassNode MY_TYPE = make(MY_CLASS);
     static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
     private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false);
-    private static final ClassNode HMAP_TYPE = makeWithoutCaching(HashMap.class, false);
     private static final ClassNode CHECK_METHOD_TYPE = make(ImmutableASTTransformation.class);
     private static final Class<? extends Annotation> MAP_CONSTRUCTOR_CLASS = MapConstructor.class;
     private static final Map<Class<?>, Expression> primitivesInitialValues;
@@ -128,8 +128,10 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
             List<String> includes = getMemberStringList(anno, "includes");
             boolean allNames = memberHasValue(anno, "allNames", true);
             if (!checkIncludeExcludeUndefinedAware(anno, excludes, includes, MY_TYPE_NAME))
return;
-            if (!checkPropertyList(cNode, includes, "includes", anno, MY_TYPE_NAME, includeFields,
includeSuperProperties, allProperties, includeSuperFields, false)) return;
-            if (!checkPropertyList(cNode, excludes, "excludes", anno, MY_TYPE_NAME, includeFields,
includeSuperProperties, allProperties, includeSuperFields, false)) return;
+            if (!checkPropertyList(cNode, includes, "includes", anno, MY_TYPE_NAME, includeFields,
includeSuperProperties, allProperties, includeSuperFields, false))
+                return;
+            if (!checkPropertyList(cNode, excludes, "excludes", anno, MY_TYPE_NAME, includeFields,
includeSuperProperties, allProperties, includeSuperFields, false))
+                return;
             final GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader()
: source.getClassLoader();
             final PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader,
cNode);
             if (handler == null) return;
@@ -251,7 +253,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         }
 
         boolean hasMapCons = hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
-        cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, params.toArray(new Parameter[params.size()]),
ClassNode.EMPTY_ARRAY, body));
+        int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
+        cNode.addConstructor(new ConstructorNode(modifiers, params.toArray(new Parameter[params.size()]),
ClassNode.EMPTY_ARRAY, body));
 
         if (sourceUnit != null && !body.isEmpty()) {
             VariableScopeVisitor scopeVisitor = new VariableScopeVisitor(sourceUnit);
@@ -266,7 +269,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
             ClassNode firstParamType = params.get(0).getType();
             if (params.size() > 1 || firstParamType.equals(ClassHelper.OBJECT_TYPE)) {
                 String message = "The class " + cNode.getName() + " was incorrectly initialized
via the map constructor with null.";
-                addSpecialMapConstructors(cNode, message, false);
+                addSpecialMapConstructors(modifiers, cNode, message, false);
             }
         }
     }
@@ -293,7 +296,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         return initialExp;
     }
 
-    public static void addSpecialMapConstructors(ClassNode cNode, String message, boolean
addNoArg) {
+    public static void addSpecialMapConstructors(int modifiers, ClassNode cNode, String message,
boolean addNoArg) {
         Parameter[] parameters = params(new Parameter(LHMAP_TYPE, "__namedArgs"));
         BlockStatement code = new BlockStatement();
         VariableExpression namedArgs = varX("__namedArgs");
@@ -301,13 +304,13 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         code.addStatement(ifElseS(equalsNullX(namedArgs),
                 illegalArgumentBlock(message),
                 processArgsBlock(cNode, namedArgs)));
-        ConstructorNode init = new ConstructorNode(ACC_PUBLIC, parameters, ClassNode.EMPTY_ARRAY,
code);
+        ConstructorNode init = new ConstructorNode(modifiers, parameters, ClassNode.EMPTY_ARRAY,
code);
         cNode.addConstructor(init);
         // potentially add a no-arg constructor too
         if (addNoArg) {
             code = new BlockStatement();
             code.addStatement(stmt(ctorX(ClassNode.THIS, ctorX(LHMAP_TYPE))));
-            init = new ConstructorNode(ACC_PUBLIC, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY,
code);
+            init = new ConstructorNode(modifiers, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY,
code);
             cNode.addConstructor(init);
         }
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/spec/doc/core-metaprogramming.adoc
----------------------------------------------------------------------
diff --git a/src/spec/doc/core-metaprogramming.adoc b/src/spec/doc/core-metaprogramming.adoc
index 7230e11..888bed9 100644
--- a/src/spec/doc/core-metaprogramming.adoc
+++ b/src/spec/doc/core-metaprogramming.adoc
@@ -1769,6 +1769,13 @@ during class construction. It is ignored by the main Groovy compiler
but is refe
 like `@TupleConstructor`, `@MapConstructor`, and `@ImmutableBase`. It is frequently used
behind the
 scenes by the `@Immutable` meta-annotation.
 
+[[xform-VisibilityOptions]]
+===== `@groovy.transform.VisibilityOptions`
+
+This annotation allows you to specify a custom visibility for a construct generated by another
transformation.
+It is ignored by the main Groovy compiler but is referenced by other transformations
+like `@TupleConstructor`, `@MapConstructor`, and `@NamedVariant`.
+
 [[xform-ImumtableOptions]]
 ===== `@groovy.transform.ImmutableOptions`
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/spec/test/CodeGenerationASTTransformsTest.groovy
----------------------------------------------------------------------
diff --git a/src/spec/test/CodeGenerationASTTransformsTest.groovy b/src/spec/test/CodeGenerationASTTransformsTest.groovy
index f2bb2b7..efc73ae 100644
--- a/src/spec/test/CodeGenerationASTTransformsTest.groovy
+++ b/src/spec/test/CodeGenerationASTTransformsTest.groovy
@@ -1434,15 +1434,20 @@ firstLastAge()
 // tag::builder_initializer_immutable[]
 import groovy.transform.builder.*
 import groovy.transform.*
+import static groovy.transform.options.Visibility.PRIVATE
 
 @Builder(builderStrategy=InitializerStrategy)
 @Immutable
+@VisibilityOptions(PRIVATE)
 class Person {
     String first
     String last
     int born
 }
 
+def publicCons = Person.constructors
+assert publicCons.size() == 1
+
 @CompileStatic
 def createFirstLastBorn() {
   def p = new Person(Person.createInitializer().first('Johnny').last('Depp').born(1963))

http://git-wip-us.apache.org/repos/asf/groovy/blob/a98efdb3/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
index 991cdb4..71050ff 100644
--- a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
@@ -214,6 +214,69 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
         '''
     }
 
+    // GROOVY-7981
+    void testVisibilityOptions() {
+        assertScript '''
+            import groovy.transform.*
+            import static groovy.transform.options.Visibility.*
+            import static java.lang.reflect.Modifier.isPrivate
+            import static org.codehaus.groovy.control.CompilePhase.*
+
+            @VisibilityOptions(PRIVATE)
+            @Immutable
+            @ASTTest(phase = CANONICALIZATION,
+                     value = {
+                         node.constructors.every { isPrivate(it.modifiers) }
+                     })
+            class Person {
+                String first, last
+                int age
+                static makePerson(Map args) {
+                    new Person(args)
+                }
+            }
+
+            @CompileStatic
+            def method() {
+                def p = Person.makePerson(first: 'John', last: 'Smith', age: 20)
+                assert p.toString() == 'Person(John, Smith, 20)'
+            }
+            method()
+        '''
+    }
+
+    // GROOVY-7981
+    void testMultipleVisibilityOptions() {
+        assertScript '''
+            import groovy.transform.*
+            import java.lang.reflect.Modifier
+            import static groovy.transform.options.Visibility.*
+            import static org.codehaus.groovy.control.CompilePhase.*
+
+            @VisibilityOptions(value = PROTECTED, id = 'first_only')
+            @VisibilityOptions(constructor = PRIVATE, id = 'age_only')
+            @TupleConstructor(visibilityId = 'first_only', includes = 'first', defaults =
false, force = true)
+            @TupleConstructor(visibilityId = 'age_only', includes = 'age', defaults = false,
force = true)
+            @ASTTest(phase = CANONICALIZATION,
+                     value = {
+                         assert node.constructors.size() == 2
+                         node.constructors.each {
+                             assert (it.typeDescriptor == 'void <init>(java.lang.String)'
&& it.modifiers == Modifier.PROTECTED) ||
+                             (it.typeDescriptor == 'void <init>(int)' && it.modifiers
== Modifier.PRIVATE)
+                         }
+                     })
+            class Person {
+                String first, last
+                int age
+                static test() {
+                    assert new Person('John').first == 'John'
+                    assert new Person(42).age == 42
+                }
+            }
+            Person.test()
+        '''
+    }
+
     // GROOVY-8455, GROOVY-8453
     void testPropPsuedoPropAndFieldOrderIncludingInheritedMembers() {
         assertScript '''


Mime
View raw message