[patterns] Function pattern inconsistency between implementation and spec.

[modulovalue]

Hello, again 😃

Consider:

void main() {
  if (() {} case Function()) {
    print("foo");
  }
}

On DartPad, it prints foo and terminates successfully. However, it looks like the spec does not intend for that to be possible. Function is not a typeIdentifier so objectPatterns do not describe Function() and there's no other pattern that explicitly introduces Function patterns.

 === Analyzer ===
Scan errors: 0
Parse errors: 0
...
        ┃  ┣━ <CaseClauseImpl> [26-41]
        ┃  ┃  ┣━ 'case' [26-30]
        ┃  ┃  ┗━ <GuardedPatternImpl> [31-41]
        ┃  ┃    ┗━ <ObjectPatternImpl> [31-41]
        ┃  ┃      ┣━ <NamedTypeImpl> [31-39]
        ┃  ┃      ┃  ┗━ 'Function' [31-39]
        ┃  ┃      ┣━ '(' [39-40]
        ┃  ┃      ┗━ ')' [40-41]
...
--------------------------------------------------------------------------------
 === ANTLR ===
line 2:17 missing ')' at 'Function'
Errors of type 1: [[@15,41:41=')',<7>,2:27], [@15,41:41=')',<7>,2:27], [@15,41:41=')',<7>,2:27], [@15,41:41=')',<7>,2:27]]
Errors of type 2: [<missing ')'>, Function, (, ), )]
line 2:17 missing ')' at 'Function'
<startSymbol>
┗━ <libraryDefinition>
  ┣━ <metadata>
...

[eernstg]

Good catch!

The implementations (the analyzer as well as the common front end) do already support a pattern of a form like Function() or Function(: var toString), and they treat it as an <objectPattern>. This treatment is also consistent with the treatment of Function as the name of a class, and the treatment of object patterns otherwise.

I created a PR that introduces support for this construct into the grammar: dart-lang/language#3496, and a spec_parser update: https://dart-review.googlesource.com/340181.

[lrhn]

Not sure we should allow it, actually.

