+ implicit_reopen

[pq]

UPDATE: ready to land! 🦅

Fixes: #3920.

---

This is a WIP but far enough along that I could benefit from some feedback.

Specifically:

1. @kallentu, @eernstg, @lrhn: could you look at the test cases and tell me what I've missed? (I was working off the excellent tables in [Class Modifiers] Overlap in lint and base/final subtyping restriction language#2871 but I'm sure some of the induction cases are going untested.)
2. @MaryaBelanger, @bwilkerson: the docs are light and could use massaging minimally although ultimately I do think we want to link out to some place more authoritative; could you advise?

General comments welcome too. (I'm less concerned w/ implementation at this point and more on capturing the right test cases FWIW but happy to receive any feedback at all.)

Thanks in advance!

[coveralls]

Coverage: 96.325% (+0.02%) from 96.306% when pulling bdb1539 on implicit_reopen into 3049702 on main.

[bwilkerson]

One minor suggestion, but I thought the description was good.

I didn't look at the implementation.

[srawlins]

Just drive-bys.

[eernstg]

Commenting on the test cases:

I agree with @kallentu that it would be good to have cases where there is more than one step of implicit provision of class modifiers. E.g., class A is interface, sealed class B extends A is then implicitly interface, and and so es sealed class C extends B.

Similarly, being implicitly interface can propagate through any number of mixin applications (and that case also does not seem to be covered):

interface mixin M1 {}
mixin M2 {}
...
mixin Mk {}

sealed class C with M1, ... Mk {} // Implicitly `interface`.

@kallentu also mentioned the following case (and I don't think it's covered yet), which is the second half of the first rule in my ruleset:

interface class S {}
base class I {}
sealed class A extends S implements I {} // Implicitly `final`.

A couple of other cases I couldn't spot are these ones (that's the second rule):

base class I {}
sealed class A implements I {} // Implicitly `base`.

final class I2 {}
sealed class A2 implements I2 {} // Implicitly `base`.

sealed mixin /* or mixin class */ M implements I {} // Implicitly `base`.
sealed mixin /* or mixin class */ M2 implements I2 {} // Implicitly `base`.
sealed mixin on I {} // Implicitly `base`.
sealed mixin on I2 {} // Implicitly `base`.

[pq]

Source and tests updated. Nothing will pass until a new analyzer is released so still WIP.

[pq]

@eernstg, @kallentu : I added a handful of tests but would love for you to take a look... It looks like some of the cases we discussed above have been recently disallowed and will produce analyzer diagnostics (and so not a lint).

If you see negative tests I should implement please let me know too. I'd much rather under than over report!

[pq]

(And needless to say, thank you all for your patience and perseverance. I think we're closing in!)

[pq]

@bwilkerson, @kallentu, @eernstg: polite ping for a review... Hoping to get this in before the cut off. 🏁

[kallentu]

I've exhausted my brain for test cases (which you've added already) so I'll leave whatever is missing to Erik.

I think this is how I imagined the initial implementation for the lint -- LGTM

[eernstg]

Oops, I missed this one. I'll take a look within the next couple of hours.

[pq]

No worries, @eernstg. Thanks so much!

[eernstg]

LGTM! Left a few comments.

[pq]

Thanks again to everyone for all the feedback!