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.rushfo...@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ß
    <michaelstr...@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.hendr...@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.
        >



Reply via email to