Hi Paul,

On Sat, Jul 31, 2021 at 8:09 AM Paul King <pa...@asert.com.au> wrote:

> [...]
> Questions/Discussion points:
> (1) The annotations would allow some or all of the initial change to
> be backported to versions of Groovy (e.g. to a future 3.0.x or 3.1.x)
> where the grammar change isn't made. Are we happy to support both
> styles? For traits, the @Trait annotation still exists, but we promote
> use of the keyword variant and regard the annotation as mostly an
> internal implementation detail.
>

Indeed, it would be nice if we allowed sealed classes to 3.x with an
annotation.
We've often provided features that are JDK version specific to older JDKs
when developers use Groovy, so that's great if we can continue that
tradition.


> (2) The @NonSealed annotation is aligned with what would be the
> `non-sealed` modifier. Other forums have discussed the hyphenated
> 'keyword' as being a little ugly. Kotlin uses 'open' and defaults to
> classes being final. Scala uses sealed and final but just regards no
> modifier as being the non-sealed case. Java designers thought that in
> general, non-sealed would be the least used option and didn't want to
> align that with no modifier (for fear of developers accidentally
> choosing that option accidentally). Do we follow Scala's lead and
> align omitting the non-sealed modifier as designating open
> inheritance? (This aligns somewhat with Groovy's goal of minimising
> boilerplate and not assuming its users need excessive hand holding).
> Do we have an alias like unsealed (and @Unsealed) for those wanting to
> avoid the "ugly" hyphenated keyword? Java suggests it may likely have
> further such keywords in the future in any case.
>

Personally, yes, I'd tend to follow Scala's approach, like we did with
public-by-default in Groovy.


> (3) We could use Class[] rather than String[] for permittedSubclasses
> or also support permittedSubclassNames or similar.
>

When seeing the examples, that's the first thought that occured to me.
I'd rather have Class[] than String[], as it'll also help refactoring tools
in IDE to correctly rename types.
So I'm in favor of permittedSubclasses to take Class[], and would be okay
if we supported permittedSubclassNames with String[] if needed.


> (4) Java17 will produce an error if not all of the classes mentioned
> in the permits clause are found at compilation time. Currently the PR
> doesn't attempt to do this.
> (5) Like Java17, this PR opens the possibility of compilers (perhaps
> just for type checked code in Groovy's case) to check for
> exhaustiveness if sealed hierarchies are used in switch
> expressions/statements but doesn't currently implement this feature.
>

For (4) and (5) what happens at runtime if the exhaustiveness is not
covered?
For now as an incubating feature, I think it's okay if we don't provide a
compilation error yet, but I would like to see this implemented at some
point, as it'll probably avoid some problems further down the line (runtime
errors, cases not covered leading to bugs...)

That's really nice to see sealed classes coming to Groovy!!!
Great work, as always!

Guillaume


>
> Any thoughts?
>
> Thanks, Paul.
>
> [1] https://openjdk.java.net/jeps/409
> [2]
> https://github.com/apache/groovy-website/blob/asf-site/site/src/site/wiki/GEP-13.adoc
> (Draft)
> [3] https://github.com/apache/groovy/pull/1606
>


-- 
Guillaume Laforge
Apache Groovy committer
Developer Advocate @ Google Cloud Platform

Blog: http://glaforge.appspot.com/
Twitter: @glaforge <http://twitter.com/glaforge>

Reply via email to