Coalesce @javax.inject.Named and @com.google.inject.Named

[GoogleCodeExporter]

Having both is certainly cause for confusion. We should canonicalize these, 
such that a binding for 
one can be used to inject the other.

Original issue reported on code.google.com by limpbizkit on 8 Sep 2009 at 12:20

[GoogleCodeExporter]

I'm not sure what approach you were thinking of for this, but here's a patch I 
put 
together for it. It canonicalizes Named annotations at Key creation and retains 
the 
original type in order to make error messages make as much sense as possible.

Original comment by cgdec...@gmail.com on 25 Mar 2010 at 2:09

Attachments:

• canonicalize_named.patch

[GoogleCodeExporter]

It would be nice to use the above patch (which, after a short glance, seemed 
fine to
me) or to write something similar soon. Gin cannot adopt JSR330 annotations 
easily
(without taking a performance hit) if this is not solved.

Original comment by aragos on 22 Apr 2010 at 1:11

[GoogleCodeExporter]

I'll merge it in this weekend if no-one gets to it before then.

Original comment by sberlin on 22 Apr 2010 at 2:08

[GoogleCodeExporter]

Comments on the patch...

Overall, the patch is great. I like that canonicalization happens in Key. 
That's the right place, and it means 
the rest of Guice doesn't have to think at all.

But Guice shouldn't expose an implementation of the JSR 330 Named annotation. 
When the user binds the 
JSR 330 type, we should just swap it out with the Guice Named annotation and 
forget the original 
annotation type. The extra error handling isn't worthwhile, especially since 
the "implements both" 
annotation cannot implement equals correctly (it breaks transitivity).

Colin - have you done the necessary paperwork to submit code to Guice? 
  http://code.google.com/legal/individual-cla-v1.0.html

Sam - would you like to make the changes above and commit? Otherwise I can.

Original comment by limpbizkit on 22 Apr 2010 at 7:11

[GoogleCodeExporter]

If you have the time, go for it.  One quirk (I think) with "forgetting" the 
jsr-330
named annotation would be error messages wouldn't be quite correct (it would say
something is annotated with guice's Named, I think).  If it's possible to keep 
the
error messages looking correct without really implementing jsr-330, that's 
sounds
like a good middle ground.

Original comment by sberlin on 22 Apr 2010 at 12:35

[GoogleCodeExporter]

I've submitted the online agreement form now.

You're right about NamedImpl not needing to implement or equal JSR 330 Named. I 
had been 
initially thinking something like that would be needed for the implementation 
(which it 
wasn't) and didn't see any need to change it... didn't think about the 
transitivity 
issue.

If NamedImpl only implements the Guice type of Named, it might be strange for 
its 
toString() to say it was @javax.inject.Named... would it make sense to have it 
just use 
"@Named" without the package? That seems like it might make more sense and 
wouldn't 
imply to users that Guice differentiates between the two types.

Original comment by cgdec...@gmail.com on 22 Apr 2010 at 2:32

[GoogleCodeExporter]

fixed in r1152.  i slightly modified your patch, Colin, so please review!  (I
followed Jesse's suggestion of not implementing both interfaces, which led to 
not
requiring the new NamedImpl class, and slightly tweaked the test methods.)

Original comment by sberlin on 24 Apr 2010 at 10:32

• Changed state: Fixed