dart-lang / language

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

Switch expressions don't exaustevely check for local functions that return exceptions. #3313

Open feinstein opened 1 year ago

feinstein commented 1 year ago

I tried to make the following code with switch expression.

  static FirebaseOptions forPlatform({FirebaseOptions? android, FirebaseOptions? ios}) {
    void throwUnsupportedPlatform(TargetPlatform platform) => throw UnsupportedError('DefaultFirebaseOptions are not supported for $platform.');

    return switch (defaultTargetPlatform) {
      TargetPlatform.android => android ?? throwUnsupportedPlatform(defaultTargetPlatform),
      TargetPlatform.iOS => ios ?? throwUnsupportedPlatform(defaultTargetPlatform),
      _ => throwUnsupportedPlatform(defaultTargetPlatform),
    };
  }

The analyzer complains that A value of type 'void' can't be returned from the method 'forPlatform' because it has a return type of 'FirebaseOptions'.. I imagined the analyzer would be able to know that throwUnsupportedPlatform always throws, so this is valid code.

This code works:

    return switch (defaultTargetPlatform) {
      TargetPlatform.android => android ?? (throw UnsupportedError('DefaultFirebaseOptions are not supported for $defaultTargetPlatform.')),
      TargetPlatform.iOS => ios ?? (throw UnsupportedError('DefaultFirebaseOptions are not supported for $defaultTargetPlatform.')),
      _ => throw UnsupportedError('DefaultFirebaseOptions are not supported for $defaultTargetPlatform.'),
    };
srawlins commented 1 year ago

The type of throw is not void, but Never. I believe you want the return type of throwUnsupportedPlatform to be Never.

feinstein commented 1 year ago

@srawlins excellent point, I forgot about Never, it works now.

Is there a reason why the analyzer needs Never for simple cases like that?

srawlins commented 1 year ago

My only answer is a tautology: Never is the type that can be given when an int, or anything else, is expected. A function like int foo() => print('hi'); is illegal because void is very much not a valid int. But Never is that special type that works, as it expresses something akin to throwing.

feinstein commented 1 year ago

So maybe the linter should point me out that void is not a valid return for a function that only throws and Never would be more suitable?

srawlins commented 1 year ago

That's a great idea. If you'd like to follow up, you could suggest that at https://github.com/dart-lang/linter/issues.

rrousselGit commented 1 year ago

So maybe the linter should point me out that void is not a valid return for a function that only throws and Never would be more suitable?

It sometimes is. You may want to assign a Never Function() to void Function()

feinstein commented 1 year ago

@rrousselGit when do you think this would happen? I thought about unsupported features and it made me think that the linter warning would be triggered at every UnsupportedExcetion that abstract classes usually have, unless the linter has the ability to exclude that exception.

Do you guys think it's worthy for me to suggest that linter, or should I just close this issue?