dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

Non-interface class declarations. #704

Open lrhn opened 4 years ago

lrhn commented 4 years ago

As a simple approach to sealing classes (#349), I propose a minimal, yet powerful, feature: Non-interface classes.

Background

Dart classes are "Kotlin open" classes and interfaces. It means that anyone can implement the interface of a class and extend the class—at least if it has a public generative constructor.

That may sometimes be more affordance than desired. There are requests for "Sealed" classes intended to prevent other users from implementing or extending a class. The reasons given for that feature is to control dependencies, so that additions are not breaking changes, and to enable performance improvements when compilers can locally deduce properties of all implementations of a type.

Proposal: Non-Interface struct Classes

Introduce struct as a modifier on class and mixin declarations, struct class and struct mixin, which makes the class name not denote an interface. It still introduces a type, and a class/mixin, but not an interface name.

The class has an "interface" in the sense of a set of implemented types and a mapping from member names to member signatures (which is what the language uses to see if a member access is allowed), but the name will not be usable in implements clauses.

A struct class can be extended as long as it has a public generative constructor. Such a subclass of a struct class must also be a struct class.

If a struct mixin is applied in on any allowable super-type, the result is a struct class.

We allow a lone struct to work as a shorthand for struct class whenever struct is not immediately followed by class or mixin. This reduces the extra typing when declaring a struct class.

Examples

If we combine this with improved default constructors (#469), then you can declare a simple data class as:

struct Point {
  final int x, y;
}

It's possible to extend the class, but not implement it.

A properly sealed class would be:

struct Point {
  final int x, y;
  Point._(this.x, this.y);
  factory Point(int x, int y) = Point._;
}

Maybe we can even find a way to avoid the intermediate constructor, say factory Point._(this.x, this.y); would be a generative constructor which cannot be used as such. (Probably not the best syntax, but something to start from).

Benefits

The benefit of a non-interface class is that it is guaranteed that all objects matching the type will also extend the class (or mix in the mixin). That ensures that private members are available, and that invariants ensured by the constructor are maintained.

Together with only having a private generative constructor, this ensures that nobody else can implement the class.

If it's possible to restrict the availability of a subclass with a public generative constructor, say to the same package using package-local libraries and not exporting the subclass from a public library, it allows a class that can only be extended locally in the same package.

To ensure that specific methods are not changed, we will also need to be able to make methods final (non-overridable).

Drawbacks

A class that doesn't have an implementable interface, cannot be mocked. It also cannot be implemented for any other reason, which is a restriction compared to the flexibility provided by all current Dart classes.

Changing a class into a struct class is a breaking change. This makes it impossible to apply the struct to existing platform classes when the feature is implemented. Packages can increment their major version number and add struct where they prefer to have it. For clients who are not depending on implementing the package's interface, it's free to update to the new version, and everybody else gets a fair warning where things are going.

eernstg commented 4 years ago

Looks good!

At first it seems a bit confusing use struct to indicate that a given class cannot be used as an interface, also because that keyword might be used for things named struct in other languages (e.g., it is an inline allocated type in C#).

But I agree that the ability to control the affordances of a class C is useful, and preventing non-subclass subtypes will guarantee that any instance with type C will run a generative constructor body in C (so, being in control of those constructor bodies we can guarantee that an invariant violation concerned with final values is enforced).

Availability of private members could be enforced with a library scoped sealed directive as well, and it would be nice if we could ensure that this mechanism and sealedness are as orthogonal as possible (such that we don't end up having two or more subtly distinct ways to do the same thing).

So we should consider whether struct classes would be subsumed by a sealed class mechanism that we would wish to support at some point anyway?

icatalud commented 4 years ago

This is something that could be managed at the annotation level, through analyzer warnings in the same way Flutter does it for example with immutable classes. As a Dart developer I would not like to come across a class constrained like that, since it eliminates the possibility of being able to modify a behavior after carefully considering how the internals work.

OO programming attempts to achieve interface abstraction, which could be understood as “pass me an object with this interface and I will do the job”. Practice proves this abstraction does not satisfy all the use cases as sometimes it is required that an object not only has a certain interface, but the invocation of methods from the interface have side effects that interact with internals of a library or framework. In those cases it is required that the received object extends the object declaring the interface from the library itself. As well as extending sometimes it can be required that the values returned by methods from the extended class are exactly the ones returned by methods from the inherited class, something that could be expressed with a @mustReturnSuper.

Although annotations do not gives guarantees, I prefer much more the annotation approach over the “code constraining” approach, leaving the objects and interfaces versatility intact. Annotations pass the responsibility to the user to follow their directions for correct functioning. By doing so, the liberty to adapt the classes is still there in case users explored the internals of the library and had a clear understanding of what they are doing. As an example Flutter marks some classes as @immutable or methods as @protected. Those are very useful guidelines for using the framework, however after understanding the internals I modified the classes overlooking the annotations in a way that was convenient for what I was trying to achieve. It would be a huge limitation if for example Flutter decided to use classes like this, because it would prevent developers from creating their own side effects on top of the class. I would rather add @nonImplementable and @mustReturnSuper annotations to address this kind of application. In fact in a @nonImplementable class all methods could be defaulted to @mustReturnSuper (it’s only necessary to use the enclosing class annotation).

@ZakTaccardi is this what you were wishing for? From your proposal I see three things:

  1. Enum with actual values (constant)
  2. A class with a fixed number of subclasses that allow avoiding a "default error" in a switch statement.
  3. The "when" statement

Number 1 is essentially a class with constant static values of different types, where it is wished that all the static values types could be represented by the enclosing class. That could implemented as some kind of complex enum type where the constant values of the enum are unique for each object (they could have different classes).

In number 2 if annotations are used the default switch statement could also be avoided, because by using the classes in the way it’s expressed by the annotations, the behavior should be the expected one. Unexpected behavior could only occur if the annotations warnings are overlooked. Annotations perfectly satisfy the example you give, but it has the advantage that the freedom to modify something is still there.

The third is a regular switch statement with an automatic return (there is an open issue).

Levi-Lesches commented 3 years ago

I see this is an old issue, but I got here from the modules proposal. I agree with @icatalud: Dart should not focus on limiting devs, rather it should warn them when they may be making a mistake, while still allowing them to continue.

The most common reason I come across "non-implementable" classes is "adding a new member would be a breaking change". While I can sympathize with that, we should take a step back and remember who we're trying to protect. Avoiding breaking changes means we acknowledge that using implements is valid, and great care should be taken to not harm those who use it. To entirely remove the ability to implement a class would contradict that.

Instead, as is pointed out in the link above, most devs will write comments about what is intended and what's not, and then only focus on the "supported" breaking changes. This comment can be replaced with an annotation that warns users when they try to do something the author specifically doesn't support. But saying "the author didn't intend it" == "the API doesn't support it" is not always true, and therefore IMO Dart shouldn't be limiting devs in their (creative) use of APIs.

munificent commented 3 years ago

I agree with @icatalud: Dart should not focus on limiting devs, rather it should warn them when they may be making a mistake, while still allowing them to continue.

In software as in life, there's no perfect solution to liberty. When we give package consumers some liberty, we take agency away from package maintainers, and vice versa. Dart has historically leaned very heavily towards consumers of APIs being able to do what they want, but that comes with a real cost both for package maintainers and package consumers.

Avoiding breaking changes means we acknowledge that using implements is valid, and great care should be taken to not harm those who use it. To entirely remove the ability to implement a class would contradict that.

Sure, but whenever you say "package maintainers should take great care when doing ___", you're tacitly saying "I am OK with lowering their productivity". But package consumers also want new features and bug fixes in the packages they consume! Whenever we tie the hands of package maintainers, it means fewer features and bug fixes and slower improvement. That also harms package consumers.

Instead, as is pointed out in the link above, most devs will write comments about what is intended and what's not, and then only focus on the "supported" breaking changes.

Sure. But as anyone who maintains a popular package will tell you, users do ignore those comments and then turn around and get angry with the package maintainer when they do something unsupported and then a new version of the package breaks their code. In practice, Dart's flexibility here puts a lot of hidden back-pressure on maintainers which makes it harder to improve their APIs.

This comment can be replaced with an annotation that warns users when they try to do something the author specifically doesn't support. But saying "the author didn't intend it" == "the API doesn't support it" is not always true, and therefore IMO Dart shouldn't be limiting devs in their (creative) use of APIs.

I think it's really important to keep in mind that you have the source. Dart's package management system is entirely based on distributing packages as raw source code, and pub's dependency_override system is deliberately designed to let you globally replace a package with one of your own control when you want. If you really want an unsupported use or change to some package, you can always just crack it open yourself and make the change. It's not like C/C++ where you might be using a library that you literally only have the compiled binaries for and you're stuck with what the author gave you.

Levi-Lesches commented 3 years ago

...users do ignore those comments and then turn around and get angry with the package maintainer when they do something unsupported and then a new version of the package breaks their code.

Right, but if we make it so that users can't do what they're doing (as opposed to simply breaking it), those angry users won't just go away. I'm not denying the reality of users begging package maintainers for every last edge-case to be supported, but rather pointing out that they'll likely be twice as upset if Dart removes some of the functionality entirely.

Being able to extend or implement any class you find can open up more possibilities for consumers, and I think it would be a shame if non-open were the default. That being said, if the package author realises there's an error waiting to happen, I fully support any annotation like @sealed that would warn consumers that they're using the API incorrectly. package:meta annotations are really great because they allow the consumer to decide what they want to do with guidance from the package maintainers. If I seal a class in my package A:

// a.dart
import "package:meta/meta.dart";

@sealed
class A { }

then extend it in another package B:

// b.dart
import "package:a/a.dart";
class B extends A { } 

Dart gives me the following lint:

info - The class 'A' shouldn't be extended, mixed in, or implemented because it is sealed - lib\b.dart:3:1 - subtype_of_sealed_class

Now, I can choose to ignore it by adding an // ignore comment:

// b.dart
import "package:a/a.dart";
class B extends A { }   // ignore: subtype_of_sealed_class

Or I can set it to be an error/warning instead of just a lint:

# B/analysis_options.yaml
analyzer:
  errors:
    subtype_of_sealed_class: error

The point is that the author of A was able to say "don't extend or implement A", and B was able to decide whether they want to take that seriously or not. With the right annotations (IMO with a higher default severity, like warning instead of info), we can convey this without adding hard restrictions. I just don't see this as having the same significance as type safety, in which Dart can't even compile the code until the issues are resolved.

lrhn commented 3 years ago

There are things you can do to improved performance with enforced restrictions, that you can't without them, or if they are just guidelines. That's another reason for wanting strict restrictions.

munificent commented 3 years ago

Right, but if we make it so that users can't do what they're doing (as opposed to simply breaking it), those angry users won't just go away.

Yeah, this is a good point. However, I do believe that most users using classes in non-intended ways aren't doing so in spite of the package maintainer's preference, they're doing it because they simply don't know whether or not the package is intended to be used that way since it's hard for package authors to communicate that intent.

If a package seals a class, I think most users of that class who try to extend will instead find another path to their solution. I think it's fairly rare that not being able to extend and/or implement a class prevents a user from solving their problem.

The point is that the author of A was able to say "don't extend or implement A", and B was able to decide whether they want to take that seriously or not. With the right annotations (IMO with a higher default severity, like warning instead of info), we can convey this without adding hard restrictions. I just don't see this as having the same significance as type safety, in which Dart can't even compile the code until the issues are resolved.

I agree warnings can be nice to prevent users from being totally stuck. However, in this case, we are hoping to use these access control restrictions for things like type safety, which makes it more important that they be non-optional. In particular, I'd like Dart to support ADT-style pattern matching (#349). To do that and get good exhaustiveness checking on match statements/expressions, we likely need to be able to statically tell "These are the only subclasses of class C" so that we can tell if every case is covered. Sealed class hierarchies can give us that. But if the sealing is only semi-enforced, then the type system can't trust that is knows all the subclasses.

Levi-Lesches commented 3 years ago

You guys bring up good points -- seems to be that for performance and safety reasons, the compiler needs to be sure it knows about all subtypes of a class.

I guess my concern is that there are people currently subclassing APIs that the author may not be aware of. After all, being able to extend of implement any class without any special declarations is a huge freedom in Dart, compared to the difference between abstract and interface in other languages. In this case, making a class non-extendable by default won't allow for these use-cases. But I admit I don't really know how often this is done, and I'm mainly speaking anecdotally. Do you think this is a concern?

munificent commented 3 years ago

But I admit I don't really know how often this is done, and I'm mainly speaking anecdotally. Do you think this is a concern?

It is a concern—it comes up a lot whenever we discuss access control restrictions. I don't know how much of a concern it is in practice, and it's very hard to get good data on since we don't have any way to mechanically infer whether a class is intended to be subclassed.

My suspicion is that it's not as much of an issue as people believe. Humans can be be kind of like cats where we will sit there and meow in front of a door if it's closed to us even if we actually don't want to go through it. We just like knowing that we could go through it if we wanted to. I think access control triggers some of that same impulse.

Levi-Lesches commented 3 years ago

Yeah, after searching my own projects, I'm inclined to trust you on that even though that's clearly a dog in your profile picture

themisir commented 3 years ago

IMO I would prefer "sealed" keyword rather than "struct" for limiting class extendability. Because "struct" in other languages like C# is used for different intent than sealed classes, which might confuse or in future we might want to add features like inline type allocation to the language which might increase confusion even more.