We generally say that when Function is followed by (, it's a function type, not a reference to the name Function exported by dart:core.

Allowing Function() here to refer to that Function name would be inconsistent.
And it's not like there are any useful getters on Function where it would be necessary to use an object pattern.
To match any function, you can do Function _ instead.

I'd rather that we made it an error to write case Function():, for great consistency.

[eernstg]

We generally say that when Function is followed by ( ...

I don't think we've specified a rule of that nature. We did discuss the need to disambiguate, but I think we concluded that it wasn't needed, and I can't immediately find anything like a specified disambiguation mechanism. Also, Dart.g doesn't have any special treatment of FUNCTION followed by (. Are you aware of a specified rule about this?

Next, in an object pattern like C(...), C is a reference to a declaration that introduces a type (could be a class, mixin, typedef, extension type, enum, etc.). We do treat Function as the name of a class when it is used an a context like this: Function foo() => print;, and this does make MyClass(:var toString) similar to Function(:var toString): In both cases, we're denoting a type and then specifying a sequence of field patterns. Finally, in the situation where a pattern is expected, the grammar does not allow a function type to occur.

So we may be able to say that Function followed by ( implies function type, but we can also list several reasons why there is no such rule, or it isn't applicable right here.

[modulovalue]

I'd prefer Dart not to support constructs that essentially add no value. As Lasse said, such a Function objectPattern isn't useful, and I agree.

The ability to extend Function types was deprecated for a long time and removed recently, and I think it would make sense not to support Function objectPatterns, too.

In system libraries, the implementation already ignores the fact that classes can't be called Function. The fact that objectPatterns support Functions appears to be an accident.

[eernstg]

I'd prefer Dart not to support constructs that essentially add no value.

I can follow that, but I'm not convinced that it is a good idea to create exceptions in order to avoid having support for certain things, just because they appear to add little value.

We allow Function to be used as the name of a class in a number of situations, including Function foo() => print; and List<Function>, but we will now insist that we cannot do the same thing in an object pattern, in spite of the fact that it isn't even difficult to parse? And we will even pay for it in terms of a breaking change process? ;-)

Note that we can always emulate Function(...) as an object pattern by declaring a different name for it:

typedef MyFunction = Function;

void main() {
  Object? x = print;
  switch (x) {
    case MyFunction(): x('Hello again!');
    case Enum(): print(x.index);
    case var other: ...;
  }
}

[modulovalue]

but I'm not convinced that it is a good idea to create exceptions in order to avoid having support for certain things

That exception already exists for void, so we would not create a new exception, but, I guess, make the exception bucket bigger:

typedef MyVoid = void;

void main() {
  switch (0) {
    // Not supported
    // case void(): print("print");
    // Supported
    case MyVoid(): print("foo");
  }
}

But I'd also argue that it's not really an exception. A Function is not a valid type identifier, so it's not a type name, but a type.

Do we already have type patterns? Yes. dynamic is not a type identifier, but a type:

void main() {
  switch (0) {
    case dynamic(: final isEven): print("${isEven}");
  }
}

So either way, I think we can't remove or not introduce any exceptions unless we remove support for dynamic patterns & Function patterns, or keep Function patterns and add void patterns. That is, either support all identifier-based types as patterns or only typeIdentifier-based patterns.(*)

in spite of the fact that it isn't even difficult to parse?

That seems to be the case, but future language changes could introduce syntax that would make it difficult to parse in the future.

* dynamic is a typeidentifier today, but identifiers and types can be simplified significantly by making dynamic a type and not a typeIdentifier. I plan to open an issue for that in the future

[eernstg]

That exception already exists for void

It is true that void gets a very special treatment (it's a reserved word, no other type is a reserved word), and it hasn't been fully "normalized" in the grammar. However, that's a consequence of the history, it isn't an exception that the language team wanted and subsequently introduced, it's an oddity that had already existed for several years when, for instance, I joined Google.

I think we can't remove or not introduce any exceptions unless we remove support for dynamic patterns & Function patterns, or keep Function patterns and add void patterns.

Well, I think a pattern like void(...) is going to be difficult to use in a meaningful way: void is a top type, which means that testing whether something is void (except that we can't even do that unless we use a type alias) is true for every object whatsoever; hence, void(...) isn't capable of selecting any set of objects, it just accepts all objects. Next, any void pattern with field patterns (like void(hashCode: int h)) should arguably be a compile-time error, because it is doing the same thing as int h = s.hashCode; where s is the scrutinee object, and s has the static type void, and that is a compile-time error. In other words, a void pattern can basically never be anything other than a no-op (unless we introduce a bunch of other exceptions to the treatment of void typed objects).

In contrast, Function() will match all function objects, and that could actually be useful now and then. Also, it seems natural to me that you should be able to do things like this:

SomeType foo(Object arg) => switch (arg) {
  case Function(): ...; // Deal with functions.
  case String(): ...; // Deal with strings.
  ... // Deal with other things.
};

This works today. I'm just saying that I don't think it's going to be an improvement for anyone if we turn that usage into a compile-time error.

Granted, you would only encounter this kind of code in a "rather dynamically typed" library, because we won't know anything about which kind of function we're working with unless we proceed to test arg is void Function(int) or some other specific function type in the body of the Function() case. However, I don't think we should specifically aim for making "rather dynamically typed" code less convenient to write (and read).

For me, this points in the direction of preserving the support for Function(...) patterns, and not introducing void patterns. One might say that we'd get rid of an exception if we did add support for void patterns, but I'm not convinced that void patterns are useful anyway, whereas Function(...) patterns can actually make sense.

[modulovalue]

I agree that it makes sense to support enumerating all semantic-types (and not necessarily all syntactic-types) as patterns and that, in that sense, not supporting Function patterns would be a clear exception. I think that this kind of consistency is more valuable than the one I was going for.

I agree with keeping Function patterns and adjusting the spec to add support for them.

[eernstg]

@modulovalue, that sounds good!

@lrhn, do you still stand by this?:

I'd rather that we made it an error to write case Function():, for great consistency.

[lrhn]

I do.

Whenever you see Function() I would like it to mean the same thing.

It's the design rule of "similar syntax means similar things".

Function() should always be a function type, and should not be allowed where other function types are not allowed.

If we managed to actually disallow it in patterns, and not allow it by accident, we should hold onto that.

[eernstg]

OK, so A() should always be an instance creation and never an object pattern? ;-)