I held on proposing sealed class changes because they will be more beneficial once switch patterns are introduced. I also held on refactoring to records (of which I have a branch in my fork) for the reason that record patterns are not out yet. I think that once we have more pieces of the algebraic data types puzzle it will be very beneficial to do a decent amount of refactoring. There's nothing stopping us from starting with what we have for the purpose of upgrading errors from runtime to compile time, but we will need a second iteration anyway in some of these cases.
On Wed, Feb 1, 2023 at 9:37 PM Kevin Rushforth <kevin.rushfo...@oracle.com> 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.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. >>> > >>> >> >> >> >
