Skip to content
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

[CP] [parser] Support nullable record types in patterns #52518

Closed
stereotype441 opened this issue May 25, 2023 · 1 comment
Closed

[CP] [parser] Support nullable record types in patterns #52518

stereotype441 opened this issue May 25, 2023 · 1 comment
Assignees
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable

Comments

@stereotype441
Copy link
Member

Commit(s) to merge

284296f

Target

Stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/305824

Issue Description

If a variable pattern or wildcard pattern has an explicit type which is a nullable record type, there is a parse error. For example, this code:

f(Object? x) {
  switch (x) {
    case ((int, int)? x, String y):
      break;
  }
}

is rejected with the error message:

.../test.dart:3:23: Error: Expected ')' before this.
    case ((int, int)? x, String y):
                      ^

What is the fix

The parser's disambiguation rules are modified so that if a pattern starts with (, and the matching ) is followed by ? identifier, an attempt is made to parse the pattern as a variable pattern. Previously, the disambiguation rules only attempted to parse a pattern starting with ( as a variable pattern if the matching ) was followed by an identifier (i.e. a variable pattern with a non-nullable record type).

Why cherry-pick

Without this fix, users cannot express a pattern match using a variable or wildcard pattern with a nullable record type. It is not clear from the error message why the code is being rejected.

If we don't cherry-pick this fix, the fix will be released as part of Dart 3.1, so there will be a danger of a package author releasing code that is intended to be compatible with Dart 3.0, but which fails to work due to its use of a variable pattern with an explicit nullable record type.

The fix is low risk, because it is confined to the disambiguation logic that fires specifically for patterns that begin with (. The only syntax that is newly recognized as a variable pattern is (...)? identifier. Any ambiguities that might arise from this interpretation are also problems for the syntax (...) identifier (e.g. (...) when or (...) as), and the resolution of these ambiguities is already taken care of and well tested.

Risk

low

Issue link(s)

#52439

Extra Info

No response

@stereotype441 stereotype441 added the cherry-pick-review Issue that need cherry pick triage to approve label May 25, 2023
@mit-mit
Copy link
Member

mit-mit commented May 25, 2023

SGTM

@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-approved Label for approved cherrypick request labels May 25, 2023
copybara-service bot pushed a commit that referenced this issue May 26, 2023
Not fixed for nullable record types in
https://dart-review.googlesource.com/c/sdk/+/280106.

Fixes: #52518

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/304642
Change-Id: I5861b9fa7a0a6b8dce1b8965f6f4ef58123133a6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305824
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
@itsjustkevin itsjustkevin added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable
Projects
None yet
Development

No branches or pull requests

6 participants