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.

Sign up for GitHub

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

Jump to bottom

[CP] A roll of dart_style which can format empty nullable record types ()? #52591

Closed
munificent opened this issue Jun 1, 2023 · 4 comments
Closed

[CP] A roll of dart_style which can format empty nullable record types ()? #52591

munificent opened this issue Jun 1, 2023 · 4 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). 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

@munificent
Copy link
Member

Commit(s) to merge

7c83e8a

Target

stable

Prepared changelist for beta/stable

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

Issue Description

The formatter has a bug where a nullable type record with no fields (i.e. ()?) discards the ?. The formatter has an internal self-check that it only changes whitespace which fails and the result is that the formatter exits with an error message and doesn't format the file at all.

What is the fix

The fix is a trivial one-line change that preserves the ? if there is one. There are no other changes in this dart_style roll.

Why cherry-pick

I don't expect nullable empty record types to be very common, but they are valid and meaningful, and without this fix, users are completely unable to format any part of a file that happens to contain ()?.

Risk

low

Issue link(s)

dart-lang/dart_style#1224

Extra Info

No response
@munificent munificent added the cherry-pick-review Issue that need cherry pick triage to approve label Jun 1, 2023
@athomas
Copy link
Member

athomas commented Jun 2, 2023

The current CP CL causes build breakage and can't be landed.

copybara-service bot pushed a commit that referenced this issue Jun 6, 2023
@munificent
[stable] Roll dart_style which fixes formatting ()?.
e5bae1b 
Fixes: #52591

Bug: dart-lang/dart_style#1224
Change-Id: I5d87c487e036d69cb5e3ed3289f3b066132a8dea
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/306122
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306911
Auto-Submit: Bob Nystrom <[email protected]>
Reviewed-by: William Hesse <[email protected]>
Commit-Queue: William Hesse <[email protected]>
@munificent
Copy link
Member Author

https://dart-review.googlesource.com/c/sdk/+/306911 is now merged.

@munificent munificent reopened this Jun 6, 2023
@munificent
Copy link
Member Author

Oops, I'm not supposed to be the one to close this. :)

@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. labels Jun 7, 2023
@mit-mit mit-mit added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Jun 7, 2023
@munificent
Copy link
Member Author

Thanks, @itsjustkevin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@whesse whesse

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). 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
Milestone
No milestone
Development

No branches or pull requests
6 participants
@munificent @vsmenon @whesse @itsjustkevin @mit-mit @athomas