Copilot commented on code in PR #2424:
URL: https://github.com/apache/groovy/pull/2424#discussion_r3048327127
##########
src/main/java/org/codehaus/groovy/runtime/FormatHelper.java:
##########
@@ -61,8 +59,25 @@ private FormatHelper() {}
private static final int ITEM_ALLOCATE_SIZE = 5;
public static final MetaClassRegistry metaRegistry =
GroovySystem.getMetaClassRegistry();
- private static final String XMLUTIL_CLASS_FULL_NAME = "groovy.xml.XmlUtil";
- private static final String SERIALIZE_METHOD_NAME = "serialize";
+ private static final String GROOVY_TO_STRING = "groovyToString";
+ private static final Class[] EMPTY_TYPES = {};
+
+ /**
+ * Attempts to invoke a {@code groovyToString()} extension method on the
given object
+ * via the metaclass. Returns the result if found, or {@code null} if no
such method exists.
+ */
+ static String tryGroovyToString(Object object) {
+ if (object == null) return null;
+ try {
+ MetaMethod method =
InvokerHelper.getMetaClass(object).getMetaMethod(GROOVY_TO_STRING, EMPTY_TYPES);
+ if (method != null) {
+ return (String) method.invoke(object, EMPTY_ARGS);
+ }
+ } catch (ClassCastException | GroovyRuntimeException ignore) {
+ // method found but not applicable to this object's actual type
+ }
Review Comment:
`tryGroovyToString` currently catches `GroovyRuntimeException` and returns
`null`, which will silently fall back to `toString()` even when `safe` is
false. This can hide real failures from user-provided `groovyToString()`
implementations and makes the `safe` flag ineffective for this code path.
Consider only swallowing exceptions when `safe` is enabled (e.g., pass `safe`
into `tryGroovyToString`) or rethrow runtime exceptions so unsafe formatting
preserves the current fail-fast behavior.
##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -8144,6 +8144,101 @@ public static <G, K, V> Map<G, List<Map.Entry<K, V>>>
groupEntriesBy(Map<K, V> s
return answer;
}
+
//--------------------------------------------------------------------------
+ // groovyToString
+
+ /**
+ * Returns Groovy's default string representation for a Map.
+ * This is used by Groovy's formatting infrastructure (e.g., GString
interpolation,
+ * {@code println}, assert messages). By default, it delegates to
+ * {@link FormatHelper#toMapString(Map)}.
+ * <p>
+ * <pre class="groovyTestCase">
+ * assert [a:1, b:2].groovyToString() == '[a:1, b:2]'
+ * </pre>
+ * <p>
+ * If disabled, e.g. via {@code
-Dgroovy.extension.disable=groovyToString(Map)},
+ * the normal map {@code toString()} is used instead.
+ * Alternatively, you have the option to provide a replacement extension
method in an extension module
+ * to customize how maps are displayed throughout Groovy.
+ *
+ * @param self the Map to format
+ * @return the string representation
+ * @since 6.0.0
+ */
+ public static String groovyToString(Map self) {
+ return FormatHelper.toMapString(self);
+ }
+
+ /**
+ * Returns Groovy's default string representation for a Range.
+ * By default, it delegates to {@link Range#toString()},
+ * producing the compact {@code from..to} notation.
+ * <p>
+ * <pre class="groovyTestCase">
+ * assert (1..4).groovyToString() == '1..4'
+ * </pre>
+ * <p>
+ * This method exists to stop the {@code groovyToString(Collection)}
variant
+ * from overriding the built-in {@code Range#toString}.
+ * Since a range is a list, you can use {@code range.toListString()}
+ * to print it using normal list formatting.
+ *
+ * @param self the Range to format
+ * @return the string representation
+ * @since 6.0.0
+ */
+ public static String groovyToString(Range self) {
+ return self.toString();
+ }
+
+ /**
+ * Returns Groovy's default string representation for a Collection.
+ * This is used by Groovy's formatting infrastructure (e.g., GString
interpolation,
+ * {@code println}, assert messages). By default, it delegates to
+ * {@link FormatHelper#toListString(Collection)}.
+ * <p>
+ * <pre class="groovyTestCase">
+ * assert [1, 2, 3].groovyToString() == '[1, 2, 3]'
+ * </pre>
+ * <p>
+ * If disabled, e.g. via {@code
-Dgroovy.extension.disable=groovyToString(Collection)},
+ * the normal list {@code toString()} is used instead.
+ * Alternatively, you have the option to provide a replacement extension
method in an extension module
+ * to customize how collections are displayed throughout Groovy.
+ *
+ * @param self the Collection to format
+ * @return the string representation
+ * @since 6.0.0
+ */
+ public static String groovyToString(Collection self) {
+ return FormatHelper.toListString(self);
+ }
+
+ /**
+ * Returns Groovy's default string representation for an XML Element.
+ * This is used by Groovy's formatting infrastructure. By default, it
+ * serializes the element using {@code
groovy.xml.XmlUtil.serialize(Element)}.
+ * <p>
+ * If disabled, e.g. via {@code
-Dgroovy.extension.disable=groovyToString(Element)},
+ * the element's default {@code toString()} is used instead.
+ * Alternatively, you have the option to provide a replacement extension
method in an extension module
+ * to customize how elements are displayed throughout Groovy.
+ *
+ * @param self the Element to format
+ * @return the serialized XML string
+ * @since 6.0.0
+ */
+ public static String groovyToString(org.w3c.dom.Element self) {
+ try {
+ java.lang.reflect.Method serialize =
Class.forName("groovy.xml.XmlUtil")
+ .getMethod("serialize", org.w3c.dom.Element.class);
+ return (String) serialize.invoke(null, self);
+ } catch (Exception e) {
+ return self.toString();
+ }
Review Comment:
New `groovyToString(...)` extension methods are introduced as part of
Groovy’s core formatting hook, but there don’t appear to be corresponding tests
asserting (1) that `FormatHelper.format`/interpolation/println pick these
methods up for Map/Collection/Range/Element, and (2) that
`-Dgroovy.extension.disable=groovyToString` (and receiver-specific variants)
reliably falls back to the JDK `toString()` behavior. Adding focused tests
would help prevent regressions in this new formatting contract.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
