Partial Map pattern matching #2861
Comments
|
No, you have to use A map pattern without a The only workaround I can think of is to introduce a horde of extension getters: typedef JsonMap = Map<String, Object?>;
extension JsonGetters on JsonMap {
Object? get token => this["token"];
Object? get data => this["data"];
Object? get expires => this["expires"];
}and then do: if (result case JsonMap(data: JsonMap(: String token))) {
print("TOKEN $token");
}That allows you to treat a string map as an object with getters, and therefore use an object-pattern, which does not expect to match length. It also doesn't check whether the map has that key at all, which is why the getter can return You have to declare all the getter names you want to use. (If we allowed arbitrary selectors left of the |
|
can't we have a spread operator before the initial map so all maps are treated as partial? ie if (result case ...{'data': {'token': String token}}) {} |
|
We could do that, yes. In principle, we can do anything if it's not ambiguous and is feasible to implement. :) But in practice, I suspect that that syntactic sugar wouldn't carry its weight. It's simpler and probably also clearer and better to just put |
|
@munificent I am creating a package that is using |
That's definitely a valid use case, but it might not be the most common one.
I could have sworn there was an issue where we discussed this choice but I dug around a bunch and can't find it. But the short summary is that we did discuss this and weighed the pros and cons as best as we could. There are some good arguments to make maps soft by default: It means it's a non-breaking change to add new unused map keys in JSON and other data structures that are being consumed by map patterns. However:
Given that, we felt it made the most sense to default to strict. It means the code is a little more verbose when you want soft matching, but it's hopefully easier to learn and less error-prone. |
|
@munificent imagine that you have a worker that sends email through a service and that service is well known, thus the API shouldn't change without notice. Relying on JSON structure is asking for trouble. Having this in mind I would ask to have a lint that warns the dev to always add the
if (map case {'foo': int foo}!) {}
if (map case! {'foo': int foo}) {} |
if (map case {'foo': int foo}!) {}Already means "throw if if (map case! {'foo': int foo}) {}Should be on the map pattern itself, not the if (map case !{'foo': int foo}) {}Possible, but not particularly readable. I'm personally fine with requiring The one issue is that the error case happens dynamically. If all your tests happen to pass a map with just the expected keys, nothing prompts you to add the But if the default was the other way around, it would be equally easy to forget the If you put |
|
I understand your point. You want to make My point is that no one expects a
If I understood it correctly it would be the same as Please, give a thought about it. |
It's probably going to be a best practice to use I agree that this is a worrisome corner of the proposal. Like @lrhn notes, we can't use a postfix Syntax design is hard. Picking defaults is harder. Picking defaults for what syntax means is doubly hard. If we really think that almost every map pattern should be non-strict, maybe the right answer is to make that the default. And if you really do want a strict map, do: switch (map) {
case {'key': 1, 'other': 2} && Map(length: 2): ...
}But that's really ugly if it turns out that users will often want strict matching. :-/ |
the point @munificent is that
As postfix switch (map) {
case {'key': 1, 'other': 2}!: ...
} |
The problem is that the exact syntax you propose here already does mean something. It's a null-assert pattern whose inner pattern is a map pattern. |
|
I do get where this comes from. It makes sense when you think of a I am creating a Typescript declaration file transpiller for Dart. It will basically create JS interop from
As it is a new package I am using Dart 3 and most of the incoming features I can at this time. So in my perspective the syntax seems more valuable to use as |
|
I'll bring this up at the language meeting this week. I share your concern, but I'm not certain what a better approach would look like. Also, we have almost no time to make changes so it may be too late anyway, but I'll see what the rest of the team things. |
|
@munificent thanks =] |
|
OK, I spent a bunch of time talking about this today with @leafpetersen, @jacob314, and @jakemac53. We'll discuss it with the rest of the language team tomorrow, but I wanted to write up my thoughts first: TL;DR: I think we should change map patterns to do loose matching. Concretely, I propose:
ReasoningFirst, let me go through the reasons why we made them strict in the current proposal and why I think those reasons aren't compelling: Old proposal reasoningIt's consistent with list patternsList patterns check the length by default, and you need to do If lists weren't strict by default, we'd have to figure out which elements get bound when the list pattern has fewer elements than the incoming list object. We'd probably say that it just matches a prefix. But if the list has more elements than you expect, what reason do you have to assume that the unexpected ones happen to be at the end? For all you know, they could have been prepended or interspersed with the elements you care about. This is exactly why That doesn't apply to map patterns. In a map, every key is independent of any others. Adding more keys to a map doesn't "adjust" or "shift" any of the other keys. In fact, this is why you can only place So I don't see a very compelling claim that map patterns should behave like list patterns. There's an argument that just being consistent with another kind of pattern is valuable. But the destructuring patterns already each have their own behavior and policy:
So, already, it seems like every pattern has a policy and behavior that's tuned for its own particular needs. Given that, I think map patterns should behave the way that makes the most sense for maps, and not try to be list-like when they aren't lists. If anything, they're closer to object patterns. It provides an obvious syntax for both loose and strict matchingIf we default to strict, then it makes sense to use But do we really? You can just use a guard: switch (map) {
case {'a': 1, 'b': 2} when map.length == 2: ...
}Or if you want to be cute: switch (map) {
case {'a': 1, 'b': 2} && Map(length: 2): ...
}But the more fundamental question to me is how often that behavior is even desired? I suspect that most uses of maps and destructuring in Dart are one of:
If my suspicion is correct, then users will rarely need to opt into strict checking anyway. And, if they want to, then just doing an explicit Map pattern order is less importantConsider this example with the current proposal: print(switch (map) {
{} => 'empty',
{'a': _} => 'just a',
{'b': _} => 'just b',
{'a': _, 'b': _} => 'a and b',
{...} => 'something else',
}This basically works like the case bodies say. The first If we make map patterns loose, this switch doesn't do what you want. The first print(switch (map) {
{'a': _, 'b': _} => 'a and b',
{'a': _} => 'just a',
{'b': _} => 'just b',
{} when map.isEmpty => 'empty',
{} => 'something else',
}I do think this is one of the merits of the current proposal. If you think of maps as fairly "closed" and "disjoint" data structures, then the looser checking could trip you up. You'll have to think about ordering your map cases a little more carefully. But, if you get it wrong, the compiler should generally tell you by reporting that some later cases are unreachable. However, I think most uses of maps don't treat them like a closed-world data structure. And, certainly, the language doesn't. That's why the The current proposal encourages you to think of them as being record-like with a small number of well-known keys, but I think that's the wrong mental model. Consider an analogous switch over an object with a couple of getters: print(switch (obj) {
Obj() => '?',
Obj(a: true) => 'a is true',
Obj(b: true) => 'b is true',
Obj(a: true, b: true) => 'a and b are true',
}Here, it's clearer that the order is wrong. With an object pattern, you tend to want the cases with more subpatterns first since they are asking more specific questions about the underlying object. I think that's the right mental model for maps too. A map is an open-ended data structure. When you query a key on it, that query is unrelated to other keys you might query. However, making map patterns loose does mean that an empty map pattern is a footgun. That's why I suggest disallowing them. The point of a map pattern is to look up values by key. If you don't have any keys to look up, don't use a map pattern. Proposed change reasoningI think there is merit to the reasons we went with the old proposal, but overall I find the arguments for loose matching to be much stronger. In particular: Loose matching is what users want most of the timeThis is the most important part, by a large margin. When I look at imperative code today working with maps, it often looks like: if (map.containsKey('a') && map['b'] == 'some value') {
// Do stuff...
}Note what it's not doing? It's not checking the length. Almost all of the code I see reading from maps is like this. And this is exactly the kind of code that I think should be migrated to map patterns. That suggests that the syntax should optimize for this use case. It's certainly the case that if we keep the current proposal, we'll have to tell users to be very careful when migrating their imperative code that uses maps to map patterns. Because if the code doesn't do any length checks, they need to remember to add Strict matching isn't helpful in irrefutable contextsYou can use map patterns in variable declarations and assignments too: var {'a': a, 'b': b} = someMap;In these contexts if the length check fails, it throws a runtime exception. That's not very helpful. It is helpful to throw an exception if the key isn't present. That makes sense: we can't complete the requested operation because the data isn't there. But if all of the keys you're looking up are present and accounted for, throwing a spurious runtime exception because it happens to have extra keys that the variable declaration or assignment doesn't care about doesn't add a lot of value. The clear trend for Dart has been away from code that can fail at runtime. But this is a case where the default behavior can lead to a runtime exception and you have to opt out of it by using If we ship the current proposal, I'm certain we'll end up with a lint that says "Do use Strict matching isn't even well definedThe idea with strict matching is that a map pattern should match only maps that have the matched set of keys and no other. But "no other keys" isn't actually a well-defined concept across all reasonable implementations of The best we could come up with is "the test(Map<String, String> map) {
switch (map) {
case {'a': var littleA, 'A': var bigA}: print('$littleA $bigA');
}
}With strict matching, this will only match a map where:
But things like [CaseInsensitiveMap][] exist and are reasonable. If you were to call that function with: var map = CaseInsensitiveMap.from({'a': 'hi'});Then I think there's a good argument that it should match that case even though it's length is (This conceptual fragility is also why ConclusionOverall, I feel fairly confident that loose matching is the behavior users actually want almost all of the time. It's semantically simpler and avoids stepping into weird policy questions about the connection between map Loose checking makes maps safer to use in irrefutable contexts. If a user does want strict matching, they can opt in to it by writing a Of course, it would have been great to think through all of this months ago. We are very very close to shipping this feature, so it's hard to change. But I really don't want to ship strict checking if it will just end up being frustrating and error-prone for users every single time they use map patterns. I believe the scope of the change here is relatively small. It's:
There would be a lot of tests to update and some docs. I'm not underestimating the cost of the churn here. But I do believe that our future selves will consider it worth it. I deliberately do not propose any new syntax for opting in to strict checking to minimize the cost of the change. In a future release, we could consider adding that if it's something users want. |
|
The reasoning is sound. My only nit is
... followed by examples using empty map patterns as if it's the most natural thing in the world. 😁 Let's just keep them, for consistency. As you say, a map is defined by its (It's still a very questionable choice to make such a map. Just because something allows lookup, it doesn't have to be a I'm more open to the point that maps are intended for random-access, where lists are intended for iteration. You care about the length of a list. You don't, as often, care about all the keys of a map, just whether a particular one is there. When you do care about all the keys of a map, you'll be doing it by iteration, not by knowing the all keys up-front. Maps are more like objects with properties that are individually useful. They have a key to give them significance and distinguish them from other entries. It makes sense to look at a single entry. List elements have positions, but that's mostly an ordering, not a significant key. Looking at a specific element only is weird, unless its position is special in some way (like And because It does prompt the question of whether a trailing |
|
After giving some thought, I personally don't find loose matching compelling. It feels that the developer intention is hidden, propitious to errors. With strong matching, what the developer write is what that object is (an explicit
final obj = { 'a' : 1, 'b ': 2, 'c' : 3, 'd ': 4, 'e' : 5};
String json = switch (obj) {
case { "a": String a } => jsonEncode(obj),
case { "b": String b } => jsonEncode(obj),
_: throw Error('Bad shape'),
}
// we don't want to json serialize {"a" : 1, plus a million fields} and send it to a client. Send 'a' XOR 'b'A different topic but this feels like a related design problem present in TypeScript that is hard to express a
Personally, treating I would prefer |
Imagine that the service you are using is passing though a hard time regarding performance and they insert some harmless profilling info in their response like Just like Bob said, we never check for a map length... do we? You can't trust a thanks @munificent |
|
In that case I would explicity add IMO it's dangerous to implicity "ignore/consume" variables. final obj = { 'a': 1, 'b': 2};
switch (obj) {
case { 'a': String a }: print('a');
case { 'a': String a, 'b': String b } : print('ab');
}With this proposal this code will silently print |
Would you rather fix your logic while developing or in production? You can check for ordering of statements at any time. |
If you know that the model is subject to change, use Overall, I am not sure about what's the best approach. On the other side, I agree with the "Strict matching isn't helpful in irrefutable contexts" point, and I think that in the specific given example we would want loose matching to avoid this kind of runtime exceptions. Overall, I am more inclined to strict matching, but I see some value in loose matching too. |
|
We talked about this in the language meeting this morning and the team is on board with changing it. We need to make sure the implementers are OK with it because it's very late in the cycle to be making a change, but so far it seems promising. Thanks for pushing on this @jodinathan. I think this will make Dart a better language and we wouldn't have revisited this design choice without you and others bringing it to our attention.
With this proposal, you can still get the exact previous behavior by doing: final obj = { 'a' : 1, 'b ': 2, 'c' : 3, 'd ': 4, 'e' : 5};
String json = switch (obj) {
case { "a": String a } when json.length == 1 => jsonEncode(obj),
case { "b": String b } when json.length == 1 => jsonEncode(obj),
_ => throw Error('Bad shape'),
}Or if you really want to make it clearer that you're doing XOR: final obj = { 'a' : 1, 'b ': 2, 'c' : 3, 'd ': 4, 'e' : 5};
String json = switch (obj) {
case { "a": String a } when !json.containsKey('b') => jsonEncode(obj),
case { "b": String b } when !json.containsKey('a') => jsonEncode(obj),
_ => throw Error('Bad shape'),
}Or simply handle the OR case first: final obj = { 'a' : 1, 'b ': 2, 'c' : 3, 'd ': 4, 'e' : 5};
String json = switch (obj) {
case { "a": String _, "b": String _ } => throw Error('Bad shape'),
case { "a": String a } => jsonEncode(obj),
case { "b": String b } => jsonEncode(obj),
_ => throw Error('Bad shape'),
}The latter is what I'd probably do because it makes it clear which keys are mutually exclusive while gracefully handling extra keys you don't care about.
In that case, it's better to create a map yourself with just the data you want instead of blindly forwarding a data structure you don't control: final obj = { 'a' : 1, 'b ': 2, 'c' : 3, 'd ': 4, 'e' : 5};
String json = switch (obj) {
case { "a": String _, "b": String _ } => throw Error('Bad shape'),
case { "a": String a } => jsonEncode({'a': a}),
case { "b": String b } => jsonEncode({'b': b}),
_ => throw Error('Bad shape'),
}
With this proposal (and with the current behavior if you have
This is definitely true. But the same is true for object patterns, which I thinkn map patterns are conceptually closest to for most common use cases. I do worry that this will be a little error-prone, but the exhaustiveness and reachability errors should help. There's a fundamental concept here where when a user looks at a pattern, do they see it as describing the entire object being matched, or do they see it as describing a filter over a set of objects to match? If they have the former mindset, then shorter patterns generally describe more specific objects and match fewer things. That leads to a mindset where A real challenge with pattern matching is that both mindsets are intuitive but one or the other can lead you astray in some situations. I don't think there's a perfect solution but we're trying to pick what we think is the most intuitive for most uses and the most terse for most common use cases. Balancing all that is hard. Doing it in a language that wasn't already designed around patterns is really hard. :) |
… pattern. Bug: dart-lang/language#2861 Change-Id: I00ccb3ea03aa476f96c2ecf3e3a9e13bd4926193 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291940 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Reviewed-by: Marya Belanger <mbelanger@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
The code below prints
NOT TOKEN.It will print the token only if I change
result case {'data': {'token': String token}}toresult case {'data': {'token': String token, ...}}.Can I check for the match without using the
...or'expires': int _?Sometimes the API you are fetching changes but not the part that you use. To prevent any exception I would have to add
...in any{}checksThe text was updated successfully, but these errors were encountered: