Maybe I don’t know the full scala syntax then! How would you scope this import to a top level class declaration?
Oh yes for a top-level class declaration you’re a bit out of luck. Depending on the API guarantees that you then offer to your users, the following workaround may help:
package your.pack
import someone.elses.NonOpenClass
object NonOpenClassExtensions {
import scala.language.adhocExtensions
class OpenTheNonOpenClass extends NonOpenClass
}
class MyExtension extends NonOpenClassExtensions.OpenTheNonOpenClass
Can it? Can dotc --from-tasty foo.tasty
v3.4 fail to compile where with v3.3 it compiled?
I very much like the feature by the way!
No recompiling from tasty cannot turn a non-error into an error. But recompiling from source can.
It feels to me that this is a departure from Scala as I know it. Until now, by default, things are public and open, and you have to think about restricting your API. Now you have to think about opening as well.
The fact that the default class C
easily leads to warnings seems quite annoying, and gives people the feeling that they’re doing something wrong. In the REPL a special case would be needed, otherwise writing class C
and then class D extends C
on the next line would warn.
One aspect that bothers me is that the proposal changes the semantics of existing code. When upgrading, a library author needs to go through all the code and make sure he doesn’t miss marking the right (or all non-abstract/sealed/final) classes open
, otherwise clients break.
The feature is also problematic for cross-building with 2.13 – should it be disabled in Scala2-mode?
Since we are in the context of restricting subclassing: was it considered to make sealed
transitive (propsed by @lihaoyi on the old thread)?
One consideration is that we already have a concept of “classes to be extended”: these are classes with abstract methods, and implementing abstract methods thus doesn’t require any annotation at all.
Furthermore, we already have a concept of “things that may not be intended for extension, but you can override methods with an annotation”: that is what the override
keyword is for!
This proposal would be adding a third level of open-ness, moving the current "override
keyword" to "override
keyword with language import", and adding a “Needs override
but doesn’t need language import” level of open-ness that you can mark a class with via open
.
I also think that adding another language-import/warning will not have any significant impact on how people write Scala. I don’t think import scala.language.experimental.macros
has in any way dissuaded people from writing macros, nor has import scala.language.higherKinds
dissuaded people from using higher kinded types. My fear is that open
will likely turn into just another nuisance warning that people ignore in the compile logs, and another nuisance Cmd-Enter
auto-fix ritual for your IDE to insert a language import, when everyone (you, your compiler, your editor) all already know what you want.
From that perspective, three levels of open-ness rather than two is a bit more granularity and a bit more control, but I don’t think it crosses the bar as something useful enough that it needs to go into the language.
This difference is justified by the fact that an extension contract is much more difficult to think about, and is often not thought about at all, than an external contract. Especially in Scala where most things are immutable, external contracts are easy to specify.
And more importantly, if you screw up the external contract of a method, you can deprecate it, leave it there for compatibility and then not think about it anymore. You cannot do that with the extension contract of a class. You cannot deprecate the problematic pieces and then not think about them. You have to keep them in mind forever, and always call them with the right “timing”, in order not to break compatibility.
This is why an external contract can be public by default, whereas an extension contract should only be opened if it has specifically been designed and thought through as such.
We can probably disable it in Scala 2 mode. We could also introduce a no-op open
modifier in 2.13.x, to ease cross-compilation.
The problem with that is that there are legitimate use cases for sealing only one level, although I will admit they are pretty rare. We would need an escape hatch to mark a subclass of a sealed class as non-sealed… Oh wait, that’s exactly what open
means! I like this
Maybe a middle ground approach. Disable this feature by default and allow the library author to opt-in via -YclassesClosedByDefault
(so no need for special Scala 2 options here). So if I’m a library author who doesn’t want to forget final
by mistake I can just turn on this flag.
Regarding case classes though I think they should be final by default without any exception.
For me the primary purpose of sealed
is for folds. As I’ve suggested else where I’d prefer a self type that guarantees that the final class / object inherits from a set of traits/ classes. So I feel sealed
is already unnecessarily restrictive.
I don’t like the current proposal at all. Hasn’t Scala got enough of these irritating imports. I think they’re irritating for many experienced Scala programmers, but can be downright confusing for less experienced Scala programmers. I know I was thrown by them in the past. They seem to me to go against the whole principle of scalable language, where you can gradually increase the sophistication of your code. At the very least I’d like to hear from some developers who actually found the warnings useful.
We seem to have swung from one extreme to another. For years change in Scala has been frustratingly slow. For example trait parameters, an actually useful feature, is listed as accepted in June 2015, but still hasn’t been implemented. Now it seems like everything must be debated agreed and changed in a matter of weeks. Why can’t we make case classes final by default. Introduce the open keyword, but make the warnings for extending non explicit-open classes opt in rather than opt out?
If a class was intended to be open before the upgrade, the library would have tests that test the extension and override scenarios. Those tests would break during the upgrade (since the tests are not in the same file) so the maintainer would know to add the open
modifier too their class before releasing anything.
If the library didn’t have tests that extend the class, then it’s clear it wasn’t intended to be extended, and it is the whole point of this proposal to cause warnings when trying to extend such a class!
I disagree with this interpretation. There are a number cases of classes that have 0 abstract method but that are still intended for extension and where the overriding contracts are well-defined. In fact most of my truly open classes (the ones I would add open
to) are concrete.
If override
were the legitimate way of saying “I’m using an escape hatch now to patch that library which did not cater to my use cases”, then we should be asking for a language import every time someone uses the override
keyword!
The override
keyword itself is way too natural to communicate the dangerousness of what one is doing. So it clearly does not solve the issues presented in the motivation.
But most of us don’t know what we want! We don’t understand that when we extend a class that was not meant too be extended, and designed and tested as such, we are entering dangerous waters and expose ourselves to breakages that the library authors might not even think about. That’s the whole point of this proposal!
Hopefully we can convince the IntelliJ team not to implement such an “auto-fix”, because adding this language import is supposed to be a hack, and you should carefully think about it, instead of using it as a rule.
@soronpo This is once again the second Alternative. (Because you’d still need an open
modifier for the rare intentionally open classes within the library codebase.)
I’m curious what the number of classes that fall into this category is; I have hardly written any in my open source libraries, except for things like Mill build configurations, even though I purposely leave stuff non-final or non-private to override/use “at your own risk”. For the few non-Mill-build related concrete overrides, those are always one or two fields with default values you can set, rather than an entire class of members meant to be over-ridden ad-hoc.
Perhaps open
should apply to individual members, instead of classes?
That would be an interesting way of indicating “these are the things to override”, instead of doing the reverse and marking all the things you can’t override final and having the user scan for non-final things
Is this really true though? Among most of the people I’ve worked with, both commercially and open-source, they already know that extending some random library class and overriding arbitrary methods is a pretty sketchy and dangerous way of doing things. If these folks can get by without extending library classes, they already would, but sometimes they can’t.
Even beyond the Scala community, “composition instead of inheritance” is a mantra, and “fragile base class” is an antipattern, so I find it hard to believe that there’s a whole lot of people in the Scala community wildly extending/overriding concrete class members with reckless abandon.
That’s a reasonable thing to hope for, but empirically the track record of such efforts isn’t great
And I don’t blame the intellij folks at all. Among everyone people i’ve ever worked with, I haven’t met anyone who writes a macro unless they really need to, used a higher-kinded type unless they really need to, or extends/override class members unless they really need to.
For all these folks, language imports are just a weird dance to do that provides no value at all. IntelliJ providing auto-fixes isn’t the problem, but a symptom of the low value these language imports provide at guiding user behavior. A sort of Desire Path, if you will, and such paths should be really properly paved rather than fenced off.
I also think this is better suited as an opt-in lint warning for authors, with @open
as an annotation instead of a keyword.
Scala (the ScalableLanguage) also wants to be a lightweight language that allows you to just “code away” with minimal barriers. This feature immediatly pops up when prototyping something, or when getting started as a beginner, and draws your attention.
I’m warming up to the idea of an @open
annotation on methods intended for extension, or alternatively on whole classes if all their non-final
methods are. The rest of the proposal would still hold as is.
This has already been discussed at length over two discussions in this forum; there were no clear conclusions other than the realization that the topic is heavily controversial, and so do this proposal and its motivation. The language should not include such a core feature – tied with its own keyword – if its so controversial. On the other hand, an @open
annotation with an opt-in compiler warning? Sure, why not.
Furthermore, I find major flaws in the logic and wording of the motivation:
When writing a class, there are three possible expectations of extensibility:
- The class is intended to allow extensions.
- Extensions of the class are forbidden.
- There is no firm decision either way.
The three cases are clearly distinguished by using
open
for (1),final
for (2) and no modifier for (3).
(1) is clearly distinguished from (3) by using either trait
or abstract
.
It is good practice to avoid ad-hoc extensions in a code base, since they tend to lead to fragile systems that are hard to evolve.
This is clearly an odd statement given that implicit
is such a major feature in the language, used heavily to provide just that – ad-hoc extensions via type classes and extension methods.
I’m not really sold on open
, so I don’t really have any skin in this, but this doesn’t really make sense to me as a counter argument.
Extension methods and typeclasses can only combine the various methods of a class’s external API and can’t be used to override methods, so I don’t think it’s valid to compare them with extending a class or overriding it’s methods.
I’m not sure whether it’s a counter-argument, a comment about the particular wording, or both.
“ad-hoc extensions” is not a term I’m familiar with, but I am familiar with ad-hoc polymorphism, which type classes are a form of.
While I agree that this mechanism can be useful in some scenarios, I’m concerned that it might:
- subvert expectations
- add unnecessary inconvenience
I believe that the following idiom is very common in many programming languages:
class A
class B extends A
and that the common expectation is that this has the same meaning independent of whether A
and B
are defined in the same file.
Introducing the open
mechanism would break this expectation. In other words, a feature that might be useful in certain advanced usages of class hierarchies, would introduce a non-optional restriction that might confuse user coming from other languages, and inconvience users who have no need for the feature. This is in contrast to other modifiers such as protected
and final
which adds a restriction when present, not when absent.
Additionally, one should remember that programming is not an homogenous art - there is a wide variety of programming domains, and different programming techniques will be appropriate for different domains. As such I object to the following generalization (from the “Motivation” section):
The class is intended to allow extensions. This means one should expect a carefully worked out and documented extension contract for the class.
In a low-level library consumed by hundreds of users, yes, perhaps. But in simple scenarios like e.g. modules for code re-use and organizational purposes
class CommonInvoiceOps {
// methods
}
object AbcInvoiceMgr extends CommonInvoiceOps {
// more methods
}
object XyzInvoiceMgr extends CommonInvoiceOps {
// more methods
}
I would certainly not expect a “carefully worked out and documented extension contract for the class”.
As each of the classes above grows larger, it might make sense, simply for maintability purposes, to move them to seperate files. Having to add open
on the base class just because it is in a different file would imply a significance that is simply not there.
So, in my opinion:
At the least, the mechanism should work on the package level and not the file level.
(On a related note, I would really like sealed
to be package-level as well. Actually, I think the concept of “file” should not have any semantic meaning in a program at all.)