Skip to content

Dart type checking became more strict #5605

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
drewwarren opened this issue Aug 25, 2016 · 22 comments
Closed

Dart type checking became more strict #5605

drewwarren opened this issue Aug 25, 2016 · 22 comments
Labels
customer: fast (g3) waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds

Comments

@drewwarren
Copy link

I have a function

double get packedPercent =>
      deadline.totalItems == 0 ? 0 : packedCount / deadline.totalItems;

This function used to work fine, even when deadline.totalItems was 0. Now it fails when a type error when deadline.totalItems is 0.

I'm guessing this is because I am returning an int 0 instead of a double 0.0.

I do not get any output from dart analysis related to this, even when using https://github.com/raw/flutter/flutter/master/.analysis_options

StoreManager: ══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
StoreManager: The following assertion was thrown building _Deadline:
StoreManager: type 'int' is not a subtype of type 'double' of 'function result' where
StoreManager:   int is from dart:core
StoreManager:   double is from dart:core
StoreManager: Either the assertion indicates an error in the framework itself, or we should provide substantially
StoreManager: more information in this error message to help you determine and fix the underlying cause.
StoreManager: In either case, please report this assertion by filing a bug on GitHub:
StoreManager:   https://github.com/flutter/flutter/issues/new
StoreManager: When the exception was thrown, this was the stack:
StoreManager: #0      _Deadline.packedPercent (lib/routes/dashboard/deadlines.dart:74)
StoreManager: #1      _Deadline.packedPercentAndTimeLeft (lib/routes/dashboard/deadlines.dart:137)
StoreManager: #2      _Deadline.build (lib/routes/dashboard/deadlines.dart:93)
StoreManager: #3      ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:2027)
StoreManager: #4      BuildableElement.rebuild (package:flutter/src/widgets/framework.dart:1941)
StoreManager: #5      ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:2013)
StoreManager: #6      ComponentElement.mount (package:flutter/src/widgets/framework.dart:2008)
StoreManager: #7      Element.inflateWidget (package:flutter/src/widgets/framework.dart:1584)
StoreManager: #8      MultiChildRenderObjectElement.mount (package:flutter/src/widgets/framework.dart:2780)
StoreManager: #9      Element.inflateWidget (package:flutter/src/widgets/framework.dart:1584)
StoreManager: #10     Element.updateChild (package:flutter/src/widgets/framework.dart:1461)
StoreManager: #11     ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:2039)
StoreManager: #12     BuildableElement.rebuild (package:flutter/src/widgets/framework.dart:1941)
...
@eseidelGoogle
Copy link
Contributor

So this is a recent VM change, which was picked up in a flutter/engine roll. Is this expected @a-siva?

@eseidelGoogle
Copy link
Contributor

CC @pq since the analyzer doesn't warn about this?

@Hixie
Copy link
Contributor

Hixie commented Aug 26, 2016

Does your .analysis_options file turn on all the type checking?
I'm pretty sure this has never worked before? We've asked for it to work for a while (it's on the Flutter team's "Dart wishlist"), so I'd be surprised if it did work before...

@pq
Copy link
Contributor

pq commented Aug 28, 2016

cc @bwilkerson @jmesserly who may have better insight into the analysis side of things.

@bwilkerson
Copy link
Contributor

Was this error produced by running the AOT compiler?

@jmesserly
Copy link

This is just a checked mode failure. This hasn't ever worked in Dart.

$ cat test.dart

double get packedPercent => 0;
main() { print(packedPercent); }

$ dartanalyzer test.dart

Analyzing [test.dart]...
[warning] The return type 'int' is not a 'double', as defined by the method 'packedPercent'. (test.dart, line 1, col 29)
1 warning found.

$ dartanalyzer --strong test.dart

Analyzing [test.dart]...
[error] The return type 'int' is not a 'double', as defined by the method 'packedPercent'. (test.dart, line 1, col 29)
1 error found.

$ dart -c test.dart

Unhandled exception:
type 'int' is not a subtype of type 'double' of 'function result' where
  int is from dart:core
  double is from dart:core

#0      packedPercent (test.dart:1:29)
#1      main (test.dart:2:16)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:261)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)

@drewwarren
Copy link
Author

void main() {
  print(test(1));
  print(test(2));
  print(test(0));
}

double test(int a) => a == 0 ? 0 : 1 / a;

This currently runs on dartpad with no issues and no analyzer warnings. It will fail on the VM flutter uses.

CONSOLE

