[
https://issues.apache.org/jira/browse/GROOVY-11893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071821#comment-18071821
]
ASF GitHub Bot commented on GROOVY-11893:
-----------------------------------------
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.
> Standardise hooks for Groovy pretty printing
> --------------------------------------------
>
> Key: GROOVY-11893
> URL: https://issues.apache.org/jira/browse/GROOVY-11893
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Groovy has special hard-coded handling when printing lists, maps and in some
> cases arrays. There have been proposals for disabling this, e.g. part of:
> https://github.com/apache/groovy/pull/566
> This issue is to look at a more general approach to handling such cases
> including the ability to disable the special handling and fall back to JDK
> default formatting for e.g. maps.
> The idea that will be explored is to have groovyToString extension methods.
> They would encode the current hard-coded formatting behavior. These
> extensions could be disabled using the groovy.extension.disable system
> property to fall back to JDK defaults.
> Note that currently not all paths funnel through a single path that we can
> fix using this mechanism. Printing some objects through inspect does things
> like limit length and escape backslashes etc. Currently, we don't try to
> force those through this mechanism.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
