Specify behavior for sealed types

[Ladicek]

Sealed types are valid bean types, but they are unproxyable.

This commit only makes explicit what already implicitly follows from
the existing CDI specification and updates in the Java / JVM specifications.

[Ladicek]

This is a proposal for discussion, but I believe it's good enough.

I previously mentioned that we could make normal scoped records a definition error, but I figured it would be easier to just specify that they are unproxyable, which leads to a deployment problem. I didn't want to go through the same exercise with sealed types :-)

[Ladicek]

Writing TCK tests for this is problematic as long as the TCK is compiled for Java 11. I'm not sure how to solve that problem.

We can leave records and sealed types unspecified in CDI 4.1 and only specify them in CDI 5.0, where we could bump the Java version.

[manovotn]

Suggested change

	* A bean type may be an interface, a concrete class or an abstract class or a record, may be declared sealed or non-sealed or final, and may have final methods.
	* A bean type may be an interface, a concrete class, an abstract class or a record. It may be declared sealed, non-sealed or final, and may have final methods.

Just a minor suggestion. The original sentence is somewhat long and hard to chew on, so splitting it in two might help.

[Emily-Jiang]

I found this confusing. Records and Sealed classes cannot be managed beans. Why do we call them out for Bean Types. Can you list a use case to support such a Record or Sealed Classes as bean types and then inject them to something else. I would not add them here at all.

[manovotn]

Andrew's comment nicely sums this up #693 (comment)

Generally the use case is similar to why you would want to have a non-proxyable class (such as with no no-arg constructor) to be a bean - it is doable but only works until you try to assign it normal scope or use an interceptor.
We didn't have a specific case in mind; instead, the idea is to avoid introducing extra rules where we can apply what CDI already does.

[Ladicek]

Records and sealed types can be managed beans. (They are unproxyable, so cannot be normal scoped or intercepted or decorated, but it's perfectly legal to make a record @Dependent, as long as it defines a zero-param constructor, which it can. It is not very practical, but it is possible.)

But that's not the point here. There are other kinds of types that cannot be managed beans (such as interfaces) yet it makes perfect sense for them to be bean types. Maybe you're confusing bean types and bean classes?

[Emily-Jiang]

The point is that it is VERY problmatically. I recommend not to list them out. If we call it out here, it encourages end users to use Records in anger if they just do a quick search on the doc.
The existing phrase A bean type may be an interface, a concrete class or an abstract class, and may be declared final or have final methods. covers to Records, sealed classes.

[Ladicek]

If we call it out here, it encourages end users to use Records in anger if they just do a quick search on the doc.

That is not an argument for or against anything. This is a specification, not a microwave oven manual.

You could argue that per JLS, a record class is "a restricted kind of class", so whatever applies to classes applies to records as well, which would be correct. So we don't necessarily have to call out records.

But that doesn't cover sealed types. We specify that a bean type may be final, but we don't specify whether it may be sealed, and until we do, it remains unspecified and hence non-portable.

[Emily-Jiang]

okay, just add sealed types then.
I am also wondering whether to clearly say what kind of injections an end user can do with Records. In order to inject a Record, it must be @Dependent scoped and it must define a zero-arg constructor. Thoughts?

[Ladicek]

Records may be made beans by following the existing rules. Records are always final, so they are unproxyable, which means that a record bean cannot be normal scoped. That leaves @Singleton and @Dependent. It also has to have a zero-param constructor or an @Inject constructor. All the usual. If you don't want to mention records here (because users would want to use them and because the information is redundant), why would we want to mention them elsewhere (causing the same "problems")?

[Emily-Jiang]

okay, fine with not mentioning Record.

[Ladicek]

Done.

[manovotn]

Looks good and covers what we originally talked discussed.
Let's keep it open until next CDI meeting and see if anyone has comments on this.

The inability to test this is unfortunate but perhaps we can keep a tracking TCK issue with future version (5.0?) where we know the JDK version will be enough. It's the same case as with records and the lang model TCK. Personally, I think it's worth putting this into specification, even at the cost of missing TCK test.

[Ladicek]

One open question: should non-abstract sealed classes be managed beans? I opted into no, because it felt like the right thing to do, but I don't think there's anything fundamental preventing that from working. It's just that trying to inject it would likely lead to unresolved ambiguity, because all permitted subclasses of that class would have it among their bean types.

[manovotn]

It's just that trying to inject it would likely lead to unresolved ambiguity, because all permitted subclasses of that class would have it among their bean types.

Generally, I don't think there is much use for sealed classes as beans.
That being said, permitted subclasses can still use qualifiers to differentiate the impl, right?
Which then makes the use case equivalent to using interface as bean type and qualifiers on top to tell various impl apart and from that perspective, it would still be usable 🤷

[Ladicek]

That is correct, yes.

[Azquelt]

I think I'd lean towards just reiterating how the existing rules apply to records and sealed classes, so that there aren't new rules to remember.

• Records generally won't be managed beans because they don't have a no-arg constructor or a constructor annotated @Inject, but we don't need to explicitly say that they aren't
• Records are final so they're unproxyable
• Sealed classes can't be arbitrarily extended so they're unproxyable

While I can't see a use case for a non-abstract sealed class being a managed bean, I also don't see a reason to have a special case to make it not be a managed bean. It also won't be discovered unless it has a scope annotation or the discovery mode is all.

[Ladicek]

Good point, I agree that we shouldn't impose additional rules other than what already falls out from the existing rules and new JLS/JVMS rules. I amended the PR.

[manovotn]

LGTM, I am in favor of merging this as-is.

[Ladicek]

Removed references to records, because records are a restricted form of classes that are always final, so we technically don't have to call them out at all.