dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.25k stars 1.58k forks source link

Request: dart:collection UnmodifiableSetView #8321

Open nex3 opened 11 years ago

nex3 commented 11 years ago

The dart:async library contains nice StreamView and StreamSinkView classes for providing extendable views over existing objects. It would be nice if the same sorts of classes were provided for various collection types -- especially if there were variants that were read-only.

nex3 commented 11 years ago

Added Library-Collection label.

kevmoo commented 11 years ago

Natalie: does UnmodifiableListView do the trick?

http://api.dartlang.org/docs/releases/latest/dart_collection/UnmodifiableListView.html

nex3 commented 11 years ago

That's definitely the sort of thing I'm looking for, yes. Read-only views for Maps and Sets are still missing, though.

nex3 commented 11 years ago

Upon further inspection, UnmodifiableListView isn't really enough... it only implements Iterable, not Collection (let alone List).

floitschG commented 11 years ago

UnmodifiableListView extends UnmodifiableListBase which is a typedef for mixing in ListBase with UnmodifiableListMixin. ListBase is a typedef for mixing in Object with ListMixin. and ListMixin implements List. -> UnmodifiableListView should implement List.

kevmoo commented 11 years ago

The editor and dart_analyzer report this as not being the case at

0.4.7.5 r21658

Is this an analyzer bug?

kevmoo commented 11 years ago

Yes, it is an analyzer bug. I made a smoke test that ran through the vm,dart2js,dart2dart with testCases exposed as UnmodifiableListView.

expect(testCases is Object, isTrue); expect(testCases is List, isTrue); expect(testCases is Map, isFalse); expect(testCases is UnmodifiableListView, isTrue);

nex3 commented 11 years ago

I'm marking this as high priority, since if unittest is going to use UnmodifiableListView then it's going to cause analyzer warnings for practically every project.


Removed Priority-Medium, Area-Library, Library-Collection labels. Added Priority-High, Area-Analyzer labels.

kevmoo commented 11 years ago

See issue #10055

nex3 commented 11 years ago

I'm reassigning this to dart:collections, since issue #10055 is now tracking the analyzer bug. View classes are still needed for Map, Queue, and Set.


Removed Priority-High, Area-Analyzer labels. Added Priority-Medium, Area-Library, Library-Collection labels.

kevmoo commented 11 years ago

Removed Type-Defect label. Added Type-Enhancement label. Marked this as being blocked by #10128.

kevmoo commented 10 years ago

UnmodifiableMapView has shipped. Just need a view for Set now


Changed the title to: "Request: dart:collection UnmodifiableSetView".

srawlins commented 6 years ago

UnmodifiablSetView is now provided by the collection package: https://pub.dartlang.org/documentation/collection/latest/collection/UnmodifiableSetView-class.html

nex3 commented 6 years ago

I'd like to keep this open. It's really confusing that all but one of the core collection types has an unmodifiable view in dart:collection.

Aetet commented 5 years ago

@kevmoo @nex3 Hey! Is there possible to add read only types to dart:collection? Right now you know about modifications at runtime phase, because they throw exceptions. That's not ergonomic.

I think there's 2 options:

  1. Make read only interface - like ReadOnlySetMixin, ReadOnlyListMixin
  2. Help analyzer recognize places where you try modify unmodifiable view and warn about it.
kevmoo commented 5 years ago

CC @lrhn

lrhn commented 5 years ago

@aetet Dart has considered read-only superinterfaces like ReadOnlySet before, and decided against it. The primary concern has always been that it's one extra user concern. Whenever you make a function which takes a List as argument, you'd have to consider whether it should be a ReadOnlyList. Whenever you return a list, you have to decide as well. And that's not the only variant of lists, we also have fixed-lengh lists, which are not completely unmodifiable.

In Dart 1, the current approach of having one List type (and Set and Map) and some implementations which implemented less than the entire interface, was a good fit for the language. It can be argued that in Dart 2, with stronger type checking, a more type-heavy approach would fit better, but we have decided not to go that way. The user-complexity concern still applies, and it would be a large breaking change to modify the existing APIs to accept or return ReadOnlyList in some places, because subclasses and use-places need to adapt too. (For example, if we introduce ReadOnlyList as a super-interface of List, and we change a function foo to return ReadOnlyList instead of List, then var x = foo(); if (something) x = []; would break.)

As for helping the analyzer recognize unmodifiable lists, that still requires a marker type of some sort.

It's not enough to add an empty marker interface (often considered an antipattern) for being unmodifiable and implement it on the unmodifiable collection implementation classes. That information is lost the moment you do List<int> myList = List.unmodifiable(...);. So, to make something visible to the analyzer, we need UnmodifiableList<int> myList = List.unmodifiable(...);, where UnmodifiableList is a subtype of List, one which is recognized specially by the analyzer, so that it can warn if you call modifying methods on it. That's still too intrusive, it introduces a new type with no semantic meaning, only to allow the analyzer to detect it. I guess the best, and least intrusive approach would be:

@unmodifiable
List<int> myList = ...;

which tells the analyzer that the variable contains an unmodifiable collection, and if the analyzer recognizes the collection, then it can warn about calls to modifying members. (If it doesn't know the interface, maybe it can warn about calls to members where the type variable occurs contravariantly).

In short: We have no plan to change the language or the libraries in this direction, but it might be possible for the analyzer to do something using metadata annotations (almost anything can be done that way).

Aetet commented 5 years ago

Thanks for explanation @lrhn

filiph commented 4 years ago

I'd like to come back to the initial problem.

Here's my case for having UnmodifiableSetView in dart:collection instead of a separate pub package:

I am all for keeping the core libraries clean. But I think the problems mentioned above outweigh the downsides of adding one additional class, especially since dart:collection already has 26 classes. (I would be way more wary about adding a class to an import that carries 5 or 10 classes.)

kevmoo commented 4 years ago

I see no practical reason to punt on this given the other classes we've shipped in this package!

trinarytree commented 3 years ago

+1 to nex3's comment: can we move UnmodifiableSetView from the collection package to dart:collection already? i just wasted a bunch of time trying to figure out where this basic data structure is. searching the api docs doesn't help and google search doesn't reveal the answer very quickly either. by far the most expected place for this would be dart:collection.