Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 1.6-rc-1
-
Fix Version/s: 1.6-rc-2
-
Component/s: groovy-jdk
-
Labels:None
-
Number of attachments :1
Description
The following test case fails with Groovy version 1.5.6.
NEWLINE = System.getProperty("line.separator") void doTest(def param) { try { StringWriter sw1 = new StringWriter() StringWriter sw2 = new StringWriter() StringWriter sw3 = new StringWriter() sw1.write(String.valueOf(param)) sw2.print(String.valueOf(param)) new PrintWriter(sw3).print(param) assert sw1.toString() == sw2.toString() assert sw1.toString() == sw3.toString() sw1 = new StringWriter() sw2 = new StringWriter() sw3 = new StringWriter() sw1.write(String.valueOf(param)) sw1.write(NEWLINE) sw2.println(param) new PrintWriter(sw3).println(param) assert sw1.toString() == sw2.toString() assert sw1.toString() == sw3.toString() } catch (AssertionError e) { System.err.println(param) throw e } } doTest(null) doTest("foo") doTest(true) doTest(false) doTest((byte)123) doTest((short)1234) doTest(new Integer(1234)) doTest(new Long(9999999999)) doTest(new Float(1234.5678)) doTest(new Double(1234.5678)) doTest(new BigInteger("123456789012345678901234567890")) doTest(new BigDecimal("12345678901234567890.1234567890123456789")) doTest(new Date()) doTest(new StringBuffer("bar")) doTest([null, "foo", true, false, new Integer(1234)]) doTest(["foo" : "bar", "true": true, "int": new Integer(1234)])
-
- Groovy3237Bug.groovy
- 25/Dec/08 4:00 PM
- 1 kB
- Jim White
Issue Links
- depends upon
-
GROOVY-3238
String.valueOf(BigInteger) doesn't call String.valueOf(Object)
-
- relates to
-
GROOVY-3253
Change array formatting to be uniform
-
Activity
OK, now I got a test that fails.
I don't have time to work on this more right now, but the first difference looks like something to do with InvokerHelper.toString(Object) vs String.valueOf(Object).
groovy -v
Groovy Version: 1.6-RC-1 JVM: 1.6.0_11
groovy Groovy3237Bug.groovy
.E
Time: 0,125
There was 1 error:
1) testGroovy3227(groovy.bugs.Groovy3237Bug)java.lang.AssertionError: Expression: (t1 == t2).
Values: t1 = 1.2345678901234568E29, t2 = 123456789012345678901234567890
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:373)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:658)
at groovy.bugs.Groovy3237Bug.doTest(Groovy3237Bug.groovy:36)
at groovy.bugs.Groovy3237Bug$doTest.callCurrent(Unknown Source)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:47)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:142)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:150)
at groovy.bugs.Groovy3237Bug.testGroovy3227(Groovy3237Bug.groovy:51)
and
.E
Time: 0,188
There was 1 error:
1) testGroovy3227(groovy.bugs.Groovy3237Bug)java.lang.AssertionError: Expression: (t1 == t2).
Values: t1 =
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:373)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:658)
at groovy.bugs.Groovy3237Bug.doTest(Groovy3237Bug.groovy:36)
at groovy.bugs.Groovy3237Bug$doTest.callCurrent(Unknown Source)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:47)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:142)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:150)
at groovy.bugs.Groovy3237Bug.testGroovy3227(Groovy3237Bug.groovy:56)
groovy -v
Groovy Version: 1.5.6 JVM: 11.0-b16
groovy Groovy3237Bug.groovy
.E
Time: 0,078
There was 1 error:
1) testGroovy3227(groovy.bugs.Groovy3237Bug)java.lang.AssertionError: Expression: (t1 == t2).
Values: t1 = 1.2345678901234568E29, t2 = 123456789012345678901234567890
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:393)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:683)
at groovy.bugs.Groovy3237Bug.doTest(Groovy3237Bug.groovy:36)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:86)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:230)
at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:912)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnCurrentN(ScriptBytecodeAdapter.java:78)
at groovy.bugs.Groovy3237Bug.testGroovy3227(Groovy3237Bug.groovy:51)
and
groovy Groovy3237Bug.groovy
.E
Time: 0,078
There was 1 error:
1) testGroovy3227(groovy.bugs.Groovy3237Bug)java.lang.AssertionError: Expression: (t1 == t2).
Values: t1 = [null, foo, true, false, 1234], t2 = [null, "foo", true, false, 1234]
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:393)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:683)
at groovy.bugs.Groovy3237Bug.doTest(Groovy3237Bug.groovy:36)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:86)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:230)
at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:912)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnCurrentN(ScriptBytecodeAdapter.java:78)
at groovy.bugs.Groovy3237Bug.testGroovy3227(Groovy3237Bug.groovy:55)
and
groovy Groovy3237Bug.groovy
.E
Time: 0,078
There was 1 error:
1) testGroovy3227(groovy.bugs.Groovy3237Bug)java.lang.AssertionError: Expression: (t1 == t2).
Values: t1 = {foo=bar, true=true, int=1234}
, t2 = ["foo":"bar", "true":true, "int":1234]
at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:393)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:683)
at groovy.bugs.Groovy3237Bug.doTest(Groovy3237Bug.groovy:36)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:86)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:230)
at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:912)
at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnCurrentN(ScriptBytecodeAdapter.java:78)
at groovy.bugs.Groovy3237Bug.testGroovy3227(Groovy3237Bug.groovy:56)
the first difference looks like something to do with InvokerHelper.toString(Object) vs String.valueOf(Object).
PrintWriter#print(ln) does not go through the DGM whereas other Writers do. So PrintWriter output does not respect the InvokerHelper#toString()
String.valueOf() in the above test case is used to mimic the behaviour of PrintWriter to ensure that PrintWriter and other Writers produce the same output.
It is questionable if String.valueOf() should actually be used to implement print(ln) methods in the GDK since it bypasses the MOP and hence does not respect the possibly dynamic nature of Groovy objects.
However, to be consistent Writer#print(ln) must produce the same output as PrintWriter#print(ln). I'm not sure if the special handling of PrintWriter could be turned off easily.
See also
http://jira.codehaus.org/browse/GROOVY-2599
http://jira.codehaus.org/browse/GROOVY-2998
Yes, it acts like there is a MOP override of String.valueOf(BigInteger) although I don't know where that code is. And if there is such an override, that is a bug because Groovy has no business making incompatible changes in that fundamental Java API. One of the reasons such a change can't/shouldn't be done is that there is no way to get Java code (like PrintWriter) to use it.
But I don't think it is actually an intentional override, like i just commented in GROOVY-3238, the behavior is consistent with the wrong static method being chosen and a conversion to double being performed. The test fails on BigInteger because the result of String.valueOf(BigInteger) in the Groovy code produces the wrong result (it gets converted to a double and toStringed that way) unlike what happens when making that call in Java.
Oh, and the current implementation of DGM print(ln) uses InvokerHelper.toString which does a bunch of Groovy-specific formatting stuff that is different than the toString for those classes. That is part of the helter-skelter state of Groovy formatting and means that we're unlikely to get PrintWriter and String.valueOf doing exactly what they're supposed to do any time soon.
After the fix of GROOVY-3238, String.valueOf(BigInteger) related issue has gone away and that portion of the test provided in this JIRA goes through now.
The last remaining thing that is failing in the given test is following call:
doTest(["foo" : "bar", "true": true, "int": new Integer(1234)])
and the reason this is failing is because of the map param's formatting done by groovy, as in the code below
def map = ["foo" : "bar", "true": true, "int": new Integer(1234)] // output of the following produced by java.lang.String.valueOf(Object) is {foo=bar, true=true, int=1234} println String.valueOf(map) // output of the following produced by groovy's InvokerHelper.formatMap is [foo:bar, true:true, int:1234] println map
Why does Groovy have its own logic for formatting maps? Couldn't it simply delegate to java's implementation to be consistent in such formatting?
Groovy likes to display them it's own way. This (along with arrays) has been an area being cleaned up. I've found additional issues related to this but haven't had time to detail them here yet.
I don't think Groovy is going to give up on it's special output formatting, but I think we can get to a more consistent approach. The short version of what I think we need to do is define a GroovyPrintWriter which will work the same way as our non-PrintWriter print methods. A plain PrintWriter is not going to be able to do the same thing as Groovy does when it converts things to strings.
I have committed a first attempt at bringing order to this printing situation (cs14792). There are several decisions that are somewhat arbitrary, and several difficult ones that I have deferred until you folks weigh in on these changes.
There are three significant new features:
- groovy.io.GroovyPrintWriter - This a PrintWriter that implements print(Object) so that it prints the same thing that Groovy does for print(Object). In addition to it's use in DGM for the new/withPrintWriter methods, this is the only public API I'm aware of that can be called from Java to format objects the way Groovy does.
- Object as String has been implemented to format the string the way Groovy does when internally 'toStringing' things (like in regex, join, etc). Turns out there hasn't been a way to do that in Groovy (apart from printing). If you use Object.toString() or String.valueOf(Object) you get Java formatting, not Groovy (even in Groovy code).
- char[] as String will now return a String consisting of those characters. That is different than the way other arrays are treated, but it is consistent with the way java.io.PrintWriter.print(char[]) works.
So the idea here is that whatever you print with GroovyPrintWriter or DGM.print will be the same as the String you get from Object as String. That is very much like the relationship PrintWriter and String.valueOf(Object) have. It is also consistent with as doing "groovy" formatting.
So we have a some consistent rules now and they can be seen in PrintTest.groovy.
Probably the most significant remaining issue is that arrays of primitives are not displayed the same way as arrays of objects. The arrays of primitives are coerced to lists and are displayed that way with square brackets. Arrays of objects are displayed with curly braces. If we want the curly braces to signify "array", then it should apply to array of primitive too.
GROOVY-3253has changed Object[] to print with square brackets - table updated 1/5.
In that same vein, while char[] is now displayed like a string, Character[] is not. I think that it should display the same as char[] and the PrintWriter.print(char[]) style (like a string) is the way to go. OTOH, there are cases where you may want to display individual characters as an array, so perhaps leaving Character[] the way it is now is better.
A summary of the current (trunk) state of affairs:
| Groovy | print/as String |
| (null) | null |
| ("foo") | foo |
| (true) | true |
| (false) | false |
| ((byte)123) | 123 |
| ((short)1234) | 1234 |
| (new Integer(1234)) | 1234 |
| (new Long(9999999999)) | 9999999999 |
| (new Float(1234.5678)) | 1234.5677 |
| (new Double(1234.5678)) | 1234.5678 |
| (new BigInteger("123456789012345678901234567890")) | 123456789012345678901234567890 |
| (new BigDecimal("12345678901234567890.1234567890123456789")) | 12345678901234567890.1234567890123456789 |
| (new Date(107, 12, 31)) | Thu Jan 31 00:00:00 PST 2008 |
| (new StringBuffer("bar")) | bar |
| ([null, "foo", true, false, new Integer(1234)]) | [null, foo, true, false, 1234] |
| (["foo" : "bar", "true": true, "int": new Integer(1234)]) | [foo:bar, true:true, int:1234] |
| ([null, "foo", true, false, new Integer(1234)] as Object[]) | [null, foo, true, false, 1234] |
| (["foo",new Integer(1234)] as String[]) | [foo, 1234] |
| ([true, false] as Boolean[]) | [true, false] |
| ([true, false] as boolean[]) | [true, false] |
| ([1, 2, 3] as int[]) | [1, 2, 3] |
| ([1, 2, 3] as Integer[]) | [1, 2, 3] |
| (['a', 'b', 'c'] as char[]) | abc |
| (['a', 'b', 'c'] as Character[]) | [a, b, c] |
Once this settles down, an optimization in the DGM.print methods will be to convert directly to strings and use the Writer rather than constructing GroovyPrintWriters.
And finally I haven't addressed PrintStream, but it probably needs GroovyPrintStream in order to match what the DGM.print methods do when they don't get a Writer (which is to format as string and print to System.out).
Committed to trunk (cs14863) what seems to be most of the changes and a test case that demonstrates what I think the API is supposed to be.
A key difference btw Groovy print and Java print is that Groovy is defined in terms of Object.toString() rather than String.valueOf(Object). That is a necessary difference because Groovy can't override a static method. And this works out OK because in Groovy null.toString() results in "null" (rather than a NPE) which is one of the key virtues of valueOf. Furthermore Groovy code that wants the Java string for an object rather than the Groovy style can still call String.valueOf. That would also be necessary for large and/or recursive structures which Groovy will currently do poorly with (for another JIRA).
There are now DGM for PrintStream.print and PrintStream.println so that Groovy code that does a System.out.println(Object) will work just like println(Object).
Converted to a GroovyTestCase.