SIP: Make classes `sealed` by default

If this sip is implemented it means very simple things to me.

  • I will have to make more forks.
  • I will have to make more wraps around java instead of using scala libraries.

I will recieve nothing for increasing cost to create productive environment for my company. It will make migration to dotty more difficult.

It is good way to decrease binary incompatibility problem isn’t it…

2 Likes

I agree that it’s sometimes useful to extend a library class and override some of the methods in ways that were not foreseen by the library author. I’ve done it myself several times.

However, I think this is a bad habit, because it makes your code tightly coupled with code you have no control over, which may change in breaking ways in future versions.

So making a fork of the library, or even copying the offending class into your own project (under a different package), if it’s not too big, seem like better approaches. It means you now have full control on how that part of the code evolves, and you can freely adapt it to your needs. The latter approach works best if the library is organized in a modular way, which is hopefully the case of the average Scala library.

Still, there is value in quick and dirty solutions to Get Things Done, as long as it’s blatantly clear that it’s a temporary hack (so it could be rejected easily from clean code bases). I’d like to have an annotation named @disregardSourceRestrictions that allows one to extend final and sealed classes or methods and to access private members, perhaps only enabled under a compiler flag.

8 Likes

Not in my case. I like to extend production classes in test code. I have full control over both (production class and test one). OTOH I can add open or whatever in that case, but I would rather want the option to enable openess wholesale in e.g. whole test code.

1 Like

I have to STRONGLY disagree. Effort of spinning up a repo, jenkins job for publishing and then keeping fork in sync is so huge I almost never do it. For me its either monkey-patching or not using the library at all.

5 Likes

Prioritization issues aside, my opinion of the current proposal is that it’s probably currently too contentious to be pushed into the language in any short timescale. Clearly lots of people enjoy the ability to unilaterally “patch” classes they use via subclasses, even if only in narrow circumstances like tweaking misbehaving third-party libraries or injecting hooks into test code.

For myself personally, my work codebase does this a reasonable amount, even if we generally try to avoid it. Generally if we can’t “patch” things via inheritance, we’re forced to mangle source code and maintain patch files or branches, both of which are a huge pain. We generally don’t inherit/override concrete classes much in “normal” circumstances, so the fact that you can doesn’t really cause us any hardship or mess

What about if we were less ambitious: we only make case classes sealed/final by default, and we make the sealed modifier transitive to automatically apply to subclasses? That would fix one long-standing wart while also likely not cause any migration pain, since people generally treat case classes and sealed hierarchies as final anyway (even if they forget to seal everything as necessary to enforce that)

11 Likes

@LPTK

Still, there is value in quick and dirty solutions to Get Things Done, as long as it’s blatantly clear that it’s a temporary hack (so it could be rejected easily from clean code bases). I’d like to have an annotation named @disregardSourceRestrictions that allows one to extend final and sealed classes or methods and to access private members, perhaps only enabled under a compiler flag.

I like the general idea. In the previous thread Make concrete classes final by default, it was stated that one can distinguish three possible states when writing a class.

  1. You do intend that the class can be extended. This means you have to carefully work out the internal contract for each overridable method. You communicate this by making the class open.

  2. You forbid that the class is extended. You communicate this by making the class final.

  3. You have not made a firm decision. The class is not a priori intended for extensions, but if others find it useful to extend, let them go ahead. However, they are on their own in this case. There is no documented internal contract, and future versions of the class might break the extensions (by rearranging internal call patterns, for instance).

Right now, (1) and (3) are not distinguished. The current SIP proposal could change this by using open for (1), final for (2) and no modifier for (3).

Now, the question is, what should happen on the user side if somebody does extend a class of the third category? There should be some form of indication that this is a risky operation from the standpoint of software evolution. It strikes me that we could use a language import for that. E.g.

import language.adhocExtensions

class C extends otherPackage.D

This achieves several things:

  • Library users can still write ad-hoc extensions, like they do today, but they have to opt-in with the
    language import.
  • Mocking works without warnings. Just set the language import in your mocks.
  • A simple grep checks whether a codebase is “clean”, i.e. that it does not use ad-hoc extensions.
  • Library writers are protected: If they don’t make a class open, no internal contract is assumed and they are free to change implementations.
  • Migration headaches go away. If a library chooses to keep a class C non-open then code extending C has to add a language import. That’s a simple user-side operation. No coordination between different code bases is needed. If C is actually intended to allow extensions, the library writers will be lobbied to make it open, which would also be a good opportunity to make sure the internal contract is well documented.

So, it looks to me we have something which would be very easy to implement and would likely produce a helpful social dynamic for the interactions of library writers and users. The scheme also follows Scala’s philosophy to always provide an escape-hatch for the cases where static checking is too restrictive.

With this revised proposal, what is the role of sealed? sealed is still needed because it makes it an error to extend a class in another compilation unit instead of just requiring a language annotation to do it. But as far as pattern matching is concerned, default classes should be treated like sealed classes: we know all their planned extensions so we can do exhaustivity analysis based on this.

