Open rubenferreira97 opened 1 year ago
@rubenferreira97 what version of Dart are you one? The exhaustiveness checking is only very recently implemented.
@jakemac53 I am experimenting a master branch Dart SDK version: 3.0.0-edge.8fcaccae288d05a25a2b795b61973d28e17c2b3d (be) (Tue Mar 28 15:27:41 2023 +0000) on "windows_x64"
.
It seems like there may be two separate issues here relating to exhaustiveness checking.
void main() {}
void example<T, E>(Result<T, E> result) {
switch (result) {
case Ok():
print('OK');
case Err():
print('Err');
}
}
sealed class Result<T, E> {}
class Ok<T> extends Result<T, Never> {}
class Err<E> extends Result<Never, E> {}
bin/errors.dart:4:11: Error: The type 'Result<T, E>' is not exhaustively matched by the switch cases.
- 'Result' is from 'bin/errors.dart'.
Try adding a default case or cases that match 'Ok<dynamic>()'.
switch (result) {
^
Additionally this example.
void main() {}
void example(Container<DivideError> error) {
switch (error) {
case Box<DivideByZero>():
print('divide by zero');
case Box<DivideByNegative>():
print('divide by negative');
}
}
sealed class Container<T> {}
class Box<T> extends Container<T> {}
sealed class DivideError {}
class DivideByZero extends DivideError {}
class DivideByNegative extends DivideError {}
bin/errors.dart:4:11: Error: The type 'Container<DivideError>' is not exhaustively matched by the switch cases.
- 'Container' is from 'bin/errors.dart'.
- 'DivideError' is from 'bin/errors.dart'.
Try adding a default case or cases that match 'Box<DivideError>()'.
switch (error) {
^
Dart SDK version: 3.0.0-384.0.dev (dev) (Wed Mar 29 17:12:52 2023 -0700) on "windows_x64"
The first example in the previous comment looks like either a bug, or a place where the algorithm is not sophisticated enough to correctly enumerate the subtypes. @munificent @johnniwinther thoughts on this one?
The second example is legitimately not exhaustive, and the counter-example in the error message is correct. That switch does not match Box<DivideError>
. Generics are a bit non-intuitive that way.
Yes, in the first example the hierarchy is considered "non-trivial", though here in the very low end of non-trivial and potential candidate for improvement.
So it sounds like this is working as intended for now, though possibly something we could improve in the future. While not ideal, I believe you can work around this by making Ok
and Err
parametric over both types, that is:
class Ok<T, E> extends Result<T, E> {}
class Err<T, E> extends Result<T, E> {}
Hopefully inference could then avoid having to specify both explicitly. Alternatively, you could provide helper construction functions, e.g.:
Ok<T, Never> ok<T>() => Ok();
Thanks for the quick response! Unfortunately if I make Ok
and Err
parametric over both types I always need to rely on return inference or type it correctly even if the type does not make sense or else some errors will pop.
sealed class Result<T, E> {}
class Ok<T, E> extends Result<T, E> {
Ok(this.value);
final T value;
}
class Err<T, E> extends Result<T, E> {
Err(this.error);
final E error;
}
sealed class DivideError {}
class DivideByZero extends DivideError {}
class DivideByNegative extends DivideError {}
Result<int, DivideError> divideNonNegative(int a, int b) {
final error = Err(DivideByZero()); // infers as Err<dynamic, ...>, need to declare as final Err<int, DivideError>
if (b == 0) return error; // error
if (b < 0) return Err(DivideByNegative()); // this works because int is inferred
return Ok(a ~/ b);
}
I guess it kinda works, but it seems really fragile. I think I will wait for a better solution. I am aware the team is really busy so it's perfectly normal that the first implementation of exhaustiness will have some imperfections. I am fine with that, but personally (a little bias I know š) I think this is a nice feature to aim, express different return paths for error handling. If someone finds a better way to implement this I am all ears.
I would like to ask two more things:
Could dart allow class declarations inside sealed classes like Kotlin or Rust?
Kotlin:
sealed class Event<out T: Any> {
class Success<out T: Any>(val value: T): Event<T>()
class Error(val msg: String, val cause: Exception? = null): Event<Nothing>()
}
Rust:
enum Result<T, E> {
Ok(T),
Err(E),
}
Hypothetically, could Dart pull up this magic? I really don't see a way to implement this currently. I can only see it happen at language level. https://doc.rust-lang.org/book/ch09-02-recoverable-errors-with-result.html#a-shortcut-for-propagating-errors-the--operator
Also, I want to express my sincere appreciation to the Dart team for your hard work on Dart 3.0. Your dedication have resulted in a programming language that has made my work as a developer easier and more enjoyable.
Unfortunately if I make
Ok
andErr
parametric over both types I always need to rely on return inference or type it correctly even if the type does not make sense or else some errors will pop.
Yeah, I think you'd really want to make construction be via a helper function as I described above, instead of directly via the constructors unfortunately.
I am fine with that, but personally (a little bias I know š) I think this is a nice feature to aim, express different return paths for error handling.
I agree that it would be really nice to handle this. I expect this kind of use case to not be uncommon.
Could dart allow class declarations inside sealed classes like Kotlin or Rust?
I think @munificent has had some thoughts in that direction, not sure if he's written anything up about it yet. Obviously no promises though, it's certainly not currently on the roadmap.
Also, I want to express my sincere appreciation to the Dart team for your hard work on Dart 3.0. Your dedication have resulted in a programming language that has made my work as a developer easier and more enjoyable.
Thanks for the kinds words!
@rubenferreira97, you might find https://github.com/dart-lang/sdk/issues/51680 relevant. It isn't about exhaustiveness, but it contains some discussions about the typing of a very similar class hierarchy, and some reasons why you may not wish to use the value Never
for the "unused" type parameter.
@eernstg Thanks for pointing me to that issue, it made me realize that there are indeed some problems with this implementation. Ideally I would just want to type Ok<T>
and Err<E>
without Never
, like rust does by allowing sealed classes declarations to be nested (I really don't know the correct name for this). However I still assume we would need to make change Result T
and E
variance. In rust Result<T, E>
are invariant?
Edit: Connecting some dots. @eernstg solution (runtime inout
) seems to exhaustiveness check (since it declares Ok
and Err
parametric over both types like @leafpetersen suggested):
sealed class _Result<T, E, Invariance extends _Inv<T, E>> {
const _Result();
}
class _Ok<T, E, Invariance extends _Inv<T, E>> implements
_Result<T, E, Invariance> {
final T value;
_Ok(this.value);
}
class _Err<T, E, Invariance extends _Inv<T, E>> implements
_Result<T, E, Invariance> {
final E error;
const _Err(this.error);
}
typedef _Inv<T1, T2> = (T1, T2) Function(T1, T2);
typedef Result<T, E> = _Result<T, E, _Inv<T, E>>;
typedef Ok<T, E> = _Ok<T, E, _Inv<T, E>>;
typedef Err<T, E> = _Err<T, E, _Inv<T, E>>;
sealed class DivideError {}
class DivideByZero extends DivideError {}
class DivideByNegative extends DivideError {}
Result<int, DivideError> divideNonNegative(int a, int b) {
if (b == 0) return Err(DivideByZero());
if (b < 0) return Err(DivideByNegative());
return Ok(a ~/ b);
}
void main() {
final result = divideNonNegative(10, 0);
switch (result) {
case Ok(:final value):
print(value);
case Err(:final error):
switch (error) {
case DivideByZero():
print('Divide by zero');
case DivideByNegative():
print('Divide by negative');
}
}
}
But take this weird example:
// exhausts
switch(result) {
case Ok():
case Err():
}
// does not exhaust, bug?
switch(result) {
case Ok _:
case Err _:
}
But take this weird example:
Yeah, it's weird. Probably correct, but weird.
The Ok()
is an object pattern, which applies type inference to the type parameter of Ok
based on the matched value type.
The Ok _
is a variable pattern which has a raw type as declared type. That type is instantiated to bounds indifferently to which type and value will eventually be assigned to it.
In Ok()
, the matched type is _Ok<int, DivideError, (int, DivideError) => (int, DivideError)>
. (Checked by making it case Ok() && var x: print([x].runtimeType);
.)
In Ok _
, it's _Ok<dynamic, dynamic, (dynamic, dynamic) => (dynamic, dynamic)>
, which is not a supertype of _Ok<int, DivideError, (int, DivideError) => (int, DivideError)>
, so the latter type is not exhausted by the Ok _:
pattern. (It also doesn't match an OK result
, even if you add a default:
case to make the switch exhaustive and runnable.
The _Inv
hack gives you invariance, but invariance means that instantiate-to-bounds is no longer guaranteed to create a supertype. Whoops!
TL;DR: Always use the object pattern. :wink:
Sorry if this is a silly question, but why instantiate-to-bounds does not guarantes that the supertype of a type is at least the type itself? Well I understand that the name supertype loses meaning here š, I maybe would call it an upper bound. Is there any good reason why the type resolution algorithm behaves different from Type()
and Type _
? Why it couldn't infer a more precise type than dynamic
? I think we have the same compile time information here.
The difference is that Ok()
is an object pattern. Because it doesn't explicitly write any type arguments, it doesn't need to check the type arguments of the matched value. It only uses the static type of a matched value to infer a type arguments in order to pass those downwards to any field patterns. That's why Ok(value: var x)
matched against an Ok<int, DivideError>
will infer int
as the type of x
.
But it never checks the type arguments of the object, because it doesn't need to. The inferred type arguments cannot fail given the matched value type.
The Ok _
pattern is just a variable pattern, and works like a variable declaration. If you write Ok x = ...
, it will also not try to infer type arguments to Ok
from the righthand-side, it just instantiates to bounds. It's a "raw type", and they just work like that. (And I'm sure there is an issue asking for them to be smarter.)
The information is there, but just lkek Ok x = ...
won't use that information, the pattern Ok _
won't either.
And because Ok
is invariant, instantiate-to-bounds doesn't actually create a supertype of all Ok
instances, like it would in most normal cases.
@rubenferreira97, you might want to customize the Dart analysis a bit by putting some directives into analysis_options.yaml
in the root directory of the current package:
analyzer:
language:
strict-raw-types: true
strict-inference: true
strict-casts: true
linter:
rules:
- avoid_dynamic_calls
In particular, strict-raw-types
will enable an 'info' or 'warning' message wherever the code contains a raw type whose type arguments are chosen to be dynamic
by instantiation to bound.
class A<X> {}
class C<X extends C<X>> {}
void main() {
A a = A<int>(); // 'strict-raw-types': `A` means `A<dynamic>`.
Object o = a;
switch (o) {
case A _: print('A'); // 'strict-raw-types': `A` means `A<dynamic>`.
case C _: print('C'); // No warning.
}
}
Note that this mechanism does not report situations where the raw type gets a more complex type argument by instantiation to bound, and that type argument contains dynamic
. The standard example is the class C
above whose type parameter is F-bounded (that is: it's bound is recursive), where instantiation to bound transforms the raw C
to C<C<dynamic>>
. Nevertheless, 'strict-raw-types' will surely catch a large majority of the cases where you have accidentally used a raw type, and the effect is that type checking is less precise.
I think we could say that there is no need to prefer object patterns over variable patterns in general (they are just different, and they serve different purposes), but variable patterns do have the special pothole that is dynamic
type arguments chosen by instantiation to bound. That, in turn, is an issue that you might want to deal with in a more general manner anyway.
Yeah, unused type parameters make things harder. But keep in mind that when translating a Rust enum to Dart, you don't have to have the same type parameters on every type constructor / subclass. In your example, since Result
doesn't actually use T
or E
, you could write it like:
sealed class Result {}
class Ok<T> extends Result {
Ok(this.value);
final T value;
}
class Err<E> extends Result {
Err(this.error);
final E error;
}
Now each subclass only has the type parameter it cares about. With this, there are no exhaustiveness errors in the following:
void main() {
final result = divideNonNegative(10, 0);
switch (result) {
case Ok(:final value):
print(value);
case Err(:DivideByZero error):
print('Divide by zero');
case Err(:DivideByNegative error):
print('Divide by negative');
case Err(:final error):
print('Other error');
}
}
Note that the final Err(:final error)
case is needed because, obviously, you can parameterize Err
with error types not related to division at all.
I think @munificent has had some thoughts in that direction, not sure if he's written anything up about it yet. Obviously no promises though, it's certainly not currently on the roadmap.
I am definitely interested in exploring syntax to make it easier to define a sealed hierarchy of types, but I don't have anything concrete yet.
@munificent Thank you for the suggestion! Unfortunately, with that example, I think I would lose the main goal I am aiming for, which is type safety at the exhaustiveness level while providing separate paths for "correct" and "error" cases. Assuming we are writing Result divideNonNegative(int a, int b)
, :final value
becomes dynamic
. I also feel that checking Err(:final error)
in every case would be unnecessary boilerplate, especially when I know I have a closed set of error types that I can return. If my result Ok
becomes dynamic
I think I am not really better than a simple dynamic
return. I understand that we don't have to map Rust 1-to-1, but I would like to retain the functionality that Rust provides.
Ah, I see. Yes, you lose the ability to pin both the result and error types in the return type type annotation.
I am trying to implement a custom error handling in dart. I want to be able to represent a correct return (Ok) or an error union (yes, I really hate undocumented exceptions š, I would use exceptions only for "panic" situations, unrecoverable errors). Since unions are still not present in Dart I am trying to accomplish this with sealed classes.
I took some error handling examples from different languages and rust error handling (https://doc.rust-lang.org/book/ch09-02-recoverable-errors-with-result.html) seems to work well in many cases.
This is my implementation, however the following does not exhaustiveness checks.
First, is there a more "darty" way to represent this? Should this give an error? Since I am using sealed classes I would think that this was Ok.