Change type requirements for {} being a set literal.
#129
Conversation
The existing rule had some issue. My immediate fix would have different issues. The `FutureOr` case is particularly annoying because `FutureOr<LinkedHashSet<X>>` as a context type should create a `Set<X>`, and `FutureOr<V>` is a super-type of `V` and we don't want to get into zig-zag problems (`Iterable<int>` is incomarable to `List<num>`, and `Iterable<int>` is incomparable to `FutureOr<List<int>>`). This change requires answering a negative universal property, but I believe the types in question must be available in the context type, so it should be simple (get the `T` type as the type parameter of `Iterable` if the context type implements iterable, get the `K` and `V` as the type parmeters of `Map` if the context type implements `Map`, and you only need to check those).
|
@lrhn can we land this? |
There was a problem hiding this comment.
LinkedHashSet and LinkedHashMap are both defined in dart:collection. So far the analyzer has only needed to know about dart:core and dart:async in order to analyze code. I don't know how big the impact on the analyzer would be of requiring it to be able to also access code from dart:collection, but it won't be free and it will likely postpone our ability to implement this feature until sometime in Q1.
Hmm, I agree that we probably can't modify the analyzer to adapt to this change before January--most members of the analyzer team will be leaving for the holidays later this week and there's too much other work on our plate. However, regarding the analyzer's need to know about The change described in this PR also only affects a rare corner case. So if the language team decides to land this PR, I think it would be fine to treat it the same way; as a bug in the analyzer that we don't necessarily need to fix before enabling the "set literals" feature. |
|
Note: I filed dart-lang/sdk#35439 as a reminder to change the analyzer implementation. |
Use
LinkedHashSet<Null>as the comparison, notSet<Null>. This ensures that:is valid. Since we guarantee to return a
LinkedHashSet, that is perfectly fine code.