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

List runtime subtyping error #1581

Open
davidmartos96 opened this issue Apr 13, 2021 · 4 comments
Open

List runtime subtyping error #1581

davidmartos96 opened this issue Apr 13, 2021 · 4 comments
Labels
request Requests to resolve a particular developer problem

Comments

@davidmartos96
Copy link

Hello!
I've been migrated some code to null safety, in particular code that generates raw SQLite queries. One of these methods combines 2 arguments lists into one for the final query.
The migration I did involves generalizing the list of arguments to List<Object?>, but the problem I faced was a runtime exception when concatenating the two List<Object?>. The issue was that one of them was a variable of type List<Object?> but in reality it was created as List<Object>. Passing the non-nullable version as an argument to a function works fine but it can produce hard to catch bugs.

Example of the issue I had: https://dartpad.dev/2a154028230eeb58bb03856c020d7b8f?null_safety=true

The first print works fine whereas the second print gives a runtime exception that is hard to catch in a big codebase.
I was wondering if there is any option in the analyzer or lint that makes this issue more obvious to catch.

The solution I found is to do a .cast<Object?> but I wish there is a better solution.

void main() {
  final List<Object?> a = <Object?>[1, 2, 3, null];
  final List<Object> b = [4, 5];

  printSum(a, b);
  printSum(b, a);
}

void printSum(List<Object?> a, List<Object?> b) {
  print(a + b);
}
@davidmartos96 davidmartos96 added the request Requests to resolve a particular developer problem label Apr 13, 2021
@leafpetersen
Copy link
Member

Dart generics are always covariant, which means that there are implied runtime checks which can fail, as you observe. There's currently not a good way to prevent this, though we're exploring adding the option of using statically checked variance annotations to control this better. It would have some cost in terms of convenience however.

For this concrete example, I would probably recommend constructing the right kind of list to start with, rather than using .cast. So for example, I might suggest using [...a, ...b] instead?

I suppose we could lint on using a + b for list concatenation and suggest that pattern instead, but I'm not sure that there wouldn't be too many false positives - I think there are situations where you might want to use + instead.

cc @lrhn @munificent @natebosch for any other thoughts.

@lrhn
Copy link
Member

lrhn commented Apr 14, 2021

Not any great alternatives. Any time you up-cast a list, some methods become unsafe or surprising.
Those where E occurs contravariantly are usafe (add and operator+).
Those which returns something using E as type parameter become surprising (toList, toSet and operator+ again).

Here we have up-cast b from List<Object> to List<Object?>.
With that, b.toList()..addAll(a) will fail in the same way as b + a because b creates a List<Object> (which is then also up-cast to List<Object?>).
Using [...b, ...a] is better because it doesn't put b in charge of creating the new list.

I think my recommendation would be to never use toList and toSet and operator+ now that we have collection literals.

@davidmartos96
Copy link
Author

Thanks for the suggestions! I didn't think about the collection literals solution.

@leafpetersen

though we're exploring adding the option of using statically checked variance annotations to control this better

Is there any open issue about this? I'm interested in the topic.

@eernstg
Copy link
Member

eernstg commented Apr 15, 2021

#524, #753.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

4 participants