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

Migrate tool incorrectly treats result of emulated function calls as nullable #46263

Closed
greglittlefield-wf opened this issue Jun 4, 2021 · 2 comments
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool).

Comments

@greglittlefield-wf
Copy link
Contributor

greglittlefield-wf commented Jun 4, 2021

When running dart migrate, the return values of invocations of emulated function classes (classes that implement call) are incorrectly treated as nullable.

This only happens when calling emulated functions directly but not when using .call(...), and also doesn't affect normal function types.

As a result, functions that return the result of these calls and variables that are initialized with the result of these calls are marked as nullable.

Reduced test case: https://gist.github.com/greglittlefield-wf/3b8cb36ae407a8485b1f5029db0e9c62

Screenshot of migration tool
Screen Shot 2021-06-04 at 12 04 41 PM

After running the migration and opting into null-safety, showing that the static type of the call is indeed int and not int?.
Screen Shot 2021-06-04 at 12 23 13 PM

The function return value case can be worked around by adding /*!*/ hints comments, but unfortunately there's nothing that can prevent the tool from updating the var case.

We also have many of these types of calls in our libraries (via the invocation of OverReact UiProps classes as builders), so having to manually add the hints and update the typings on variables seems like it would be pretty tedious.

For example,

// The over_react code:
var content = Dom.div()();

// gets migrated to:
ReactElement? content = Dom.div()();

Tested and reproduced in Dart SDK versions 2.13.0 and 2.14.0-176.0.dev

@devoncarew devoncarew added the area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). label Jun 7, 2021
@greglittlefield-wf
Copy link
Contributor Author

I took a stab at fixing this; here's the CL: https://dart-review.googlesource.com/c/sdk/+/220125

@stereotype441
Copy link
Member

As of 1c7fe71, the null safety migration tool has been removed from active development and retired. No further work on the tool is planned.

If you still need help, or you believe this issue has been closed in error, please feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool).
Projects
None yet
Development

No branches or pull requests

3 participants