1
0.5
0

https://dartpad.dartlang.org/53bd8d3a960c49b0f8f22b80362aec13

@Hixie
Copy link
Contributor

Hixie commented Aug 29, 2016

dart2js has limitations with respect to how closely it implements dart's rules about numbers, due to JS only having one number type.

@sethladd
Copy link
Contributor

Not sure if analyzer knows what to do here, when ternary operator returns either int or double, and the function is marked to return a double. I'd expect an analyzer warning, regardless of later being compiled with dart2js.

cc @bwilkerson ?

@jmesserly
Copy link

jmesserly commented Aug 29, 2016

They do not appear to have "checked mode" turned on in dartpad. So type annotations are more or less ignored.

Not sure if analyzer knows what to do here, when ternary operator returns either int or double, and the function is marked to return a double. I'd expect an analyzer warning, regardless of later being compiled with dart2js.

@sethladd Analyzer knows what to do. It concludes "num", which is then implicitly downcast to "double". All of our modes allow it, if you want to disable downcasts use the recently introduced (in 1.19) --no-implicit-casts flag of strong mode. See:

http://news.dartlang.org/2016/08/dart-119-improved-developer-experiences.html

@jmesserly
Copy link

jmesserly commented Aug 29, 2016

@sethladd
Copy link
Contributor

@jmesserly kk, thanks! (all these modes and options and lints and features are making my head swim)

@drewwarren
Copy link
Author

drewwarren commented Aug 29, 2016

~$ dart --version

Dart VM version: 1.19.0-dev.5.0 (Wed Aug 10 09:29:58 2016) on "macos_x64"

~$ dartanalyzer --version

dartanalyzer version 1.19.0-dev.5.0~$ 

~$ cat test.dart

void main() {
 print(test(2));
 print(test(0));
}

double test(int a) => a == 0 ? 0 : 1 / a;

~$ dartanalyzer test.dart

Analyzing [test.dart]...
No issues found

~$ dartanalyzer --strong test.dart

Analyzing [test.dart]...
No issues found

~$ dart test.dart

0.5
0

~$ dart -c test.dart

0.5
Unhandled exception:
type 'int' is not a subtype of type 'double' of 'function result' where
  int is from dart:core
  double is from dart:core

#0      test (file:///Users/drewwarren/test.dart:6:23)
#1      main (file:///Users/drewwarren/test.dart:3:8)
#2      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:261)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)

@jmesserly
Copy link

@jmesserly kk, thanks! (all these modes and options and lints and features are making my head swim)

yeah no kidding!

If it helps to know, things will get better :). Strong mode will be "the only mode". Things like --no-implicit-casts are ways for us to gracefully introduce stricter checking we think is a good idea, and determine if it can be rolled into strong mode.

@Hixie Hixie modified the milestone: Flutter 1.0 Sep 12, 2016
@sethladd
Copy link
Contributor

Hi! How do we take action on this issue? Is this something for Flutter to do?

cc @floitschG

@sethladd sethladd added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Sep 14, 2016
@drewwarren
Copy link
Author

I guess this is more of a dart issue, but it was noticed in our application from a flutter engine roll.

The dart bug would be something like

double test(int a) => a == 0 ? 0 : 1 / a;

Should produce an error in dart analyzer but does not. Calling this function fails when running with -c.

@jmesserly
Copy link

Unfortunately in Dart1 this is not an error, but an implicit downcast :(
This is also true in strong mode, currently. We recently added --no-implicit-casts to help.

CC @leafpetersen

@leafpetersen
Copy link
Contributor

Yes, this is on long list of small but annoying problems to fix.

dart-lang/sdk#25368

@floitschG
Copy link
Contributor

@sethladd: the code is bad since it tries to return an integer where a double was required. If it hasn't been updated yet, it should be.
Once that's done we can close the issue. Tracking of better warnings and detecting these kind of errors earlier is covered in other bugs (such as dart-lang/sdk#25368).

@sethladd
Copy link
Contributor

it tries to return an integer where a double was required

Fair enough. I've asked @drewwarren if we can change the code to return deadline.totalItems == 0.0 ? 0.0 : ...

@sethladd
Copy link
Contributor

The customer's code has been fixed. They would love if the analyzer can detect if one of the cases of the ternary returns an incorrect type. In the meantime, we can close this issue in favor of dart-lang/sdk#25368

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
customer: fast (g3) waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds
Projects
None yet
Development

No branches or pull requests

9 participants