Skip to content

NNBD: Dart does not throw a compile error when async function with Future<bool> return type returns dynamic. #41437

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
iarkh opened this issue Apr 10, 2020 · 6 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release

Comments

@iarkh
Copy link
Contributor

iarkh commented Apr 10, 2020

Dart VM version: 2.8.0-dev.18.0 (dev) (Fri Mar 27 11:10:52 2020 +0100) on "windows_x64"

The following sample code example throws a compile error with analyzer on the line 3 and passes without any error with dart:

dynamic getNull() => null;

Future<bool> test1() async => await getNull();
Future<bool> test2() => getNull();
bool test3() => getNull();

main() {}

Seems like dart should throw a compile error here too. Please see discussion in the Issue #41266 for more details.

Sample output is:

$> dartanalyzer --enable-experiment=non-nullable test.dart
Analyzing test.dart...
error - A value of type 'dynamic' can't be returned from function 'test1' because it has a return type of 'bool'. - test.dart:3:31 - return_of_invalid_type
1 error found.

$> dart --enable-experiment=non-nullable test.dart
$

@vsmenon vsmenon added legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release labels Apr 13, 2020
@vsmenon
Copy link
Member

vsmenon commented Apr 13, 2020

@eernstg @leafpetersen - can you please sanity check intended behavior and reassign to analyzer if needed? Not clear to me if there should be an implicit cast and no static error above.

@leafpetersen
Copy link
Member

The analyzer behavior is correct per the current specification. @eernstg is taking a pass over the specification of returns and will follow up with action items as appropriate.

@johnniwinther
Copy link
Member

@leafpetersen @eernstg Where is this specified ?

So is it ok to have

method(dynamic o) {
  bool b = o; // implicit cast
}

but not ok to have

bool method(dynamic o) {
  return o; // no implicit cast ?
}

?

@johnniwinther
Copy link
Member

Reading analyzer error again I think I got it. The problem is assigning Future<dynamic> to Future<bool> and not, as it actually says, assigning dynamic to bool. A better message would probably be helpful.

@eernstg
Copy link
Member

eernstg commented Apr 27, 2020

I already reported the analyzer error message in #41266 (comment), but that issue was closed because the discussions about async returns was handled in language issues. So I just created a new issue #41670 about the error message.

The PR where new rules about async returns are proposed is dart-lang/language#941.

The new (null-safety specific) rules, if adopted, will make it an error to return e; in an async function with declared return type Future<U> where U is not a top type if e has type Future<dynamic>. In other words, with the switch to null-safety we can no longer obtain an implicit downcast on the object which is used to complete the returned future if we are awaiting it (not even from dynamic, even though implicit downcasts from dynamic are otherwise allowed with null-safety).

Example: Future<int> f1() async => Future<dynamic>.value(1); is a compile-time error, and so is Future<int> f2() async => 1 as FutureOr<dynamic>;. But Future<int> f3() async => 1 as dynamic; is allowed.

@leafpetersen
Copy link
Member

@leafpetersen @eernstg Where is this specified ?

https://dart.dev/guides/language/specifications/DartLangSpec-v2.2.pdf , section 17.12 has the current specified rules (adapted appropriately when running with null safety to account for the different assignability).

@eernstg is proposing changes to this for null safety, if those changes are accepted we will land them as a change to the null safety feature specification, land tests, and file implementation issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release
Projects
None yet
Development

No branches or pull requests

5 participants