Explicitly forbid wildcards in Event/Instance injection points

[manovotn]

Recently I came across a corner cases regarding injection of Event and Instance with wildcards in them.

So cases such as:

@Inject
Event<?> wildEvent;

@Inject
Event<? extends Foo> extendsFooEvent;

@Inject
Instance<?> wildInstance;

I think the specification is currently not clear on whether this should work and I propose that we explicitly forbid it. Note that this includes:

• Validating the IPs like I showed in the code above
• As well as runtime validation for cases such as BeanManager.createInstance().select(new TypeLiteral<Event<?>>(){}
  • This should be doable as you can only get to wildcards using the select(TypeLiteral, Annotation...) variant

Looking at existing implementations, I found that:

• Weld already forbids this for Instance and Event (throwing a DefinitionException)
• Quarkus (Arc) forbids this for Instance (DefinitionException) but permits Event

I think we should forbid it because of several reasons.
Firstly, in case of Instance, the specification IMO already indirectly suggests it is not permitted via required type of Instance combined with Legal injection point types combined with Legal bean types . Granted, this does not forbid it, but certainly doesn't list wildcard as allowed "type". Note that this does not apply to Event.

Secondly, usability - just having a wildcard (wildEvent field in the code above) or one with upper bound (extendsFooEvent in the code above) has basically no use. You cannot fire an actual type (i.e. String) event from it and attempting to select() a subclass won't compile either. For Event, the only "usable" case would be a wildcard with lower bound but even then it has dubious benefits as you might well have event with given actual type. The wildcard will also not help in infering any type variables found in the actual runtime payload type.
Similar applies to Instance where looking for a bean with wildcard type (which isn't allowed) doesn't make sense.

Thirdly, the type variable T of Event<T> and Instance<T> should represent a selected/required type and you normally cannot declare a type that would be a wildcard so you shouldn't be able to do it this way either.

Thoughts?

Cc @ljnelson whose question got me digging into this.

[Ladicek]

Agree especially on the 3rd point. The type argument to Instance or Event should be a "standalone" or "top-level" type (for lack of better term), and wildcard type cannot stand alone -- it must always be a type argument.

[ljnelson]

Hmm, but that's already taken care of by the typing of the fire method, right? You can't fire a wildcard already.

For sure there is no reason to permit ? extends Something as a type argument to Event<X>. But should we, in the spec, explicitly, prevent ? super Something? I guess saying the type argument must be a class or interface type or an array-type-whose-element-type-is-a-class-or-interface-or-primitive-type is one way to go, but the amount of words you would have to expend to describe this doesn't really seem worth it. Mostly just thinking out loud.

[manovotn]

That is a mouthful for sure.
We could also just write it down in the Event (and Instance) javadocs, not the spec itself. My intention was just to clarify it somewhere.

But should we, in the spec, explicitly, prevent ? super Something?

I am aware that this combination can be used, but is it really useful?
That is, can you think of a use case where this would bring value over using a simpler version without a wildcard?

[ljnelson]

The use case doesn't change, because given Event<? super Something> or Event<Something>, the signature of the fire method is still (effectively) fire(Something). No, it's more about specification language: if you base the specification in this area on a subset of type information ("you can use any type as a type argument supplied to the Event<T> type except..."), now you have to explain the "except" part: i.e. why the type bounding expressed by the Java type argument itself isn't good enough.

Right now I can inject (or look up) Event<?>, which you and I know doesn't make sense, but it doesn't make sense only because I literally can't do anything with it—Java won't let me. We don't have to explain anything here; Java does the explaining and prohibits use.

I can also inject (or look up) Event<? super Something> but right now when I try to do something with the fire method, in certain implementations it fails at runtime. It feels like it shouldn't have to do this; Java concludes all the type information involved is OK, so why should we conclude any differently? If we support this, then, again, Java does the explaining and we don't have to write anything new in the specification.

This is all minor IMHO. Anyway, why not just amend Weld/ArC/OpenWebBeans to support ? super Something as a valid type argument (easy to do) and move on? No extra spec language needed. Otherwise we have to make sure we get the language right explaining precisely what type arguments are permitted and prohibited.

[Ladicek]

Personally, I'd like to do this just for consistency. As mentioned above, the type argument to Instance or Event must be able to stand alone, which wildcards cannot. As for definitions, it's really quite simple:

A standalone type is a primitive type, class type, array type, parameterized type or type variable. Notably, wildcard type is not a standalone type, because wildcards can only occur as type arguments.

We wouldn't necessarily have to include primitive types, because they cannot occur as type arguments, but my intuition behind standalone types is "it's a type that can be used as a field type".

[ljnelson]

the type argument to…Event must be able to stand alone

Why? (I understand you would prefer this state of affairs. Why "must"?)

[Ladicek]

Why must?

1. Because the specified type must be a valid observed event type. In other words, whatever you write in Event<...>, you should be able to write in @Observes ... (this is just like with Instance: whatever you write in Instance<...>, you should be able to write in @Inject ...). This is not written in or implied by the specification, but symmetry here just makes perfect sense.

2. Because it makes no sense for it not to. The specified type of Event is used in 2 ways:

   1. it restricts the set of event types that can be fired through that particular Event
   2. it is used for inferring the parameterized type of an event object in case its class is generic

   As for i., we already established that the unbounded wildcard (?) and a wildcard with upper bound (? extends Foo) are useless, because it is not possible to fire() anything on such Event. A wildcard with lower bound (? super Foo) can in theory be used, but in practice, using just Foo as specified type has the same meaning, so banning that doesn't take away any power.

   As for ii., if the specified type is a wildcard, it cannot be used for substituting any type arguments, because wildcards don't have type arguments.

[Azquelt]

I think Ladicek's logic makes sense. I can't see any case where using wildcard in as the specified type would do anything useful. If a user has done so, I think it's likely they're making a mistake so explicitly making it a definition error seems reasonable to me.

[gavinking]

For the record, this change broke our Jakarta Data implementation. And will probably break other programs as well.

This has been explicitly allowed by the CDI spec since 1.0:

• see section 5.2.4 of the CDI 3 spec
• or section 2.4.2.4 of the CDI 4 spec

So this is a breaking, backwardly-incompatible change.

[gavinking]

Him, so it looks like the only case you considered was covariantly-wildcarded types like this:

@Inject
Event<? extends Foo> extendsFooEvent;

But that's not the use case. Event is by nature a contravariant type, so giving it a covariant wildcard makes no sense. The case which makes sense is a contravariant wildcard:

@Inject
Event<? super LifecycleEvent<?>> acceptsLifecycleEvent;

Instance is different, since Instance is by nature covariant.

So while it would be OK to:

1. reject covariant wildcards in Event, and
2. reject contravariant wildcards in Instance,

doing the opposite makes little sense; it just breaks the usual rules for matching beans to injection points. You have added in a special case that contradicts the usual rules of the section titled "Assignability of raw and parameterized types" for AFAICT no especially good reason.

[gavinking]

Now, to clarify, we can live without this, we'll just change the code we generate—but I just don't understand why you would break something perfectly functional and well-defined for no particularly good reason.

The clear rule in CDI is that if I have a bean which matches:

@Inject X<Y> x;

then it also matches and will be be injected to all of:

@Inject X<? extends Y> x1;
@Inject X<? super Y> x2;
@Inject X<?> x3;

this change introduces a breaking irregularity where this general rule suddenly does not apply to Event nor to Instance.