Skip to content

possibly warn when a class extends ListBase in null safe code, and doesn't implement add? #46646

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

Open
annagrin opened this issue Jul 16, 2021 · 7 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes P4 type-enhancement A request for a change that isn't a bug

Comments

@annagrin
Copy link
Contributor

annagrin commented Jul 16, 2021

Using

import 'dart:collection';

void main() {
  {
    var list = [];
    // works
    list.length = 1;
    print('Length: ${list.length}');
  }

  {
    var list = <Object>[];
    // throws
    list.length = 1;
    print('Length: ${list.length}');
  }
}

Expected
I suspect that the list cannot grow by adding Null elements due to null safety, so the code should throw, but it would be great to have a better error message. Even better, is it possible to give a compilation error instead?

Actual

Unhandled exception:
    type 'Null' is not a subtype of type 'Object' in type cast
    #0      List.length= (dart:core-patch/growable_array.dart:222:12)
    #1      main (file:///Users/annagrin/source/try/notAListBug/main.dart:14:10)
    #2      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:283:19)
    #3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

Dart SDK version
Dart SDK version: 2.14.0-321.0.dev (dev) (Thu Jul 15 08:55:11 2021 -0700) on "macos_x64"

Platform
VM and web

@annagrin annagrin changed the title type 'Null' is not a subtype of type 'Object' in type cast is thrown on List.length setter "type 'Null' is not a subtype of type 'Object' in type cast" is thrown on 'List.length' setter Jul 16, 2021
@lrhn
Copy link
Member

lrhn commented Jul 17, 2021

That's expected behavior for null safety. You can only increase the length of a list by setting length if the list element type is nullable. Otherwise you won't be allowed to assign null to the fresh slots.

You can reduce the length safely, so it's hard to make it a compile-time error.

The error could be more informative, but that also risks adding an overhead on the valid operations.

@annagrin
Copy link
Contributor Author

annagrin commented Jul 17, 2021

Thanks @lrhn for the explanation, it confirms what I suspected. During my tries to make it work I saw errors on non-growable lists that are much more explanatory - would it be possible to make it similar, for example , give an error that mentions that I am trying to grow a list by adding a null element to a list of non-nullable objects?

Some context - the original example was a ListBase implementation that was overriding its methods and forwarding them to an internal list of non-nullable objects. Calling add(7) on that list implementation was implicitly calling the length setter and throwing, making the error harder to understand.

@lrhn
Copy link
Member

lrhn commented Jul 18, 2021

Yes, the ListBase.add method is problematic. Pre-null-safety it was possible to implement add using .length= and []=, but with null safety that no longer works, and you have to implement add as well when using ListBase.
Since the platform libraries are shared between null safe and legacy code, the ListBase.add method still exists for backwards compatibility, but it will be removed when we (eventually) drop support for non-null-safe code.

(Could we have the analyzer warn when a class extends ListBase in null safe code, and doesn't implement add?)

@lrhn lrhn added legacy-area-analyzer Use area-devexp instead. type-enhancement A request for a change that isn't a bug labels Jul 18, 2021
@devoncarew devoncarew added the P3 A lower priority bug or feature request label Jul 19, 2021
@devoncarew devoncarew changed the title "type 'Null' is not a subtype of type 'Object' in type cast" is thrown on 'List.length' setter possibly warn when a class extends ListBase in null safe code, and doesn't implement add? Jul 19, 2021
@cedvdb
Copy link
Contributor

cedvdb commented Jun 23, 2022

So it's not possible to add on a growable list (with null safety), there is no override that can make this work ? If that's the case the documentation could be a bit clearer on that. If it is possible it's not exactly clear how without making the inner list nullable.

@lrhn
Copy link
Member

lrhn commented Jun 23, 2022

@cedvdb Not sure what you are asking.

The add on a platform growable list works perfectly well.

The add method of ListMixin/ListBase is implemented in terms of incrementing the length of the list, then storing the value in the new slot.
Whether that works or not depends on how the class implements the abstract add and length= operations.
It's just that some existing implementations won't work directly when migrated to null safety. For example, using a platform growable list with a non-nullable type as backing store.

@cedvdb
Copy link
Contributor

cedvdb commented Jun 23, 2022

Sorry, I'm also having an hard time parsing what's being said here. I meant that the following snippet won't work without overriding the add, and it is not exactly clear (or maybe it's obvious and I'm a dumbass) how to override it. I feel like it's obvious and I'm a dumbass tbh.

import 'dart:collection';

void main() {
  final list = GrowingList();
  list.add(1);
}

class A {}


class GrowingList extends ListBase<int> {
  final List<int> _innerList = [];
  
  GrowingList();

  @override
  set length(int newLength) { _innerList.length = newLength; }
  @override
  int get length => _innerList.length;
  @override
  int operator [](int index) => _innerList[index];
  @override
  void operator []=(int index, int value) { _innerList[index] = value; }
}

The only thing that worked for me was to make the inner list nullable. Ence my previous comment, which said if this cannot be done because increasing the length adds a null slot, then I don't think the doc is clear enough there.

@lrhn
Copy link
Member

lrhn commented Jun 24, 2022

Correct, that class will throw in add because it tries to increase the length of a List<int>. It will throw if you do GrowingList()..length = 2 too, because that also calls _innerList.length=.

@srawlins srawlins added devexp-warning Issues with the analyzer's Warning codes P4 and removed P3 A lower priority bug or feature request labels Aug 21, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes P4 type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants