[GitHub] [groovy] paulk-asert commented on a change in pull request #1018: GROOVY-9244: prevent ambiguous constructor if using @InheritConstructors

[GitBox <gi...@apache.org>]

paulk-asert commented on a change in pull request #1018: GROOVY-9244: prevent ambiguous constructor if using @InheritConstructors
URL: https://github.com/apache/groovy/pull/1018#discussion_r331740635
 
 

 ##########
 File path: src/main/java/org/codehaus/groovy/transform/InheritConstructorsASTTransformation.java
 ##########
 @@ -98,31 +96,27 @@ private void addConstructorUnlessAlreadyExisting(ClassNode classNode, Constructo
         if (isExisting(classNode, params)) return;
         ConstructorNode added = addGeneratedConstructor(classNode, consNode.getModifiers(), params, consNode.getExceptions(), block(ctorSuperS(args(theArgs))));
         if (copyConstructorAnnotations) {
-            added.addAnnotations(copyAnnotatedNodeAnnotations(consNode, MY_TYPE_NAME));
+            added.addAnnotations(copyAnnotatedNodeAnnotations(consNode, ANNOTATION));
         }
     }
 
     private List<Expression> buildParams(Parameter[] origParams, Parameter[] params, Map<String, ClassNode> genericsSpec, boolean copyParameterAnnotations) {
-        List<Expression> theArgs = new ArrayList<Expression>();
-        for (int i = 0; i < origParams.length; i++) {
+        List<Expression> theArgs = new ArrayList<>();
+        for (int i = 0, n = origParams.length; i < n; i += 1) {
             Parameter p = origParams[i];
             ClassNode newType = correctToGenericsSpecRecurse(genericsSpec, p.getType());
             params[i] = p.hasInitialExpression() ? param(newType, p.getName(), p.getInitialExpression()) : param(newType, p.getName());
             if (copyParameterAnnotations) {
-                params[i].addAnnotations(copyAnnotatedNodeAnnotations(origParams[i], MY_TYPE_NAME));
+                params[i].addAnnotations(copyAnnotatedNodeAnnotations(origParams[i], ANNOTATION));
             }
-            theArgs.add(varX(p.getName(), newType));
+            // cast argument to parameter type in case the value is null
+            theArgs.add(castX(p.getType(), varX(p.getName(), newType)));
         }
         return theArgs;
     }
 
     private static boolean isExisting(ClassNode classNode, Parameter[] params) {
-        for (ConstructorNode consNode : classNode.getDeclaredConstructors()) {
-            if (matchingTypes(params, consNode.getParameters())) {
-                return true;
-            }
-        }
-        return false;
+        return classNode.getDeclaredConstructors().stream().anyMatch(ctor -> matchingTypes(params, ctor.getParameters()));
 
 Review comment:
   You are probably already aware but just in case ... this is great for master/GROOVY_3_0_X but not for earlier branches. To make applying patches/PRs easier across branches we often even leave older style code in newer branches - we do hope to be focusing on Groovy 3/4 soon, so not a big issue here.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services