MoreTypes#toString(Type) double-qualifies parameterized inner classes

[GoogleCodeExporter]

Repro:

package com.google.inject;

class Foobar {
  public static class Foo {}
  public static class Bar<String> {}

  TypeLiteral<?> fooType = new TypeLiteral<Foo>() {}
  TypeLiteral<?> barType = new TypeLiteral<Bar<String>>() {}
}

The #toString() for fooType will be:

com.google.inject.Foobar$Foo (which is right)

The #toString() for barType will be:

com.google.inject.Foobar.com.google.inject.Foobar$Bar<String>

It should be:

com.google.inject.Foobar$Bar<String>

Attached patch for the fix.

Original issue reported on code.google.com by phopkins on 29 Dec 2008 at 12:02

Attachments:

• tostring.patch

[GoogleCodeExporter]

We've got a testcase (possibly misplaced in TypesTest.java, not 
MoreTypesTest.java) that ensures that our 
toString() behaviour is consistent with the JDK. That said, the JDK behaviour 
is a bit lame. For example, this is 
the JDK's output:
  com.google.inject.util.TypesTest.com.google.inject.util.TypesTest$Inner<java.lang.Float, java.lang.Double>

Perhaps there's a scenario where the enclosing type is different than the owner 
type? This might be to cover 
the case when the containing class is itself parameterized; ie:
  com.google.Foo<String, Integer>.com.google.Foo$Inner<Float>

I'll gladly change our behaviour if the JDK is wrong, but I'm not certain that 
it is...

Original comment by limpbizkit on 29 Dec 2008 at 2:59

• Changed state: Invalid

[GoogleCodeExporter]

Ah. Thanks for the heads-up about the tests. I *think* that the JDK is wrong 
about this, but I understand that 
it probably makes more sense to match the JDK behavior.

I filed a bug at bugs.sun.com. I'll fill this in with the bug ID if they accept 
it.

I tracked down what I think is the source code for ParameterizedTypeImpl here: 
http://www.docjar.com/html/api/sun/reflect/generics/reflectiveObjects/Parameteri
zedTypeImpl.java.html

You can see the oddity about doing an instanceof on the ownerType. This 
actually means that the JDK has 
different (and IMO correct) behavior in just the "parameterized containing 
class" case that you mention! (And 
is inconsistent with MoreTypes.) Guice is consistently adding the ownerType, 
while the JDK doesn't do it if the 
ownerType is a ParameterizedType. This comes up when the type is a 
parameterized, non-static inner type of 
a parameterized type. E.g.:

class Outer<S> {
  class Inner<T> {}
}

I've attached a patch that adds a failing test to TypesTest that shows how 
Guice will be consistent and double 
the Outer class's name, while the JDK will return the "correct," non-doubled 
name. I don't expect that you'll 
"fix" MoreTypes to match this rather odd JDK behavior, of course, but it does 
lend support to the buggy JDK 
opinion. We'll see if I get a response from Sun.

This is no huge deal, though graphs of Injectors with parameterized inner types 
will look a bit uglier with the 
duplication. (See the long row of interfaces in the middle of the graph.) :)

Original comment by phopkins on 29 Dec 2008 at 4:55

Attachments:

• more-tostring.patch
• grapher-graph.png

[GoogleCodeExporter]

There's also a small bug in the current MoreTypes.toString code, specifically 
the
ParameterizedType section: If the type has 0 type arguments (does that ever 
happen?
the code checks it...) the method will return "com.a.b.MyType>" (note the 
closing
bracket).

Original comment by aragos on 19 May 2009 at 2:12