10 Likes

sealed can be extended as proposed here: Pre-SIP: sealed enumerating allowed sub-types

If you never extend a normal class than you probably shouldn’t be using a normal class. case class definitely makes sense to not be extended, but a normal class being used in the Java OOP sense are designed in such a way that being open as a default makes sense.

In general I don’t think this is a great idea, the only data structures that should be non extensible by default are case class's, because it really doesn’t make any sense to make the data structure open.

Its also too big of a change for Scala3, Scala3 already having enough changes.

Mm that doesn’t make much sense to me. A case class is like an OOP record with equals, hashcode, copy, etc. If a class doesn’t need those there is also no need to make it a case class. I use classes for ‘services’, not for data, and rarely need to extend a service with more functionality, so they are almost always final.

If we take the time to do this it shouldn’t be that disruptive. If you first deprecate the current behavior before disabling it.

2 Likes

I agree with the proposed change, but I am not sure I agree with this line of thinking. Scala 3 will not be perfect and not the last breaking change of Scala. And if it will, it is likely to stay behind. Breaking code is on the long term inevitable if you want to get rid of warts / not so great design decisions. As long as there is a clear migration path & enough time to migrate it should be fine.

So even if this preSIP will not make the Scala 3 cut, I think it would be quite a loss to not consider it ever again.

1 Like

It seems that’s precisely the kind of situations where you’d be advised to use the escape hatch that I proposed — or the proposed adhocExtensions — which you would enable for the whole test project.

1 Like

And thats the point, I also use classes for clients to services (I assume thats what you mean), but in this case I find myself having to extend such clients on many occasions (i.e. mocks is a classic example, but also when testing corner cases)

I guess we differ in style then, because we use classes the same, but not the way to test them.

I still think it makes sense to be explicit when something is meant to be extended, and I would like to default to the one with the least amount of power, thus final (or sealed) instead of open.

The suggestion LPTK makes is probably best of both words. You default to the one with the least amount of power, but still have the ability to break free from that convention for whatever reason you deem necessary.

2 Likes

I guess my point is, the whole point of classes is the open extensibility paradigm that comes with OOP, and they are primarily an OOP mechanism. Defaults should convery the intent of how the abstraction is meant to be used and the paradigm that comes with it, and when you use class the way its meant to be used you actually do find yourself needing to extend them fairly often (this is also a side effect that classes are often used to couple state with data, and since the state is coupled with data, inheritence is the primary way of extending functionality).

tl;dr is, if you are using classes the way they are designed to be used (in the Java OOP sense) then extended this is intended and should be classified as default behaviour. This is even more important if you write libraries (as opposed to applications) where the number of instance that you need to extend a class increase even more (as others have commented the only way around this is to manage manual forks)

1 Like

I guess we have to agree to disagree. Yes, extending classes is very OOP in Java. But I would not say it is recommended in Scala. OOP in Scala is much more about using objects as modules, not as containers of mutable state that can be extended indefinitely. At least, that is how I see it used often.

Patching a library should be quite rare I would say. And even if it is necessary, it makes more sense to use an escape hatch rather than have it be the default behavior available to you.

3 Likes

Oh of course, but thats why Scala has different abstractions for this. For example I strongly believe that case class should be final (and we can go even further if possible), but if you are using class’s you are already implying that you are coding in the Java/OOP style.

At least in non trivial applications, its definitely not rare (from my experience).

That’s just plain not true – you really should stop casually asserting it. Java/OOP style is fundamentally mutable in nature, and there are plenty of us who use classes for other purposes…

7 Likes

I have worked on Kotlin where classes are final by default. If you are going to mock your classes, you end up opening them all.

2 Likes

Inheritance is not particularly unsafe when applied to a carefully-designed API that is intended for it. (E.g. pretty much anything that supplies a default implementation for a visitor pattern.)

But I agree with your characterization for the (seemingly?) most common style used in Scala.

Note that lack of a MyType poses a considerably larger safety (and usability) barrier for the most common style than does allowing open inheritance by default. Dropping sealed here and there really doesn’t impede reading very much. Dropping [A <: Foo[A]] { self: A => all over the place is a lot more awkward, a lot harder to understand for novice/intermediate users (and it isn’t even perfectly safe). People don’t extend immutable stuff because it doesn’t work anyway–you learn this pretty fast, though, yeah, it would be nicer as a new user if you didn’t have to (and nicer as a library author if it was just taken care of, not something you had to do explicitly).

So I sort of technically agree, but I don’t think on the scale of things it’s worth the likely disruption. And I think the disruption would be substantial: library X recompiles successfully for the new version, changing something from innocuously inheritable to not inheritable, and some fraction of downstream users’ code ends up completely broken, requiring a major investment of manpower to fix (fork library, or submit PRs and discussion, at which point there are probably bincompat issues as well, or refactor code to (try to) work around the issue).

2 Likes

This saves a developer from writing one word. From the other side it makes class’s properties more difficult to track.

2 Likes