Repository: groovy Updated Branches: refs/heads/master afa59389f -> aad354ab6
GROOVY-7941: MissingMethodException should limit argument types in getMessage() (closes #426) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/aad354ab Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/aad354ab Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/aad354ab Branch: refs/heads/master Commit: aad354ab6ba9c067327e6a8b0a5fea55656a2485 Parents: afa5938 Author: paulk <pa...@asert.com.au> Authored: Mon Sep 19 16:03:18 2016 +1000 Committer: paulk <pa...@asert.com.au> Committed: Thu Sep 22 06:10:41 2016 +1000 ---------------------------------------------------------------------- .../groovy/lang/MissingMethodException.java | 4 +- .../codehaus/groovy/runtime/InvokerHelper.java | 55 +++++++++++++++++--- .../typing/StaticCompilationIntroTest.groovy | 2 +- src/test/groovy/GroovyMethodsTest.groovy | 15 ++---- src/test/groovy/lang/MethodMissingTest.groovy | 7 ++- .../runtime/InvokerHelperFormattingTest.groovy | 2 +- 6 files changed, 57 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/main/groovy/lang/MissingMethodException.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/lang/MissingMethodException.java b/src/main/groovy/lang/MissingMethodException.java index efc45fb..0902150 100644 --- a/src/main/groovy/lang/MissingMethodException.java +++ b/src/main/groovy/lang/MissingMethodException.java @@ -26,8 +26,6 @@ import org.codehaus.groovy.runtime.MethodRankHelper; * <p> * Note that the Missing*Exception classes were named for consistency and * to avoid conflicts with JDK exceptions of the same name. - * - * @author <a href="mailto:ja...@coredevelopers.net">James Strachan</a> */ public class MissingMethodException extends GroovyRuntimeException { @@ -60,7 +58,7 @@ public class MissingMethodException extends GroovyRuntimeException { + "." + method + "() is applicable for argument types: (" - + InvokerHelper.toTypeString(arguments) + + InvokerHelper.toTypeString(arguments, 60) + ") values: " + InvokerHelper.toArrayString(arguments, 60, true) + MethodRankHelper.getMethodSuggestionString(method, type, arguments); http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/main/org/codehaus/groovy/runtime/InvokerHelper.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/runtime/InvokerHelper.java b/src/main/org/codehaus/groovy/runtime/InvokerHelper.java index 41fb360..faa142c 100644 --- a/src/main/org/codehaus/groovy/runtime/InvokerHelper.java +++ b/src/main/org/codehaus/groovy/runtime/InvokerHelper.java @@ -35,6 +35,7 @@ import groovy.lang.SpreadMap; import groovy.lang.SpreadMapEvaluatingException; import groovy.lang.Tuple; import groovy.lang.Writable; +import org.codehaus.groovy.control.ResolveVisitor; import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl; import org.codehaus.groovy.runtime.metaclass.MissingMethodExecutionFailed; import org.codehaus.groovy.runtime.powerassert.PowerAssertionError; @@ -59,10 +60,12 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Enumeration; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -377,7 +380,7 @@ public class InvokerHelper { } return new SpreadMap(values); } - throw new SpreadMapEvaluatingException("Cannot spread the map " + value.getClass().getName() + ", value " + value); + throw new SpreadMapEvaluatingException("Cannot spread the map " + typeName(value) + ", value " + value); } public static List createList(Object[] values) { @@ -676,7 +679,7 @@ public class InvokerHelper { } catch (Exception ignored) { hash = "????"; } - return "<" + item.getClass().getName() + "@" + hash + ">"; + return "<" + typeName(item) + "@" + hash + ">"; } private static String formatMap(Map map, boolean verbose, int maxSize, boolean safe) { @@ -748,19 +751,57 @@ public class InvokerHelper { * @return the string representation of the type */ public static String toTypeString(Object[] arguments) { + return toTypeString(arguments, -1); + } + + /** + * A helper method to format the arguments types as a comma-separated list. + * + * @param arguments the type to process + * @param maxSize stop after approximately this many characters and append '...', -1 means don't stop + * @return the string representation of the type + */ + public static String toTypeString(Object[] arguments, int maxSize) { if (arguments == null) { return "null"; } StringBuilder argBuf = new StringBuilder(); for (int i = 0; i < arguments.length; i++) { - if (i > 0) { - argBuf.append(", "); + if (maxSize != -1 && argBuf.length() > maxSize) { + argBuf.append("..."); + break; + } else { + if (i > 0) { + argBuf.append(", "); + } + argBuf.append(arguments[i] != null ? typeName(arguments[i]) : "null"); } - argBuf.append(arguments[i] != null ? arguments[i].getClass().getName() : "null"); } return argBuf.toString(); } + private static Set<String> DEFAULT_IMPORT_PKGS = new HashSet<String>(); + private static Set<String> DEFAULT_IMPORT_CLASSES = new HashSet<String>(); + static { + for (String pkgName : ResolveVisitor.DEFAULT_IMPORTS) { + DEFAULT_IMPORT_PKGS.add(pkgName.substring(0, pkgName.length() - 1)); + } + DEFAULT_IMPORT_CLASSES.add("java.math.BigDecimal"); + DEFAULT_IMPORT_CLASSES.add("java.math.BigInteger"); + } + /** + * Gets the type name + * + * @param argument the object to find the type for + * @return the type name (slightly pretty format taking into account default imports) + */ + private static String typeName(Object argument) { + Class<?> aClass = argument.getClass(); + String pkgName = aClass.getPackage() == null ? "" : aClass.getPackage().getName(); + boolean useShort = DEFAULT_IMPORT_PKGS.contains(pkgName) || DEFAULT_IMPORT_CLASSES.contains(aClass.getName()); + return useShort ? aClass.getSimpleName() : aClass.getName(); + } + /** * A helper method to return the string representation of a map with bracket boundaries "[" and "]". * @@ -775,7 +816,7 @@ public class InvokerHelper { * A helper method to return the string representation of a map with bracket boundaries "[" and "]". * * @param arg the map to process - * @param maxSize stop after approximately this many characters and append '...' + * @param maxSize stop after approximately this many characters and append '...', -1 means don't stop * @return the string representation of the map */ public static String toMapString(Map arg, int maxSize) { @@ -807,7 +848,7 @@ public class InvokerHelper { * A helper method to return the string representation of a list with bracket boundaries "[" and "]". * * @param arg the collection to process - * @param maxSize stop after approximately this many characters and append '...' + * @param maxSize stop after approximately this many characters and append '...', -1 means don't stop * @param safe whether to use a default object representation for any item in the collection if an exception occurs when generating its toString * @return the string representation of the collection */ http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/spec/test/typing/StaticCompilationIntroTest.groovy ---------------------------------------------------------------------- diff --git a/src/spec/test/typing/StaticCompilationIntroTest.groovy b/src/spec/test/typing/StaticCompilationIntroTest.groovy index 46344b8..34d2643 100644 --- a/src/spec/test/typing/StaticCompilationIntroTest.groovy +++ b/src/spec/test/typing/StaticCompilationIntroTest.groovy @@ -83,7 +83,7 @@ class StaticCompilationIntroTest extends GroovyTestCase { shell.evaluate(TYPESAFE_PROGRAM+RUNTIME_MAGIC+RUN) assert false } catch (MissingMethodException e) { - assert e.message.contains('No signature of method: Computer.compute() is applicable for argument types: (java.util.Date)') + assert e.message.contains('No signature of method: Computer.compute() is applicable for argument types: (Date)') } } http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/test/groovy/GroovyMethodsTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/GroovyMethodsTest.groovy b/src/test/groovy/GroovyMethodsTest.groovy index cd05a95..7f550af 100644 --- a/src/test/groovy/GroovyMethodsTest.groovy +++ b/src/test/groovy/GroovyMethodsTest.groovy @@ -23,17 +23,7 @@ import java.util.concurrent.LinkedBlockingQueue import org.codehaus.groovy.util.StringUtil /** - * Tests the various new Groovy methods - * - * @author <a href="mailto:ja...@coredevelopers.net">James Strachan</a> - * @author Guillaume Laforge - * @author Dierk Koenig - * @author Paul King - * @author Joachim Baumann - * @author Mike Dillon - * @author Tim Yates - * @author Dinko Srkoc - * @author Yu Kobayashi + * Tests various GDK methods */ class GroovyMethodsTest extends GroovyTestCase { @@ -98,7 +88,8 @@ class GroovyMethodsTest extends GroovyTestCase { try { ['one hundred', 200] as Dimension } catch (Exception e) { - assert e.message.contains("java.lang.String") + assert e.message.contains("Could not find matching constructor for:") + assert e.message.contains("String,") } } http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/test/groovy/lang/MethodMissingTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/lang/MethodMissingTest.groovy b/src/test/groovy/lang/MethodMissingTest.groovy index 6473786..c519fc4 100644 --- a/src/test/groovy/lang/MethodMissingTest.groovy +++ b/src/test/groovy/lang/MethodMissingTest.groovy @@ -20,8 +20,7 @@ package groovy.lang /** * Tests for method missing handling in Groovy - - * @author Graeme Rocher + * * @since 1.5 */ class MethodMissingTest extends GroovyTestCase { @@ -106,7 +105,7 @@ class MethodMissingTest extends GroovyTestCase { sw.withPrintWriter { pw -> exception.printStackTrace(pw) assert sw.toString().contains("groovy.lang.MissingMethodException: No signature of method") - assert sw.toString().contains("is applicable for argument types: (groovy.lang.Broken) values: [<groovy.lang.Broken@????>]") + assert sw.toString().contains("is applicable for argument types: (Broken) values: [<Broken@????>]") assert !sw.toString().contains("toString broken") } sw = new StringWriter() @@ -114,7 +113,7 @@ class MethodMissingTest extends GroovyTestCase { sw.withPrintWriter { pw -> exception.printStackTrace(pw) assert sw.toString().contains("groovy.lang.MissingMethodException: No signature of method") - assert sw.toString().contains("is applicable for argument types: (groovy.lang.Broken) values: [<groovy.lang.Broken@9>]") + assert sw.toString().contains("is applicable for argument types: (Broken) values: [<Broken@9>]") assert !sw.toString().contains("toString broken") } } http://git-wip-us.apache.org/repos/asf/groovy/blob/aad354ab/src/test/org/codehaus/groovy/runtime/InvokerHelperFormattingTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/runtime/InvokerHelperFormattingTest.groovy b/src/test/org/codehaus/groovy/runtime/InvokerHelperFormattingTest.groovy index 479588bc..3c327e9 100644 --- a/src/test/org/codehaus/groovy/runtime/InvokerHelperFormattingTest.groovy +++ b/src/test/org/codehaus/groovy/runtime/InvokerHelperFormattingTest.groovy @@ -93,7 +93,7 @@ class InvokerHelperFormattingTest extends GroovyTestCase { InvokerHelper.format(eObject..eObject2) } - assert InvokerHelper.format(eObject..eObject, false, -1, true) == '<groovy.lang.ObjectRange@????>' + assert InvokerHelper.format(eObject..eObject, false, -1, true) == '<ObjectRange@????>' } void testToStringLists() {