-
Notifications
You must be signed in to change notification settings - Fork 205
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
Allow the type of an object pattern to be inferred when specified as _
#4124
Comments
Seems like a duplicate of #2563. |
Good catch, thanks! It is similar, but that one was dropped because it used a syntax which is indistinguishable from record patterns. At least, I haven't heard anybody arguing in favor of the record-syntax-means-inferred-object-pattern proposal for a long, long time. On the other hand, this comment from @samandmoore and this comment from @mraleph in #2563 already mention the syntax This means that this issue is indeed an attempt to breathe new life into an idea which was mentioned in #2563, and also to give this idea a dedicated issue of its own. I think this should be sufficient to justify having this issue. |
I think the interpretation where an object pattern of the form If we interpret the |
Would we also be able to reduce in scenarios like: void main() {
Map<String, int> map = {};
for (var MapEntry(:key, :value) in map.entries) {
print('$key : $value');
}
} Where we would then be able to do: for (var _(:key, :value) in map.entries) { ? |
I propose the syntax That matches the proposed |
Tangentially related: if dot stands for the context type, this opens the way to defining a block expression syntax in two steps:
|
I'm not really a fan of Its kinda small and easily forgotten, whereas Alternatively, it can also mean "infer" which could be useful for something like
notice the difference? final int i = .parse('1');
enum X {uno, dos}
final X x = .uno;
class const Foo(String str);
void thing() {
// static member. `.new(...)` becomes a shorthand.
final Foo foo = .new(switch (x) {
// static members
.uno => 'one',
.dos => 'two', // implies `_.dos` which implies `X.dos`
});
return switch (foo /* as Foo */) {
// infer type `_ = Foo`
_(str: 'one') => 1,
_(str: 'two') => 2,
_(:final str) => str.length,
};
} after all, Note: turning |
I know this approach is probably easier to parse, but syntax wise I prefer #2563. for (var _(:key, :value) in map.entries) {} for (var (:key, :value) in map.entries) {} That said, if we go with this approach of using a symbol to denote "infer this", I agree with @lrhn that
|
we cant use the second option because that's a record. i don't see how Since we're talking about pattern matching, 2 is irrelevant. additionally, for the case of extension KVEntries<K, V> on Map<K, V> {
Iterable<(K, V)> get kv => entries.map((e) => (e.key, e.value));
} The "best" solution for this in particular is to turn |
It's not impossible. If you read the discussion in the aforementioned issue, @munificent proposes disambiguation rules that can work, and IMO it would be better.
It's not irrelevant. Even if context can disambiguate syntax, pragmatically speaking, the readability of the syntax is better and easier to quickly grasp if each syntax has a single meaning. This is inevitable in some cases, but this is a case where it can be avoided. |
By that logic we can't use . either, since it would then have multiple different meanings. By using _() it does not overlap with _-as-wildcard nor -as-nonbinding-variable because neither can have parens the way this does, and it also cleanly lines up with explicitly requesting inference in type parameters, which is proposed to use <> which has similar semantics to this, where it means "the implied type" If we can just use record types outright, then that could be neat, but it feels problematic. It becomes unclear through scanning if the value is a record or an object of implied type. |
Yeah, I never said I think Not as terse as what was suggested here, but if we consider something like #3616, we could do for (var .new(:key, :value) in map.entries) {} |
Thats interesting, but wouldn't work, because we aren't checking constructors, (even though it looks like it) we're pattern matching. Therefore, .new() gives the wrong impression. I am of the opinion that based on the currently proposed options, If anyone has better ideas than this (or hell, worse ones - just to get those juices flowing) then let's see 'em! |
I would certainly be worried about reusing the syntax for record patterns (at least for the ones with named getters) to denote an object pattern with an inferred type. It might be unambiguous for an analyzer and a compiler, but to me it looks a lot like a footgun. For example: import 'dart:math';
void main() {
var s = 'Hello!';
switch (s) {
case (length: 6): print('Reached: Object pattern');
}
var r = (length: 6);
switch (r) {
case (length: 6): print('Reached: Record pattern');
}
dynamic d = Random().nextBool() ? s : r;
switch (d) {
case (length: 6): print('Reached?');
}
} For the last one, the matched value type is This would presumably imply that some patterns intended as record patterns will be implicitly reinterpreted as dynamic object patterns, which again implies that they will match the records they are intended to match, plus all records that have the required getters satisfying the given requirements, plus any number of additional getters: It's not a record pattern any more, and Similarly, we can have spurious matches where I don't think this kind of confusion will help anybody. So I'll continue to recommend that we use an identifier, preferably Note that The syntax |
It's "no new grammar", but it changes the existing meaning of the term, treating I can probably get used to it, but my initial reading of class _ {
int get secret => 42;
}
bool isItSecret(Object? o) {
if (o case _(secret: 42)) return true;
print("Not secret");
return false;
}
void main() {
print(isItSecret((secret: 42))); // "Not secret", "false"
print(isItSecret(_())); // "true"
} I'm not sure a type being named |
Both And so i would like to propose similar, but imo more readable variation of this syntax, where instead of using for (var <>(:key, :value) in map.entries) {}
if (o case <>(secret: 42)) return true;
return switch (foo /* as Foo */) {
// infer type `<> = Foo`
<>(str: 'one') => 1,
<>(str: 'two') => 2,
<>(:final str) => str.length,
}; You can say that it is kind of long, but i'd say that compared to the full class name, it is still short, and it retains some readibility. anyway, this proposal is more of what-if, to show a different possible direction for the syntax |
I still very much like #2563 and would like to do it. I've just been too busy with other stuff to push on it. Agree with your earlier comment that #2563 could be a footgun in a couple of places, especially when the scrutinee has type If only ASCII had more bracket characters so we could have come up with something different for records... |
@lrhn I'd argue that "match anything" isnt to different to "match this". switch (thing as T) {
T it => it // is T
// vs
_ => thing // is T
} you find they match the same. and if we did I also really like class Foo<T extends Bar<R>, R> {}
class Bar<R> {}
final foo = Foo<Bar<int>, _>(); // Foo<Bar<int>, int>
class BarString extends Bar<String> {}
final foo2 = Foo<BarString, _>(); // Foo<BarString, String> which has very similar meaning to using It's a placeholder, and what's a wildcard if not a placeholder? To me, it means "something goes here". or perhaps "anything goes here" But because we have a type system, we happen to know that we can promote "something/anything" to the context type. Its nothing more than an expansion of its existing usage. ps: pss: |
It's inconsistent. |
...Then I would first recommend making How does that even interact with the wildcard? What happens if you have a literal type by that name? A constant value by that name? What do you mean by "local" declarations? Isn't all usage of |
Honestly speaking, i don't understand why Also, i also hate the fact that As such, wouldn't it be better to disallow identieffiers I think that the new possibilities for syntax gained this way completely compensate for breaking someones code, mainly because the fix is to just do a simple find-and-replace. |
OK, we can fight over syntax forever, so let me ask about another thing, assuming that there is some syntactic marker to say "this is an object pattern whose type is inferred (not a record pattern)": Would you recommend using such a feature liberally, all over the place? Or would you consider it to be detrimental to the readability of the code, only to be used when it's absolutely obvious which type is being inferred? |
Assuming some syntax for not having to write the matched value type in an object pattern:
I don't know if I'd recommend anything, but I would use it liberally, all over the place. You can take that as endorsement, even if it's not a recommendation. I'd probably also only use it if it is obvious which type is being inferred, but it usually is. So, all over the place. A switch is often one of two kinds:
The third option is a "parsing switch" that checks a value, often of type In declaration patters and plain destructuring, the matched value type is usually also obvious. So I'd use it every time I do a destructuring or a value match, a switch where all the cases have the same type, where switching on the matched value type is meaningful. |
Considering that the inferred type pattern would be used to destructure an object, i would only allow it in cases where the type is obvious, since the developer needs to directly interact with the object in question. I wouldn't allow it in unreadable cases, purely because while you can just hover over the variable to get it's type in IDE, that doesn't work in Github, Gitlab, StackOverflow ... I want it to be readable because much of the work of developers is not done in IDE. Edit - i basically agree with the usecases that @lrhn outlined, all of which are very easy to understand |
Could we go even further and intersect types in our inference? For instance: void main() {
final Foo foo = getFoo();
switch (foo) {
_(:final a) => ..., // Equivalent to Bar(:final a) || Baz(:final a) => ...
Qux(:final b) => ...,
}
}
sealed class Foo {}
final class Bar {
final int a;
}
final class Baz {
final int a;
}
final class Qux {
final int b;
} |
I don't like that. it is imo unintuitive and you can get the same by correctly ordering the cases (just like you have to do now) void main() {
final Foo foo = getFoo();
// same as Bar(:final a) || Baz(:final a), because of early Qux return
switch (foo) {
Qux(:final b) => ...,
_(:final a) => ...,
}
}
sealed class Foo {}
final class Bar {
final int a;
}
final class Baz {
final int a;
}
final class Qux {
final int b;
} |
Does this code work today? |
@mateusfccp yes it does (after adding all the definitions). here is the correct code void main() {
final Foo foo = getFoo();
// same as Bar(:final a) || Baz(:final a), because of early Qux return
print(switch (foo) {
Qux(:final b) => "its Qux",
_ => "its else"
});
}
Foo getFoo(){
return Qux();
}
sealed class Foo {
int a = 1;
}
final class Bar extends Foo {
}
final class Baz extends Foo {
Baz(){
super.a = 2;
}
}
final class Qux extends Foo{
final int b = 0;
} Run it in dartpad if you want. |
Yes, but this is completely different from what I suggested. But thinking again, I don't think it would be too useful, useless they were unrelated types that could be exhaustively checked. |
switch (foo) {
Qux(:final b) => ...,
_(:final a) => ...,
} To me, the expression |
No, "any class using getter a" would be It feels like some of you might be mistaking _ for dynamic. By definition,
|
No. The underscore is not always a wildcard. Sometimes it's a valid identifier. And it would be treated as a valid, very concrete, identifier in the class _ {
final a=1;
}
main() {
var x = _();
switch(x) {
case _(:final a):
print(a);
}
} |
DISCLAIMER: this is more of a throwaway comment
Welcome to language design, where ideas are spawning so quickly, that rather than creating new issue, people just argue about it in a sort of (not really) related issue. Welcome to language design, where we don't argue behaviour, but cosmetics. Please let's get back to the issue at hand and discuss the allowed behaviour and the implementation so that it is as easy for developer to integrate into existing code as possible. Also, Dart doesn't have structural typing, aka "anything with getter a", and adding it only to this single pattern, that is already more difficult to read, is a bad decision. Furthermore, there were already some proposals for From application design, this functionality is what classes are for. They give you both the structure and the context of the data. And with that you can already do "anything* with getter a". *anything in with the correct context / class |
@hydro63 : I found your earlier post where you proposed |
I would, yes. A valid criticism of the current pattern syntax is that if you just want to destructure an object but don't want to do a type test, there's no real way to express that. Say you get a string and want to destructure the length: String getThing() => ...
main() {
switch (getThing()) {
case String(:var length): ...
}
} Later, someone changes Object getThing() => ... Now the behavior of that switch is different. It doesn't have a compile error (because it's a switch statement, so doesn't have to be exhaustive), but now instead of simply destructuring, it also does a type test. There's no way to say "just destructure the object but don't type test". If we had "inferred" object patterns, that would give you a way to express that. Then, when you see a named object pattern, it's clear that the intent is to type test and downcast. So, yes, I would use inferred object patterns liberally whenever I don't want to type test. |
@tatumizer wrote:
I'd very much prefer if it is never used as such. We allow it for non-local declarations (including Here are some cases where We may omit the name of a local variable or parameter because it shouldn't be accessed anyway. We may omit the name from an identifier pattern by using the identifier It has been suggested that an actual argument for an optional positional parameter could be omitted even if it is not the last positional argument (so we would call As this illustrates, the usages of I think this perspective makes Of course, we could use a syntax like Similarly, Hence, I still prefer |
I think I'd prefer to keep using Doesn't mean you should use I'm warming up to a pattern of Then I'd still use For omitting arguments, I'd use Ok, And |
If we let var p = Person(name: 'Joe', ...);
switch (p) {
case _(name: 'Joe'): ...
} If this is a valid program, then consider Person p = _(name: 'Joe'); Is this a valid program? If not, why not? |
That's tempting! 😄 We just discussed shortcuts in the language team. Several of the proposals about shortcuts would allow this: Person p = .new(name: 'Joe'); // Call the constructor whose name is `Person` and `Person.new`. I would actually expect |
Tempting? Then what stops you from succumbing to the temptation? 😄 Really, why bother with |
What if we had this? // Library
sealed class Foo {}
final class Bar implements Foo {
const Bar(String a);
}
final class Baz implements Foo {
const Baz(int b);
} // Client
Foo f = _("String"); // Imply Bar
Foo f2 = _(10); // Imply Baz We would want it to be a static error when the constructors have conflicting signatures, though. |
Foo f = _("String"); // Imply Bar
Foo f2 = _(10); // Imply Baz I don't like this. I am against inferring subclasses. This feels like a footgun waiting to be fired. Right now, it is somewhat easy to understand what is being inferred, but if you had more than one argument, optional argument, or named parameters in there, it would take a lot of effort to guess what is inferred. And that's not even talking about having more than 2 subclasses that extend the desired class. You could say, that it isn't a problem if it's not obvious what is inferred, since the IDE infers can infer it for the developer, but that's a really stupid argument. Ideally, you don't want to rely on IDE too much, since significant portion of developer's time is not being spent in IDE. For example, code reviews, fix searching (StackOverflow), or reading documentation are done in a browser, not an IDE. You want the developer to be able to read stuff that's just plain text, with no hints. |
I think either I would personally use this frequently, and my hunch is that it would get enough usage in general to become the recommended pattern. I'd imagine arguments for or against using this will be similar to the arguments for using I don't think we should expand the scope of this feature, or this discussion, beyond pattern matching. |
OK, I agree that we should stick to patterns in this issue. Discussions about constructor invocations using In any case, it's a very interesting idea that there are additional syntactic positions where we could use |
Ideas similar to this one have been discussed already when patterns were introduced, but I don't think there's an issue where the proposal has been presented in a concrete form.
We could make object patterns a bit more concise and convenient by using a wildcard
_
to indicate the type of the object pattern when it can be inferred:(I'm assuming that we have primary constructors, hopefully we will have them soon.)
It might be claimed that
_
causes the code to be less readable than it would have been with the explicit type. However, it's worth noting that when the_
occurs repeatedly at the top level in cases like the above example then it is easy to see that every_
stands for the same type, and it must be the static type of the scrutinee. Similarly, when_
is used in a nested object pattern, e.g.,PairClassWithATerriblyLongName(x: _(isEven: true))
, the type of the nested object pattern could be rather easy to determine based on the enclosing patterns.In any case, it will be a matter of style whether
_
is an acceptably readable type on any given object pattern, and individual organizations and developers will choose a style for this, just like they are choosing a style for many other things.The inferred type of an object pattern may include actual type arguments:
In the object pattern
Myclass(x: var y)
, the type argumentint
is inferred. We can see that it is actuallyint
becausey.isEven
is accepted with no errors, and also becausey.isEven
is an error when we haveMyClass<num>(x: var y)
. We can also see that the inferred type isn'tdynamic
, becausey.unknownMember
is an error.This should all work the same when we use the proposed feature:
The text was updated successfully, but these errors were encountered: