[
https://issues.apache.org/jira/browse/GROOVY-7563?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14727716#comment-14727716
]
Thibault Kruse commented on GROOVY-7563:
----------------------------------------
Okay, there are 4 distinct changes that I suggest to make. We can discuss for
each whether they need to be provided with a new API, or can replace existing
behavior without unduly breaking existing code.
h2. 1. toString(Range x)
The current state is this:
{code}
InvokerHelper.format(1..4, false) == 1..4
InvokerHelper.format([1..4], false) == [1..4]
InvokerHelper.toString(1..4) == [1, 2, 3, 4] // surprise
InvokerHelper.toString([1..4]) == [1..4]
{code}
So ranges get expanded to lists in printing, unless they are contained in any
other Structure (Collection, array, Map).
That is because ranges get special treatment in format, but not in toString(),
and toString uses format for recursions.
Possible alternatives for groovy-2.5+:
- Always print ranges in List-form in toString()
- Always print ranges in range-form in toString.
- Keep current inconsistent behavior.
Any change here can impact users of Groovy who rely on the current inconsistent
behavior, so I don't know how to handle this.
h2. Provide safe printing for maps and arrays as well
The current state is this:
{code}
class ExceptionOnToString {String toString(){ throw new
UnsupportedOperationException()}}
def eInst = new ExceptionOnToString()
InvokerHelper.toListString([eInst], -1, true) ==
'[<ExceptionOnToString@a1b2c3d4>]'
InvokerHelper.toListString([[eInst]], -1, true) == '[<ArrayList@a1b2c3d4>]' //
unclear exactly what Object threw exception
InvokerHelper.toListString([[[[[eInst, 1, 2, 3]]]]], -1, true) ==
'[<ArrayList@a1b2c3d4>]' // same as preceding line
InvokerHelper.toListString([[x: eInst]] -1, true) ==
'[<LinkedHashMap@a1b2c3d4>]'
InvokerHelper.toListString([[x: [y: [z: eInst, a: 1, b: 2]]]] -1, true) ==
'[<LinkedHashMap@a1b2c3d4>]' // same as preceding line
// no similar api with 'safe' argument for arrays or maps
{code}
So toListString has an argument safe that makes the method catch exceptions
(though not for nested structures), but that feature does not exist for the
other data structures. As a result, the provided information is poor in some
cases, and there just is not the same useful feature for printing other things
than lists.
Possible alternatives for groovy-2.5+:
- Keep current inconsistent behavior.
- remove (/ deprecate+later remove) method with safe flag
- Provide Safe variant non-recursive methods for other collections
- Provide recursively safe method for nested structures
I believe no client code can justifiedly rely on the fact that passing safe
will *NOT* be safe unless for arguments directly (not nested) contained in a
collection. So I would say providing safe alternatives that are recursively
safe would be okay for Groovy2.5+
One problem is whether to make that change to the format method, thus changing
the output of power-asserts in cases when toString() throws Exceptions, or
whether to provide an alternative safe format method that does not impact
powerasserts in any way. Note though that in GROOVY-7569 I suggest changing the
output of powerasserts in a much more drastic way anyway, so if that were to be
accepted, it would seem strange to refuse changes in this case.
h2. Map: key-self detection
The current state is this:
{code}
Map m = [:]
m.put('x', m)
InvokerHelper.toMapString(m) == '[x: (this Map)]'
Map m2 = [:]
m2.put(m2, 'x')
InvokerHelper.toMapString(m2) // StackOverflowError
{code}
Again I would argue that no client code can justifiedly rely on a
StackOverFlowError being thrown, so changing this to not throw it cannot impact
users in an unjustified way.
h2. Provide safe printing for Ranges as well
When the objects defining the range borders throw an exception in toString(),
the question is whether to handle that like for Collections. This is such a
rare and exotic case that I would think it does not matter either way. Since
implementing safety here is simple (if a safe format method exists), I would
suggest to allow it being safe.
I changed the PR to have 4 commits to closely match the 4 suggested features
above.
> InvokerHelper formatting methods have inconsistent API
> ------------------------------------------------------
>
> Key: GROOVY-7563
> URL: https://issues.apache.org/jira/browse/GROOVY-7563
> Project: Groovy
> Issue Type: Improvement
> Components: groovy-jdk
> Reporter: Thibault Kruse
> Assignee: Guillaume Laforge
> Priority: Minor
>
> in class org.codehaus.groovy.runtime.InvokerHelper, there are methods used to
> print out Objects. The methods sometimes have a maxsize, verbose or safe
> argument, but they do not have them consistently. I suggest changing the
> private API so that methods for all Collection/Map types have the same API
> and same functionality.
> See https://github.com/apache/incubator-groovy/pull/96
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
