hmanikkothu / atinject

Automatically exported from code.google.com/p/atinject
0 stars 0 forks source link

Should @Target be allowed on @Qualifier and @Scope? #5

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The doc seems to disallow @Target on scope or qualifier annotations.

I don't see why this restriction exists and would like to see it removed. 

Original issue reported on code.google.com by michael....@gmail.com on 17 Jun 2009 at 1:58

GoogleCodeExporter commented 8 years ago
Agreed.

Original comment by gavin.k...@gmail.com on 18 Jun 2009 at 11:47

GoogleCodeExporter commented 8 years ago
We don't want to restrict the ways injectors can use qualifiers. For example, 
while
the annotation portion of this specification only covers the use of qualifiers 
on
fields and parameters, Guice uses qualifiers on methods (for provider methods, 
as
does 299 on producer methods), 299 uses them on classes, and other injectors 
may use
them in other places. For example, if you extended 299 to allow qualifiers on
stereotype annotations, you'd have to add ANNOTATION_TYPE to the list of 
targets.

If we were to allow @Target, we'd have to specify the set of element types 
you'd need
to include to ensure compatibility across injectors. I don't think this is worth
doing. If you only care about supporting a certain set of injectors, you can 
ignore
this rule and apply @Target, but your qualifiers may not be fully usable by all
injectors.

Original comment by crazybob...@gmail.com on 23 Jun 2009 at 9:52

GoogleCodeExporter commented 8 years ago
Please reopen and provide a list of target element types we should require if 
you
disagree with this decision.

Original comment by crazybob...@gmail.com on 23 Jun 2009 at 11:22

GoogleCodeExporter commented 8 years ago
I don't mind if the JavaDoc is silent on this issue, but it shouldn't say what 
it
currently says, which is that there should be no @Target.

299 recommends @Target({METHOD, FIELD, TYPE, PARAMETER}) for qualifiers and
@Target({TYPE, METHOD, FIELD}) for scopes.

I really don't know what a qualifier or scope could possibly mean on a 
CONSTRUCTOR,
PACKAGE or LOCAL_VARIABLE!

So I don't think we're done on this one.

Original comment by gavin.k...@gmail.com on 24 Jun 2009 at 1:42

GoogleCodeExporter commented 8 years ago
I thought it would be best if the comments on @Target was removed altogether, 
but if 
you would feel more comfortable, would you both be okay with a comment like:

It is not prohibited to apply @Target to @Qualifier and @Scope annotations to 
limit 
the program elements that may be annotated, although it may restrict the usage 
of 
the defined Qualifier or Scope across injectors.

Original comment by michael....@gmail.com on 24 Jun 2009 at 2:00

GoogleCodeExporter commented 8 years ago
Do you find that users apply qualifier and scope annotations in the wrong 
places?
We've seen users apply annotations to interface methods in places where we only
support them on class methods, but @Target wouldn't help there. Of course, 
frameworks
can still detect such cases and generate warnings. 

299 only has to consider uses covered by its own spec, but 330 must accommodate 
uses
we haven't thought of. We could try to come up with a list of element types we 
know
qualifiers and scopes will *never* be used on, but I'm not sure how we'd 
support such
assertions. Doing so seems risky relative to the small reward, and we could 
probably
utilize our time in better ways. Also, if we add more ElementType values in the
future, users may end up needing to retrofit their annotations.

I guess we could lighten the restriction on using @Target, but I dislike doing 
that
without providing more guidance to the user. It's difficult to do that without
knowing about all the ways these annotations will be used. Do we plan to update 
the
docs every time an annotation is used in new ways?

It's not that I'm adamantly in favor of prohibiting @Target. I just don't think 
this
is worth spending much time on. Is there a particular error you're interested in
preventing?

Original comment by crazybob...@gmail.com on 24 Jun 2009 at 9:05

GoogleCodeExporter commented 8 years ago
If my application defines a qualifier then I have a particular use for it, and 
it 
may be that I only allow it on a limited subset of available targets (say only 
on 
fields or methods). I don't see why I am not allowed to mark my own qualifier 
as 
being restricted to those elements. I defined it and I know what it is used 
for, so 
why can't I protect it from being specifiable on types or parameters?

I'm a little puzzled why you would want to take away that flexibility. Saying 
that 
other injectors may want to provide broader support doesn't apply, since I am 
limiting the qualifier to mean something, and having an injector broaden that 
support won't make sense to my qualifier.

PS Can you please reopen this issue so that it is visible alongside the rest? 
Thanks.

Original comment by michael....@gmail.com on 25 Jun 2009 at 4:22

GoogleCodeExporter commented 8 years ago
The spec enables portability. If someone doesn't care about portability, then 
they can just ignore the spec, right?

Original comment by crazybob...@gmail.com on 25 Jun 2009 at 6:39

GoogleCodeExporter commented 8 years ago
I think we basically have two options:

  1) prohibit @Target
  2) don't mention @Target

I don't like #2 because it looks like we overlooked this aspect. If we were to 
pick a
middle ground, I'd want to provide specific advice for using @Target, but 
that's not
possible. It would also add complexity to the spec and impl/usage of qualifiers 
with
little real benefit.

That leaves us with #1.

If someone strongly disagrees, please reopen. I'm flexible.

Original comment by crazybob...@gmail.com on 26 Jun 2009 at 10:58

GoogleCodeExporter commented 8 years ago

Original comment by crazybob...@gmail.com on 26 Jun 2009 at 10:58

GoogleCodeExporter commented 8 years ago

"The spec enables portability. If someone doesn't care about portability, then 
they 
can just ignore the spec, right?"

The spec represents a common subset, and there is a great deal of value in 
knowing 
and using the subset, even though you might also go into the injector-specific 
world 
to access additional features. You know you are using them, and can potentially 
put 
them in a place that makes it more obvious or easy to maintain/keep track of.

Original comment by michael....@gmail.com on 27 Jun 2009 at 1:58

GoogleCodeExporter commented 8 years ago
"I think we basically have two options:

  1) prohibit @Target
  2) don't mention @Target"

I don't know why you dropped the third option, which I see as the compromise 
between 
1 and 2. This option is that @Target be mentioned, but not prohibited. The only 
guidance that needs to be given is just the bit that you are concerned about. 
What 
about replacing the current text:

"is not annotated with @Target. While this specification only covers applying 
qualifiers to fields and parameters, some injector configurations might use 
qualifier annotations in other places (on methods or classes for example)."

with the expected guidance further down in the javadoc:

"Applying @Target to @Qualifier or @Scope annotations may restrict the usage of 
the defined Qualifier or Scope across injectors."

Original comment by michael....@gmail.com on 27 Jun 2009 at 2:25

GoogleCodeExporter commented 8 years ago

Original comment by crazybob...@gmail.com on 27 Jun 2009 at 2:34

GoogleCodeExporter commented 8 years ago
Fixed in r8. Please review.

Original comment by crazybob...@gmail.com on 27 Jun 2009 at 3:00