[1/2] groovy git commit: GROOVY-8467: IOGroovyMethods readLine(Reader) method works in unexpected way

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[1/2] groovy git commit: GROOVY-8467: IOGroovyMethods readLine(Reader) method works in unexpected way

paulk
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X 7cfd426d9 -> e30e86cb9


GROOVY-8467: IOGroovyMethods readLine(Reader) method works in unexpected way


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

Branch: refs/heads/GROOVY_2_5_X
Commit: e30e86cb927ac0e742d1624c8bd076ac2b9102a8
Parents: 16b532f
Author: paulk <[hidden email]>
Authored: Thu Feb 8 22:57:28 2018 +1000
Committer: paulk <[hidden email]>
Committed: Thu Feb 8 23:03:46 2018 +1000

----------------------------------------------------------------------
 .../java/org/codehaus/groovy/runtime/IOGroovyMethods.java | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/e30e86cb/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java
index 5fd2ef1..f65c85f 100644
--- a/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java
@@ -636,11 +636,19 @@ public class IOGroovyMethods extends DefaultGroovyMethodsSupport {
     }
 
     /**
-     * Read a single, whole line from the given Reader.
+     * Read a single, whole line from the given Reader. This method is designed for use with
+     * Readers that support the {@code mark()} operation like BufferReader. It has a fallback
+     * behavior for Readers that don't support mark() but the behavior doesn't correctly
+     * detect multi-character line termination (e.g. carriage return followed by linefeed).
+     * We recommend for Readers that don't support mark() you consider using one of the
+     * following methods instead: eachLine, readLines, or iterator.
      *
      * @param self a Reader
      * @return a line
      * @throws IOException if an IOException occurs.
+     * @see #readLines(java.io.Reader)
+     * @see #iterator(java.io.Reader)
+     * @see #eachLine(java.io.Reader, groovy.lang.Closure)
      * @since 1.0
      */
     public static String readLine(Reader self) throws IOException {

Reply | Threaded
Open this post in threaded view
|

[2/2] groovy git commit: GROOVY-8416: Map-based constructor generated by @Immutable works only with HashMap (plus a bit of refactoring and clarification in doco)

paulk
GROOVY-8416: Map-based constructor generated by @Immutable works only with HashMap (plus a bit of refactoring and clarification in doco)


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

Branch: refs/heads/GROOVY_2_5_X
Commit: 16b532fbe80c5d46bacc50db3ceec504ed6974a1
Parents: 7cfd426
Author: paulk <[hidden email]>
Authored: Thu Feb 8 15:24:44 2018 +1000
Committer: paulk <[hidden email]>
Committed: Thu Feb 8 23:03:46 2018 +1000

----------------------------------------------------------------------
 .../groovy/groovy/transform/MapConstructor.java | 11 ++++++
 .../groovy/transform/TupleConstructor.java      | 21 ++++++++--
 .../groovy/classgen/EnumCompletionVisitor.java  |  2 +-
 .../transform/ImmutableASTTransformation.java   | 40 +++++++++++++-------
 .../MapConstructorASTTransformation.java        | 18 +++++----
 .../TupleConstructorASTTransformation.java      | 33 ++++++----------
 .../test/builder/ObjectGraphBuilderTest.groovy  |  2 +-
 .../transform/ImmutableTransformTest.groovy     | 20 ++++++++++
 .../swingui/AstNodeToScriptAdapterTest.groovy   |  2 +-
 9 files changed, 100 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 eb1e069..e59ce75 100644
--- a/src/main/groovy/groovy/transform/MapConstructor.java
+++ b/src/main/groovy/groovy/transform/MapConstructor.java
@@ -66,6 +66,17 @@ import java.lang.annotation.Target;
  *     first = first?.toUpperCase()
  * }
  * </pre>
+ * <p>
+ * Known limitations/special cases:
+ * <ul>
+ * <li>
+ * The generated map constructor will have an argument of type {@code Map} unless a single property (or field)
+ * is included and the type of that property (or field) is Object, AbstractMap, Map or HashMap.
+ * In this case, the generated constructor will be of type {@code LinkedHashMap}.
+ * This allows the possibility of also adding a tuple constructor without conflict, although
+ * no such constructor is added automatically.
+ * </li>
+ * </ul>
  *
  * @since 2.5.0
  */

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 f330fdd..e611cb5 100644
--- a/src/main/groovy/groovy/transform/TupleConstructor.java
+++ b/src/main/groovy/groovy/transform/TupleConstructor.java
@@ -158,7 +158,24 @@ import java.lang.annotation.Target;
  * assert student.courses == ['IT']
  * </pre>
  * <p>
