Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dartanalyzer should report an error when catchError returns a different type #35545

Closed
tomyeh opened this issue Jan 3, 2019 · 3 comments
Closed
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@tomyeh
Copy link

tomyeh commented Jan 3, 2019

Dartanalyzer doesn't detect any error/warning for the following code. However, there will be a run-time error: type 'String' is not a subtype of type 'FutureOr<List>'

It is hard to pick up this kind of errors by eyes.

IMO, it is better to allow Future.catchError() to return another type like Future.then<R>() does. But, it is OK as long as dartanlyser can pick up the errors for us.

import "dart:async";

void main() {
  foo()
  .catchError((ex) {
  	return "error";
  });
}
Future<List> foo() async {
  throw "error";
}

Dart SDK: 2.1

@vsmenon vsmenon added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jan 7, 2019
@stereotype441 stereotype441 added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Jan 7, 2019
@stereotype441
Copy link
Member

The reason the analyzer isn't catching this case is that the type of Future<T>.catchError's argument is Function, which is too general to allow the type system to detect the problem; it allows any function whatsoever is allowed to be passed in, regardless of what it returns.

In an ideal world, we would add union types to the language (see dart-lang/language#145), and then catchError's argument could be changed to have a more precise type: ((dynamic) -> FutureOr<T>) | ((dynamic, StackTrace) -> FutureOr<T>). Then the analyzer would detect the mistake in the code above.

But fully general support for union types isn't on the language funnel right now, so in the meantime we might want to could consider special-case logic in the analyzer to handle this case. Accordingly I'm classifying this as an enhancement rather than a bug.

@stereotype441 stereotype441 changed the title Dartanalyzer shall report an error when catchError returns a different type Dartanalyzer should report an error when catchError returns a different type Jan 7, 2019
@srawlins
Copy link
Member

I'm working on #35825, which I think is more-or-less a feature which solves this issue.

@srawlins
Copy link
Member

Fixed in work for #35825

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants