Skip to content

Analyzer doesn't show a hint for shorthand function with wrong return type but throws at runtime in checked mode #26625

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

Closed
zoechi opened this issue Jun 6, 2016 · 4 comments
Labels
legacy-area-analyzer Use area-devexp instead. 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

@zoechi
Copy link
Contributor

zoechi commented Jun 6, 2016

dartanalyzer doesn't produce a hint or warning for

new Future<Null>(() => style.opacity = '1');

but it throws at runtime

Uncaught Unhandled exception:
type 'String' is not a subtype of type 'Null' of 'value' where
String is from dart:core
Null is from dart:core

0 _Future._setValue (dart:async/future_impl.dart:296)

1 _Future._complete (dart:async/future_impl.dart:466)

2 Future.Future. (dart:async/future.dart:118)

3 Timer._createTimer. (dart:async-patch/timer_patch.dart:16)

4 _Timer._Timer. (dart:html:49213)

(anonymous function)

Dart VM version: 1.17.0-edge.b37747934e762dbb4973ddd8798c4ca6126f5f24 (Fri Jun 3 15:14:30 2016) on "linux_x64"

  • strong mode enabled
  • browser application run in Dartium from pub serve
@lrhn
Copy link
Member

lrhn commented Jun 6, 2016

This is a spec-mode and strong-mode valid program that fails at runtime. The problem is that:
new Future<Null>(...) expects an argument of type ()->dynamic where dynamic stands for T or Future<T>, but we can't actually represent that. Since we can't represent it, we also can't check it.

It gets an argument of type ()->String, which is valid when assigned to ()->dynamic. When the resulting string is attempted converted to Null, it fails predictably.

Short of having union types, or separate constructors for Future<T> and T computations, I don't see a good solution. We can't express the actual constraint in the spec type system. The current strong-mode comments contains union types, so it might be fixable in strong mode only.

@leafpetersen
Copy link
Member

@jmesserly is working on handling the special case of the future API for at least Future.then, and possibly other cases like this one.

@jmesserly
Copy link

Yeah, I have been slowly working on a change that teaches Strong Mode inference that Future<T>.then<S> type is (T --> (S | Future<S>) ) --> Future<S>.

We could perhaps extend that trick to treat the constructor here as taking a parameter with type () --> (T | Future<T>)

Note we don't currently do any upwards inference on constructors (#25220), so it wouldn't infer the T, but it would improve the static error checking. In your case, it would issue a warning that Null or Future<Null> was expected as the return value of that function.

@bwilkerson bwilkerson 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 Jun 8, 2016
@leafpetersen
Copy link
Member

This is obsolete. FutureOr should have addressed this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. 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

6 participants