- * Known Limitations:
+ * Named-argument support:
+ * <ul>
+ * <li>Groovy supports named-arguments for classes with a no-arg constructor or a constructor
+ * with a Map as the first argument. This is compatible with the default kind of constructor(s)
+ * that {@code @TupleConstructor} produces.</li>
+ * <li>If the {@code defaults} annotation attribute is set to {@code false},
+ * and no other map-based constructor are added then named-argument processing will not be available.</li>
+ * <li>If there is more than one included property (and/or field) and the first property (or field) has type
+ * Object, AbstractMap, Map or HashMap, then a special {@code LinkedHashMap} constructor will be created
+ * in addition to the tuple constructor to support named parameters in the normal way. This won't be created
+ * if the class is already annotated with {@code @MapConstructor} or if the {@code defaults}
+ * annotation attribute is set to {@code false}.</li>
+ * <li>If the first property (or field) has type {@code LinkedHashMap} or if there is
+ * a single Object, AbstractMap, Map or HashMap property (or field), then no additional constructor
+ * will be added and Groovy's normal map-style naming conventions will not be available.</li>
+ * </ul>
+ * <p>
+ * Known limitations/special cases:
  * <ul>
  * <li>This AST transform might become a no-op if you are defining your own constructors or
  * combining with other AST transforms which create constructors (e.g. {@code @InheritConstructors});
@@ -169,8 +186,6 @@ import java.lang.annotation.Target;
  * combining with other AST transforms which create constructors (e.g. {@code @InheritConstructors});
  * the order in which the particular transforms are processed becomes important in that case.
  * See the {@code defaults} attribute for further details about customizing this behavior.</li>
- * <li>Groovy's normal map-style naming conventions will not be available if the first property (or field)
- * has type {@code LinkedHashMap} or if there is a single Map, AbstractMap or HashMap property (or field)</li>
  * </ul>
  *
  * @since 1.8.0

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 7cc5375..5d286c7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumCompletionVisitor.java
@@ -144,7 +144,7 @@ public class EnumCompletionVisitor extends ClassCodeVisitorSupport {
     }
 
     private static void addMapConstructors(ClassNode enumClass) {
-        TupleConstructorASTTransformation.addMapConstructors(enumClass, false, "One of the enum constants for enum " + enumClass.getName() +
+        TupleConstructorASTTransformation.addSpecialMapConstructors(enumClass, true, "One of the enum constants for enum " + enumClass.getName() +
                 " was initialized with null. Please use a non-null value or define your own constructor.");
     }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 3e8fd8f..5fb6874 100644
--- a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
@@ -183,7 +183,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
     private static final ClassNode READONLYEXCEPTION_TYPE = make(ReadOnlyPropertyException.class);
     private static final ClassNode DGM_TYPE = make(DefaultGroovyMethods.class);
     private static final ClassNode SELF_TYPE = make(ImmutableASTTransformation.class);
-    private static final ClassNode HASHMAP_TYPE = makeWithoutCaching(HashMap.class, false);
+    private static final ClassNode HMAP_TYPE = makeWithoutCaching(HashMap.class, false);
     private static final ClassNode MAP_TYPE = makeWithoutCaching(Map.class, false);
     private static final ClassNode REFLECTION_INVOKER_TYPE = make(ReflectionMethodInvoker.class);
     private static final ClassNode SORTEDSET_CLASSNODE = make(SortedSet.class);
@@ -225,6 +225,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
             ensureNotPublic(this, cName, fNode);
         }
         if (hasAnnotation(cNode, TupleConstructorASTTransformation.MY_TYPE)) {
+            // TODO make this a method to be called from TupleConstructor xform, add check for 'defaults'?
             AnnotationNode tupleCons = cNode.getAnnotations(TupleConstructorASTTransformation.MY_TYPE).get(0);
             if (unsupportedTupleAttribute(tupleCons, "excludes")) return;
             if (unsupportedTupleAttribute(tupleCons, "includes")) return;
@@ -337,8 +338,21 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
         }
     }
 
