- SIP public review: Open classes
- Make concrete classes final by default
- SIP: Make classes
I think it makes sense to introduce
closed as new extensibility modifier and kind of reactivate the intended behaviour of
open. Yet, I suggest modifications to the current documentation.
IMO this proposal would achieve the following points
- open closed principle by default, opt-out if required
- also on a member level (ability to use
openon member level)
- covers part of Implement keyword (instead of override)
I suggest to introduce a new extensibility modifier
closed next to
open which will lead to 3 main states of extensibility:
final=> cannot be extended from and methods/fields without extensibility modifier are thus implicitly
closed=> is not intended to be extended from but one can (how will be outlined in section “extend closed types” below).
open=> is intended to be extended
The current documentation states that omitting the extensibility modifier equals to
closed. IMO it is better to have an explicit modifier as it allows to re-interpret the omitted modifier differently (see suggestion for finalFirst in QA). Nonetheless, I also propose to use
closed as default if an extensibility modifier is omitted.
open on methods/fields/inner type produces currently an error. Yet, omitting the extensibility modifier on a method/field/type results currently in
I propose that one can use
closed on the member level in the same way one can use
final. Moreover, I suggest that omitting the extensibility modifier on member level is the same as using
closed (also for inner
types). As consequence, I suggest that one can use
open modifier for members to open them up.
abstract members (including inner
traits) are implicitly
open and I suggest we generate a warning if one specifies both modifiers (e.g.
open abstract def) as we do already for
open abstract class.
This leads to the following behaviour
final=> this method/field/type cannot be overriden in a sub-type (this inner class/trait cannot be extended from)
closed=> this method is not intended to be overriden but one can (how will be outlined in section “override closed method” further below)
abstract=> this method can be overriden and it is intended to do so (e.g.
open overridein case of the class adapter pattern).
I am aware of that this is quite a drastic change and I can imagine that many don’t feel comfortable with going through all libraries which one wants to release with the new scala version containing this change. Thus, I suggest to introduce an annotation on class/trait level:
@memberExtensibility with values
"open" which has the effect, that members without extensibility modifier are either
open instead of
closed (members of inner classes/traits kind of inherit the behaviour). This way the migration should be easy IMO (regex search and replace) .
The extensibility modifier on member level would give the possibility, that one can:
- define that a member is
finalin a class / trait without
@memberExtensibilityor in a class / trait which has
- define that a member is
closedin a class / trait which has
- define that a member is
openin a class / trait without
@memberExtensibilityor in a class / trait which has
- omitting an extensibility modifier on a member results in the member being
- allow to set a memember to
finalwhich is already possible)
- introduce the ability to modify the default extensibility modifier via
@memberExtensibilityon the class/trait
- remove the current warnings regarding using
I would take a similar approach as outlined in the documentation but with slight modifications
Classes that are
closedcan still be extended, but only if at least one of two alternative conditions is met:
The extending class is in the same source file as the extended class. In this case, the extension is usually an internal implementation matter.
The language feature
adhocExtensionsis enabled for the extending class. This is typically enabled by an import clause in the source file of the extension:
Alternatively, the feature can be enabled by the compiler option
The doc states:
If the feature is not enabled, the compiler will issue a “feature” warning.
the feature warning for ad-hoc extensions is produced only under
-source future. It will be produced by default from Scala 3.1 on.
I don’t know what the difference is between a feature warning and a regular warning. I guess it is related to
-source future. If this is the case then I suggest we still emit a feature warning in 3.1.x (even though the documentation states the compiler will emit a regular warning in 3.1) and a regular warning starting from 3.2 or later.
Yet, I propose that we do not emit a warning if the feature is absent but an error because warnings can easily be overlooked and IMO it should be opt-out, not opt-in. If one has enabled the language feature
adhocExtensions then I would still emit a specific warning for extending a
closed class. One can suppress the warning per class, per file or even globally (e.g. for test sources). Maybe I am just not aware of it, in case a language feature can somehow be enabled on a top level class (and not for the whole file), then we would not need to emit a warning.
- compile error when extending a
- specific feature warning if
adhocExtensionsis enabled in 3.1 (only if
- specific regular warning if
adhocExtensionsis enabled in 3.x
Members that are
closed can still be overriden (extended from in case of an inner class/trait) but only if the
adhocExtensions feature is enabled in the corresponding file or globally. If the language feature is enabled, then a specific feature warning (a different one than the one we emit when extending a
closed class) is emitted in 3.1 and a regular warning in 3.2 or later
One can suppress such a warning per member, per class, per file or even globally.
Personally, I see the point that
override is not the best chosen name in case a sub-type implements an
abstract member. This proposal does not address this point. Neither that omitting
override in a sub-trait in case it implements an
abstract member does not lead to an error. However, since omitting an extensibility modifier on member level makes it
closed we cover at least part of it (see
compile error if adhocExtensions below).
trait A final def foo1() final def foo2() def bar1() def bar2() closed def bar3() closed def bar4() open def bar5() abstract def zulu1() abstract def zulu2() abstract def zulu3() abstract def zulu4() abstract def zulu5() trait B extends A : def foo1() // compile error, foo1 is final override foo1() // compile error, foo2 is final final def bar1() // compile error if adhocExtensions is not in scope otherwise warning (no warning about missing override though) def bar2() // compile error if adhocExtensions is not in scope otherwise warning override def bar3() // compile error if adhocExtensions is not in scope otherwise warning closed override def bar4() // compile error if adhocExtensions is not in scope otherwise warning open override def bar5() // OK, as bar5 is open in B final def zulu1() // OK (no warning about missing override) def zulu2() // OK (no warning about missing override) override def zulu3() // OK closed override def zulu4() // OK open override def zulu4() // OK trait C extends B : def bar1() // compile error, bar1 is final in B def bar2() // compile error if adhocExtensions is not in scope otherwise warning, bar 2 is closed (extensibility modifier missing in B) -- no warning about missing override override def bar3() // compile error if adhocExtensions is not in scope otherwise warning override def bar4() // OK def bar5() // OK (no warning about missing override) override def zulu1() // compile error, zulu1 is final in B def zulu2() // compile error if adhocExtensions is not in scope otherwise warning (no warning about missing override though) override def zulu3() // compile error if adhocExtensions is not in scope otherwise warning override def zulu4() // OK
I see from the previous discussions, that a final first approach will not have many advocates, thus I propose a
closed-first approach instead as already proposed by the documentation.
Personally, I think it makes sense in application code to take a
final first approach (i.a. to mitigate security holes), meaning that if one omits an extensibility modifier on a class then it should not be implicitly
final. Same goes for members which are currently implicitly
One could argue that
closed is kind of
final if no
adhocExtensions is present but the resulting jvm byte code is still vulnerable to attacks using non-final classes/methods as attack vector. In times of increasing supply-chain attacks, we think it would be better if we took a security-first approach and thus use
final in case one omits an extensibility modifier (for classes as well as for members).
closed-first approach might be a better fit for libraries sometimes but I suggest to have “end-users”, in the sense of application code writers, as first target audience of scala and library authors as second target. I think even in libraries it’s most of the time better to use
final per default and only open up if really required/requested. I am aware that this is controversial in the scala community but I believe that if a library author prefers
open classes/traits, then it’s not much work to add the corresponding modifier to the class/trait, use
Yet, I guess this proposal would lose ground if I really go with
final-first because I saw from the previous discussions, that a final first approach will not have many advocates, thus I propose a
closed-first approach instead as suggested in the documentaiton.
Nevertheless, I am
open (pun intended) to add a language feature flag
finalFirst – or maybe something like
-security:finalFirst on (with default
If the security feature is not in scope, then the default extensibility modifier for a class/member without one is
closed as already advertised in the docs.
Only part of it. You still want (or accidentality) expose concrete classes and those should be
final per default.
Also, it seems to me that the scala ecosystem (still supporting jdk 8) is not yet fully ready for jvm modules.
- Kotlin uses
finalfirst on class level but not for members
- C# uses final first on member level but not on class level
=> IMO scala can do it even better and combine both approaches
…in the sense of, if they want to release their libraries against the scala version including this change, then they need to go through all libraries and have a look at each class and each member and re-decide if it should be
open-ed up again or stay
Personally I don’t think so, the migration path is smooth:
- if you don’t want to spend time (which is fair enough), just add
opento existing classes. This can easily be done with regex search and replace. Same same for adding
@memberExtensibility("open")to classes and traits
- you can (but don’t have to) step by step transition away from using
finalat your own pace.
One can use
adhocExtensions globally for test sources as already advertised in the documentation.
In case we should go with
final-first, then IMO it is not really difficult but less easy. IMO one should not mock a class but its
abstract class or
trait. And in case the implementation which one wants to replace in the mock is in the concrete class then it is easy to use scala 3’s
export capabilities (I guess this feature did not yet exist when the discussion about
final by default was held). If there isn’t any contract (abstract class or trait) but just the concrete class and one does not own the code, then one is out of luck. Personally, I cannot remember when I had the need to mock a concrete class without contract.
I think more or less the same arguments as for mocking applies.
… also if we should go with a
If you use a concrete class of a library and you can patch it up currently and pass the sub-class instead of the library class to some method, then in all cases where I had this desire, the method does not expect the concrete class but an
abstract class or
trait. i.e. patching up just means implement the
abstract class or
trait use the concrete class as composite and delegate everything (with scala 3’s
export mechanism) to the concrete class except the methods you want to patch.
If we go with the
closed-first approach, then you can still use the
adhocExtenions feature. If we go with the final-first thought, then you are out of luck. But honestly, if someone defines a method
protected in a concrete class then this person will most likely use
closed for this method as well once this change is implemented.
I am new to scala 3 so might be I don’t have the full picture. I guess it should be possible to re-compile a library based on the tasty byte code included in the jar of the library. If a tool (lets call it “monkey patcher”) allows to modify the tasty byte code in such a way that one can modify the extensibility modifiers before compilation, then this would be an alternative way which would still allow that one can patch up also a final class (of course you loose things like singed jars and the monkey patcher would need to be crafted carefully not to become an own source for security holes). Btw. I think a monkey patcher could also be interesting for the opposite way: kind of harden a dependency, set all classes/members to
final with some exceptions (the ones we want to override).
Indeed, and I guess I won’t have the time to implement it alongside with this SIP. As a middle ground between
closed-first, we could introduce a 4th extensibility state. If one omits an extensibility modifier, then it is quasi-final. The class as such will behave like
closed (i.e. one can still extend it via
adhocExtensions) but it is basically syntactic sugar for creating two classes under the hood:
- abstract class => this class will contain all definitions and will be used everywhere the class is used as type (so that passing another sub-class works)
- final class => this class is empty (
final class Xy extends _BASE_Xy) is used to instantiate the class
E.g. if one sub-classes a quasi-final class via
class Xy extends Foo then it actually sub-classes the abstract-class. For the scala user, this should be transparent (setting reflection aside).
I am sure this is not well thought through and certainly a pain to implement but maybe gives someone else a hint for even a better idea
See previous discussion SIP: Make classes
sealed by default . I guess the core team has dropped the idea mainly because mocking and patching isn’t easy, but a core team member would have to respond to this question.
Since that discussion, Java’s sealed classes/interfaces have landed as full feature in jdk 17 and I don’t know if there are plans to map Scala’s sealed classes to Java’s on a byte code level. Java’s sealed classes are final and a sealed class is always abstract in contrast to scala where we can have a concrete sealed class.
There are several points which speak for this IMO:
- basically the same arguments as for classes apply for members. If you open up a class for extension via adhocExtensions then you most likely do this for some specific reason (setting mocking aside) and you don’t want that someone (including yourself) in the future override stuff you did not intend to extend adhoc without a warning.
- it makes sense to have a symmetry between class and member modifiers as it is easier to remember/learn
- not being able to state that a member is open even though it implicitly is indeed, make some people nervous
Possible improvements for the future (not yet thought through):
- allow to widen
closed[package]as it is very common in a project making use of jvm modules to put the concrete implementation in an sub-package
implso that one does not need to expose it via module-info. For instance, put an abstract class in package com.example. this abstract class defines a default implementation marked with
closed[com.example.impl]and in the impl package there are two implementations where one of them
overrides the method. This override would not need the
adhocExtensionsfeature in scope due to the package definition in
- allow to widen the
closed(and sealed could follow) with a
permitsclause similar to Java’s sealed class allowing that certain classes outside of the file and even in a different package hierarchy are allowed to extend the class without the need to use
- I see Implement keyword (instead of override) as a good supplement including emitting an error if one overrides without