Skip to content

Migration: better handling of list constructor invocations #44198

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
stereotype441 opened this issue Nov 13, 2020 · 3 comments
Closed

Migration: better handling of list constructor invocations #44198

stereotype441 opened this issue Nov 13, 2020 · 3 comments
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). NNBD Issues related to NNBD Release P2 A bug or feature request we're likely to work on

Comments

@stereotype441
Copy link
Member

stereotype441 commented Nov 13, 2020

Currently when the migration tool encounters a list constructor invocation, it leaves it alone, because it doesn't know how to migrate it without help from the user. This leads to a bad user experience because the user sees compilation errors and doesn't know why.

It would be a lot nicer if the migration tool could add a TODO comment along with a link to some documentation of how list constructors should be migrated.

Edit: as @lrhn points out below, there actually is a reasonable thing the migration tool can do:

  • Migrate List() to []
  • Migrate List<T>() to <T>[]
  • Migrate List(n) to List.filled(n, null)
  • Migrate List<T>(n) to List<T?>.filled(n, null)

Of course the migrations with List.filled result in the list element type being nullable, but that's the sort of thing they'll be fixing anyway as part of the migration process.

@stereotype441 stereotype441 added legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on analyzer-nnbd-migration NNBD Issues related to NNBD Release labels Nov 13, 2020
@lrhn
Copy link
Member

lrhn commented Nov 13, 2020

How about migrating List()/List<T>() to []/<T>[] and List(n)/List<T>(n) to List.filled(n, null)/List<T?>.filled(n, null).

The latter two can be wrong when nullability of the result doesn't line up, but that problem is inherent in having a null-filled array, and something they'll have to fix one way or another.

@stereotype441
Copy link
Member Author

How about migrating List()/List<T>() to []/<T>[] and List(n)/List<T>(n) to List.filled(n, null)/List<T?>.filled(n, null).

The latter two can be wrong when nullability of the result doesn't line up, but that problem is inherent in having a null-filled array, and something they'll have to fix one way or another.

You're right, @lrhn. That's a better approach. I'll update the issue description accordingly.

@stereotype441 stereotype441 changed the title Migration: add TODO comments for list constructor invocations Migration: better handling of list constructor invocations Nov 13, 2020
@stereotype441 stereotype441 added area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). and removed analyzer-nnbd-migration legacy-area-analyzer Use area-devexp instead. labels Nov 16, 2020
@stereotype441
Copy link
Member Author

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). NNBD Issues related to NNBD Release P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

2 participants