-    static boolean isSpecialHashMapCase(List<PropertyNode> list) {
-        return list.size() == 1 && list.get(0).getField().getType().equals(HASHMAP_TYPE);
+    static boolean isSpecialNamedArgCase(List<PropertyNode> list, boolean checkSize) {
+        if (checkSize && list.size() != 1) return false;
+        if (list.size() == 0) return false;
+        ClassNode firstParamType = list.get(0).getField().getType();
+        if (firstParamType.equals(ClassHelper.MAP_TYPE)) {
+            return true;
+        }
+        ClassNode candidate = HMAP_TYPE;
+        while (candidate != null) {
+            if (candidate.equals(firstParamType)) {
+                return true;
+            }
+            candidate = candidate.getSuperClass();
+        }
+        return false;
     }
 
     @Deprecated
@@ -367,7 +381,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
             argMap.addMapEntryExpression(constX(pNode.getName()), varX(pNode.getName()));
         }
         final BlockStatement orderedBody = new BlockStatement();
-        orderedBody.addStatement(stmt(ctorX(ClassNode.THIS, args(castX(HASHMAP_TYPE, argMap)))));
+        orderedBody.addStatement(stmt(ctorX(ClassNode.THIS, args(castX(HMAP_TYPE, argMap)))));
         doAddConstructor(cNode, new ConstructorNode(ACC_PUBLIC, orderedParams, ClassNode.EMPTY_ARRAY, orderedBody));
     }
 
@@ -410,7 +424,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
         return castX(type, smce);
     }
 
