[External] : Re: Some classes could be sealed

[John Hendrikx]

I'm curious to know why these classes are not allowed to be subclassed 
directly, as that may be important in order to decide whether these 
classes should really be sealed.

--John

On 01/02/2023 20:37, Kevin Rushforth wrote:
> I read the spec for sealed classes more carefully, and it turns out we 
> can't make Node sealed. At least not without API changes to SwingNode 
> and MediaView (and implementation changes to Printable in the 
> javafx.web module). All of the classes listed in the "permits" clause 
> must be in the same module, and SwingNode (javafx.swing) and MediaView 
> (javafx.media) extend Node directly they would need to be "permitted" 
> subtypes, but there is no way to specify that. We would also need to 
> do something about the tests that extend Node and run in the unnamed 
> module. So this doesn't seem feasible.
>
> We could still seal Shape, Shape3D, LightBase, and Material, since all 
> permitted implementation are in the javafx.graphics module. It may or 
> may not be worth doing that.
>
> -- Kevin
>
>
> On 2/1/2023 9:45 AM, Nir Lisker wrote:
>> I'll add that internal classes, mostly NG___ peers, can also benefit 
>> from sealing. NGLightBase is an example.
>>
>> Material is another public class that can be sealed.
>>
>> On Wed, Feb 1, 2023 at 7:37 PM Kevin Rushforth 
>> <kevin.rushforth at oracle.com> wrote:
>>
>>     I agree that we should only seal existing classes that could not
>>     have been extended by application classes. The ones I listed in
>>     my previous email fit that bill, since an attempt to subclass
>>     them will throw an exception when it is used in a scene graph.
>>     Each documents that subclassing is disallowed.
>>
>>     Btw, we've already started making use of pattern-matching
>>     instanceof in the implementation anyway. It would be the first
>>     API change that relies on a JDK 17 feature, but for JavaFX 21, I
>>     see no problem in doing that.
>>
>>     -- Kevin
>>
>>
>>     On 2/1/2023 9:06 AM, Philip Race wrote:
>>>     In the JDK we've only sealed existing classes which provably
>>>     could not have been extended by application classes,
>>>     so I'm not sure about this ..
>>>
>>>     also I think that might be the first change that absolutely
>>>     means FX 21 can only be built with JDK 17 and later ..
>>>
>>>     -phil
>>>
>>>     On 2/1/23 8:59 AM, Thiago Milczarek Sayão wrote:
>>>>     Yes, sorry, I made the email title in plural, but I meant what
>>>>     Michael said, Node would be sealed permitting only what is
>>>>     needed for JavaFx internally.
>>>>
>>>>
>>>>     -- Thiago
>>>>
>>>>
>>>>     Em qua., 1 de fev. de 2023 às 13:48, Michael Strauß
>>>>     <michaelstrau2 at gmail.com> escreveu:
>>>>
>>>>         I don't think that's what Thiago is proposing. Only `Node`
>>>>         would be sealed.
>>>>         The following subclasses would be non-sealed: Parent, SubScene,
>>>>         Camera, LightBase, Shape, Shape3D, Canvas, ImageView.
>>>>         And then there are additional subclasses, which don't fit
>>>>         into this
>>>>         idea since they are in other modules: SwingNode (in
>>>>         javafx.swing),
>>>>         MediaView (in javafx.media), Printable (in javafx.web).
>>>>
>>>>
>>>>
>>>>         On Wed, Feb 1, 2023 at 5:39 PM John Hendrikx
>>>>         <john.hendrikx at gmail.com> wrote:
>>>>         >
>>>>         > I think this may be a bit unclear from this post, but
>>>>         you're proposing I think to make `Node`, `Shape` and
>>>>         `Shape3D` sealed.  For those unaware, you're not allowed to
>>>>         extend these classes (despite being public).  For example
>>>>         Node says in its documentation:
>>>>         >
>>>>         >    * An application should not extend the Node class
>>>>         directly. Doing so may lead to
>>>>         >    * an UnsupportedOperationException being thrown.
>>>>         >
>>>>         > Currently this is enforced at runtime in NodeHelper.
>>>>         >
>>>>         > --John
>>>>         >
>>>>         > On 01/02/2023 15:47, Thiago Milczarek Sayão wrote:
>>>>         >
>>>>         > Hi,
>>>>         >
>>>>         > NodeHelper.java has this:
>>>>         >
>>>>         > throw new UnsupportedOperationException(
>>>>         >         "Applications should not extend the "
>>>>         >         + nodeType + " class directly.");
>>>>         >
>>>>         >
>>>>         > I think it's replaceable with selead classes. Am I right?
>>>>         >
>>>>         > The benefit will be compile time error instead of runtime.
>>>>         >
>>>>         >
>>>>         > -- Thiago.
>>>>         >
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20230201/5fe494cc/attachment-0001.htm>