You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/07/24 11:37:35 UTC
[groovy] branch master updated: GROOVY-9648: Bad error message when
attempting to call a missing constructor with named args
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new b8c62d2 GROOVY-9648: Bad error message when attempting to call a missing constructor with named args
b8c62d2 is described below
commit b8c62d2f8acb3ae89e4c5044fd17341da9ee879b
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Jul 24 21:25:32 2020 +1000
GROOVY-9648: Bad error message when attempting to call a missing constructor with named args
---
src/main/java/groovy/lang/MetaClassImpl.java | 93 +++++++++++++---------
.../groovy/reflection/CachedConstructor.java | 5 ++
src/test/groovy/ConstructorMismatchTest.groovy | 65 +++++++++++++++
3 files changed, 125 insertions(+), 38 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 4313bb0..340941c 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1756,19 +1756,18 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
public MetaMethod retrieveConstructor(Object[] arguments) {
checkInitalised();
if (arguments == null) arguments = EMPTY_ARGUMENTS;
- Class[] argClasses = MetaClassHelper.convertToTypeArray(arguments);
+ Class[] argTypes = MetaClassHelper.convertToTypeArray(arguments);
MetaClassHelper.unwrap(arguments);
- Object res = chooseMethod("<init>", constructors, argClasses);
+ Object res = chooseMethod("<init>", constructors, argTypes);
if (res instanceof MetaMethod) return (MetaMethod) res;
CachedConstructor constructor = (CachedConstructor) res;
if (constructor != null) return new MetaConstructor(constructor, false);
- if (arguments.length == 1 && arguments[0] instanceof Map) {
- res = chooseMethod("<init>", constructors, MetaClassHelper.EMPTY_TYPE_ARRAY);
- } else if (
- arguments.length == 2 && arguments[1] instanceof Map &&
+ // handle named args on class or inner class (one level only for now)
+ if ((arguments.length == 1 && arguments[0] instanceof Map) ||
+ (arguments.length == 2 && arguments[1] instanceof Map &&
theClass.getEnclosingClass() != null &&
- theClass.getEnclosingClass().isAssignableFrom(argClasses[0])) {
- res = chooseMethod("<init>", constructors, new Class[]{argClasses[0]});
+ theClass.getEnclosingClass().isAssignableFrom(argTypes[0]))) {
+ res = retrieveNamedArgCompatibleConstructor(argTypes, arguments);
}
if (res instanceof MetaMethod) return (MetaMethod) res;
constructor = (CachedConstructor) res;
@@ -1777,27 +1776,50 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return null;
}
+ private Object retrieveNamedArgCompatibleConstructor(Class[] origArgTypes, Object[] origArgs) {
+ // if we get here Map variant already not found so allow for no-arg plus setters
+ Class[] argTypes = Arrays.copyOf(origArgTypes, origArgTypes.length - 1);
+ Object[] args = Arrays.copyOf(origArgs, origArgs.length - 1);
+ Object res = chooseMethod("<init>", constructors, argTypes);
+ // chooseMethod allows fuzzy matching implicit null case but we don't want that here
+ // code here handles inner class case but we currently don't do fuzzy matching for inner classes
+ if (res instanceof ParameterTypes && ((ParameterTypes) res).getParameterTypes().length == origArgTypes.length) {
+ String prettyOrigArgs = InvokerHelper.toTypeString(origArgs);
+ if (prettyOrigArgs.endsWith("LinkedHashMap")) {
+ prettyOrigArgs = prettyOrigArgs.replaceFirst("LinkedHashMap$", "Map");
+ }
+ throw new GroovyRuntimeException(
+ "Could not find named-arg compatible constructor. Expecting one of:\n"
+ + theClass.getName() + "(" + prettyOrigArgs + ")\n"
+ + theClass.getName() + "(" + InvokerHelper.toTypeString(args) + ")"
+ );
+ }
+ return res;
+ }
+
private Object invokeConstructor(Class at, Object[] arguments) {
checkInitalised();
if (arguments == null) arguments = EMPTY_ARGUMENTS;
- Class[] argClasses = MetaClassHelper.convertToTypeArray(arguments);
+ Class[] argTypes = MetaClassHelper.convertToTypeArray(arguments);
MetaClassHelper.unwrap(arguments);
- CachedConstructor constructor = (CachedConstructor) chooseMethod("<init>", constructors, argClasses);
+ CachedConstructor constructor = (CachedConstructor) chooseMethod("<init>", constructors, argTypes);
if (constructor != null) {
return constructor.doConstructorInvoke(arguments);
}
-
- if (arguments.length == 1) {
- Object firstArgument = arguments[0];
- if (firstArgument instanceof Map) {
- constructor = (CachedConstructor) chooseMethod("<init>", constructors, MetaClassHelper.EMPTY_TYPE_ARRAY);
- if (constructor != null) {
- Object bean = constructor.doConstructorInvoke(MetaClassHelper.EMPTY_ARRAY);
- setProperties(bean, ((Map) firstArgument));
- return bean;
- }
+ // handle named args on class or inner class (one level only for now)
+ if ((arguments.length == 1 && arguments[0] instanceof Map) ||
+ (arguments.length == 2 && arguments[1] instanceof Map &&
+ theClass.getEnclosingClass() != null &&
+ theClass.getEnclosingClass().isAssignableFrom(argTypes[0]))) {
+ constructor = (CachedConstructor) retrieveNamedArgCompatibleConstructor(argTypes, arguments);
+ if (constructor != null) {
+ Object[] args = Arrays.copyOf(arguments, arguments.length - 1);
+ Object bean = constructor.doConstructorInvoke(args);
+ setProperties(bean, ((Map) arguments[arguments.length - 1]));
+ return bean;
}
}
+
throw new GroovyRuntimeException(
"Could not find matching constructor for: "
+ theClass.getName()
@@ -3549,25 +3571,20 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
*/
public CallSite createConstructorSite(CallSite site, Object[] args) {
if (!(this instanceof AdaptingMetaClass)) {
- Class[] params = MetaClassHelper.convertToTypeArray(args);
- CachedConstructor constructor = (CachedConstructor) chooseMethod("<init>", constructors, params);
+ Class[] argTypes = MetaClassHelper.convertToTypeArray(args);
+ CachedConstructor constructor = (CachedConstructor) chooseMethod("<init>", constructors, argTypes);
if (constructor != null) {
- return ConstructorSite.createConstructorSite(site, this, constructor, params, args);
- } else {
- if (args.length == 1 && args[0] instanceof Map) {
- constructor = (CachedConstructor) chooseMethod("<init>", constructors, MetaClassHelper.EMPTY_TYPE_ARRAY);
- if (constructor != null) {
- return new ConstructorSite.NoParamSite(site, this, constructor, params);
- }
- } else if (args.length == 2 && theClass.getEnclosingClass() != null && args[1] instanceof Map) {
- Class enclosingClass = theClass.getEnclosingClass();
- String enclosingInstanceParamType = args[0] != null ? args[0].getClass().getName() : "";
- if (enclosingClass.getName().equals(enclosingInstanceParamType)) {
- constructor = (CachedConstructor) chooseMethod("<init>", constructors, new Class[]{enclosingClass});
- if (constructor != null) {
- return new ConstructorSite.NoParamSiteInnerClass(site, this, constructor, params);
- }
- }
+ return ConstructorSite.createConstructorSite(site, this, constructor, argTypes, args);
+ }
+ if ((args.length == 1 && args[0] instanceof Map) ||
+ (args.length == 2 && args[1] instanceof Map &&
+ theClass.getEnclosingClass() != null &&
+ theClass.getEnclosingClass().isAssignableFrom(argTypes[0]))) {
+ constructor = (CachedConstructor) retrieveNamedArgCompatibleConstructor(argTypes, args);
+ if (constructor != null) {
+ return args.length == 1
+ ? new ConstructorSite.NoParamSite(site, this, constructor, argTypes)
+ : new ConstructorSite.NoParamSiteInnerClass(site, this, constructor, argTypes);
}
}
}
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedConstructor.java b/src/main/java/org/codehaus/groovy/reflection/CachedConstructor.java
index 347b9b0..cf7921b 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedConstructor.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedConstructor.java
@@ -95,6 +95,11 @@ public class CachedConstructor extends ParameterTypes {
setReason ? e : null);
}
+ @Override
+ public String toString() {
+ return cachedConstructor.toString();
+ }
+
public int getModifiers () {
return cachedConstructor.getModifiers();
}
diff --git a/src/test/groovy/ConstructorMismatchTest.groovy b/src/test/groovy/ConstructorMismatchTest.groovy
new file mode 100644
index 0000000..23b9485
--- /dev/null
+++ b/src/test/groovy/ConstructorMismatchTest.groovy
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy
+
+import groovy.test.GroovyTestCase
+
+class ConstructorMismatchTest extends GroovyTestCase {
+ void testConstructorMismatch() {
+ def message = shouldFail(GroovyRuntimeException, '''
+ class Event {
+ Date when
+ Event(Date when) {
+ this.when = when
+ }
+ }
+ new Event(42G)
+ ''')
+ assert message == 'Could not find matching constructor for: Event(BigInteger)'
+ }
+
+ void testConstructorMismatchNamedArgs() {
+ def message = shouldFail(GroovyRuntimeException, '''
+ class Event {
+ Date when
+ Event(Date when) {
+ this.when = when
+ }
+ }
+ new Event(when: new Date())
+ ''')
+ assert message == 'Could not find named-arg compatible constructor. Expecting one of:\nEvent(Map)\nEvent()'
+ }
+
+ void testMapConstructorThroughEMC() {
+ assertScript'''
+ class Person {
+ String name
+ Person(String name) {
+ this.name = name
+ }
+ }
+
+ Person.metaClass.constructor << { Map m -> new Person(m.name) }
+
+ def j = new Person(name: 'John')
+ assert j.name == 'John'
+ '''
+ }
+}