-    static void createConstructorMapCommon(ClassNode cNode, BlockStatement body) {
+    static void createConstructorMapCommon(ClassNode cNode, BlockStatement body, Parameter[] params) {
         final List<FieldNode> fList = cNode.getFields();
         for (FieldNode fNode : fList) {
             if (fNode.isPublic()) continue; // public fields will be rejected elsewhere
@@ -421,7 +435,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
                 body.addStatement(checkFinalArgNotOverridden(cNode, fNode));
             body.addStatement(createConstructorStatementDefault(fNode, true));
         }
-        doAddConstructor(cNode, new ConstructorNode(ACC_PUBLIC, params(new Parameter(HASHMAP_TYPE, "args")), ClassNode.EMPTY_ARRAY, body));
+        doAddConstructor(cNode, new ConstructorNode(ACC_PUBLIC, params, ClassNode.EMPTY_ARRAY, body));
     }
 
     private static Statement checkFinalArgNotOverridden(ClassNode cNode, FieldNode fNode) {
@@ -682,7 +696,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
                 new VariableScope(),
                 ifElseS(
                         callX(
-                                varX("map", HASHMAP_TYPE),
+                                varX("map", HMAP_TYPE),
                                 "containsKey",
                                 args(constX(pNode.getName()))
                         ),
@@ -691,7 +705,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
                                 declS(
                                         varX("newValue", ClassHelper.OBJECT_TYPE),
                                         callX(
-                                                varX("map", HASHMAP_TYPE),
+                                                varX("map", HMAP_TYPE),
                                                 "get",
                                                 args(constX(pNode.getName()))
                                         )
@@ -716,7 +730,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
                                         )
                                 ),
                                 stmt(callX(
-                                        varX("construct", HASHMAP_TYPE),
+                                        varX("construct", HMAP_TYPE),
                                         "put",
                                         args(
                                                 constX(pNode.getName()),
@@ -727,7 +741,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
                         block(
                                 new VariableScope(),
                                 stmt(callX(
-                                        varX("construct", HASHMAP_TYPE),
+                                        varX("construct", HMAP_TYPE),
                                         "put",
                                         args(
                                                 constX(pNode.getName()),
@@ -744,12 +758,12 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
         body.addStatement(ifS(
                 orX(
                         equalsNullX(varX("map", ClassHelper.MAP_TYPE)),
-                        eqX(callX(varX("map", HASHMAP_TYPE), "size"), constX(0))
+                        eqX(callX(varX("map", HMAP_TYPE), "size"), constX(0))
                 ),
                 returnS(varX("this", cNode))
         ));
         body.addStatement(declS(varX("dirty", ClassHelper.boolean_TYPE), ConstantExpression.PRIM_FALSE));
-        body.addStatement(declS(varX("construct", HASHMAP_TYPE), ctorX(HASHMAP_TYPE)));
+        body.addStatement(declS(varX("construct", HMAP_TYPE), ctorX(HMAP_TYPE)));
 
         // Check for each property
         for (final PropertyNode pNode : pList) {
@@ -758,7 +772,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation {
 
         body.addStatement(returnS(ternaryX(
                 isTrueX(varX("dirty", ClassHelper.boolean_TYPE)),
-                ctorX(cNode, args(varX("construct", HASHMAP_TYPE))),
+                ctorX(cNode, args(varX("construct", HMAP_TYPE))),
                 varX("this", cNode)
         )));
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 468419f..1293d5b 100644
--- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
@@ -41,6 +41,7 @@ import org.codehaus.groovy.control.SourceUnit;
 
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -79,6 +80,7 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation {
     static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
     private static final ClassNode MAP_TYPE = makeWithoutCaching(Map.class, false);
     private static final ClassNode IMMUTABLE_XFORM_TYPE = make(ImmutableASTTransformation.class);
+    private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false);
 
     public void visit(ASTNode[] nodes, SourceUnit source) {
         init(nodes, source);
@@ -153,9 +155,9 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation {
         }
         final BlockStatement inner = new BlockStatement();
         superList.addAll(list);
-        boolean specialHashMapCase = ImmutableASTTransformation.isSpecialHashMapCase(superList);
-        if (!specialHashMapCase) {
-            processProps(xform, cNode, makeImmutable && !specialHashMapCase, useSetters, allNames, excludes, includes, superList, map, inner);
+        boolean specialNamedArgCase = ImmutableASTTransformation.isSpecialNamedArgCase(superList, true);
+        if (!specialNamedArgCase) {
+            processProps(xform, cNode, makeImmutable, useSetters, allNames, excludes, includes, superList, map, inner);
             body.addStatement(ifS(equalsNullX(varX("args")), assignS(varX("args"), new MapExpression())));
         }
         body.addStatement(inner);
@@ -163,16 +165,16 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation {
             ClosureExpression transformed = (ClosureExpression) transformer.transform(post);
             body.addStatement(transformed.getCode());
         }
-        if (makeImmutable && !specialHashMapCase) {
+        if (makeImmutable && !specialNamedArgCase) {
             body.addStatement(stmt(callX(IMMUTABLE_XFORM_TYPE, "checkPropNames", args("this", "args"))));
-            createConstructorMapCommon(cNode, body);
-        } else if (specialHashMapCase) {
+            createConstructorMapCommon(cNode, body, params(map));
+        } else if (specialNamedArgCase) {
             inner.addStatement(createConstructorStatementMapSpecial(superList.get(0).getField()));
-            createConstructorMapCommon(cNode, body);
+            createConstructorMapCommon(cNode, body, params(new Parameter(LHMAP_TYPE, "args")));
         } else {
             cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, params(map), ClassNode.EMPTY_ARRAY, body));
         }
-        if (noArg && !superList.isEmpty() && !hasNoArgConstructor(cNode) && !specialHashMapCase) {
+        if (noArg && !superList.isEmpty() && !hasNoArgConstructor(cNode) && !specialNamedArgCase) {
             createNoArgConstructor(cNode);
         }
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/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 2b7088a..3ee15a6 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -188,8 +188,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         List<PropertyNode> list = getAllProperties(names, cNode, true, includeFields, false, allProperties, false, true);
 
         boolean makeImmutable = makeImmutable(cNode);
-        boolean specialHashMapCase = (ImmutableASTTransformation.isSpecialHashMapCase(list) && superList.isEmpty()) ||
-                (ImmutableASTTransformation.isSpecialHashMapCase(superList) && list.isEmpty());
+        boolean specialNamedArgCase = (ImmutableASTTransformation.isSpecialNamedArgCase(list, !defaults) && superList.isEmpty()) ||
+                (ImmutableASTTransformation.isSpecialNamedArgCase(superList, !defaults) && list.isEmpty());
 
         // no processing if existing constructors found unless forced or ImmutableBase in play
         if (!cNode.getDeclaredConstructors().isEmpty() && !force && !makeImmutable) return;
@@ -214,7 +214,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
             boolean hasSetter = cNode.getProperty(name) != null && !fNode.isFinal();
             if (callSuper) {
                 superParams.add(varX(name));
-            } else if (!superInPre && !specialHashMapCase) {
+            } else if (!superInPre && !specialNamedArgCase) {
                 if (makeImmutable) {
                     body.addStatement(createConstructorStatement(xform, cNode, pNode, false));
                 } else {
@@ -265,7 +265,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         }
 
         boolean hasMapCons = hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
-        if (!specialHashMapCase || !hasMapCons) {
+        if (!specialNamedArgCase || !hasMapCons) {
             cNode.addConstructor(new ConstructorNode(ACC_PUBLIC, params.toArray(new Parameter[params.size()]), ClassNode.EMPTY_ARRAY, body));
         }
 
@@ -278,22 +278,11 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         // we don't do it for LinkedHashMap for now (would lead to duplicate signature)
         // or if there is only one Map property (for backwards compatibility)
         // or if there is already a @MapConstructor annotation
-        if (!params.isEmpty() && defaults && !hasMapCons) {
-            ClassNode firstParam = params.get(0).getType();
-            if (params.size() > 1 || firstParam.equals(ClassHelper.OBJECT_TYPE)) {
+        if (!params.isEmpty() && defaults && !hasMapCons && specialNamedArgCase) {
+            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.";
-                if (firstParam.equals(ClassHelper.MAP_TYPE)) {
-                    addMapConstructors(cNode, true, message);
-                } else {
-                    ClassNode candidate = HMAP_TYPE;
-                    while (candidate != null) {
-                        if (candidate.equals(firstParam)) {
-                            addMapConstructors(cNode, true, message);
-                            break;
-                        }
-                        candidate = candidate.getSuperClass();
-                    }
-                }
+                addSpecialMapConstructors(cNode, false, message);
             }
         }
     }
@@ -320,7 +309,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         return initialExp;
     }
 
-    public static void addMapConstructors(ClassNode cNode, boolean hasNoArg, String message) {
+    public static void addSpecialMapConstructors(ClassNode cNode, boolean addNoArg, String message) {
         Parameter[] parameters = params(new Parameter(LHMAP_TYPE, "__namedArgs"));
         BlockStatement code = new BlockStatement();
         VariableExpression namedArgs = varX("__namedArgs");
@@ -330,8 +319,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
                 processArgsBlock(cNode, namedArgs)));
         ConstructorNode init = new ConstructorNode(ACC_PUBLIC, parameters, ClassNode.EMPTY_ARRAY, code);
         cNode.addConstructor(init);
-        // add a no-arg constructor too
-        if (!hasNoArg) {
+        // 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);

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/src/spec/test/builder/ObjectGraphBuilderTest.groovy
----------------------------------------------------------------------
diff --git a/src/spec/test/builder/ObjectGraphBuilderTest.groovy b/src/spec/test/builder/ObjectGraphBuilderTest.groovy
index 4f3ccc5..f3e487f 100644
--- a/src/spec/test/builder/ObjectGraphBuilderTest.groovy
+++ b/src/spec/test/builder/ObjectGraphBuilderTest.groovy
@@ -125,7 +125,7 @@ builder.classNameResolver = "com.acme"
 
 // tag::newinstanceresolver[]
 builder.newInstanceResolver = { Class klazz, Map attributes ->
-    if (klazz.getConstructor(HashMap)) {
+    if (klazz.getConstructor(Map)) {
         def o = klazz.newInstance(attributes)
         attributes.clear()
         return o

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
index fb73e88..e457855 100644
--- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
@@ -1055,4 +1055,24 @@ class ImmutableTransformTest extends GroovyShellTestCase {
             assert mmm.toString() == 'Person(Fred, Brooks, 1931-04-19)'
         '''
     }
+
+    // GROOVY-8416
+    @Test
+    void testMapFriendlyNamedArgs() {
+        assertScript '''
+            import groovy.transform.Immutable
+            @Immutable
+            class Point {
+                int x, y
+            }
+            def coordinates = [x: 1, y: 2]
+            assert coordinates instanceof LinkedHashMap
+            def p1 = new Point(coordinates)
+            assert p1.x == 1
+            def p2 = new Point(new HashMap(coordinates))
+            assert p2.x == 1
+            def p3 = new Point(new TreeMap(coordinates))
+            assert p3.x == 1
+        '''
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/16b532fb/subprojects/groovy-console/src/test/groovy/groovy/inspect/swingui/AstNodeToScriptAdapterTest.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-console/src/test/groovy/groovy/inspect/swingui/AstNodeToScriptAdapterTest.groovy b/subprojects/groovy-console/src/test/groovy/groovy/inspect/swingui/AstNodeToScriptAdapterTest.groovy
index a194286..085a553 100644
--- a/subprojects/groovy-console/src/test/groovy/groovy/inspect/swingui/AstNodeToScriptAdapterTest.groovy
+++ b/subprojects/groovy-console/src/test/groovy/groovy/inspect/swingui/AstNodeToScriptAdapterTest.groovy
@@ -542,7 +542,7 @@ class AstNodeToScriptAdapterTest extends GroovyTestCase {
 
         String result = compileToScript(script, CompilePhase.CANONICALIZATION)
         assert result.contains('private int $hash$code')
-        assert result.contains('public Event(java.util.HashMap args)')
+        assert result.contains('public Event(java.util.Map args)')
 
         // assert toString()... quotations marks were a hassle
         assert result.contains('public java.lang.String toString()')