-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Implementing switch expressions: everything in flutter/lib/src/
#143634
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
Implementing switch expressions: everything in flutter/lib/src/
#143634
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| AlignmentDirectional get _drawerInnerAlignment { | ||
| switch (widget.alignment) { | ||
| case DrawerAlignment.start: | ||
| return AlignmentDirectional.centerEnd; | ||
| case DrawerAlignment.end: | ||
| return AlignmentDirectional.centerStart; | ||
| } | ||
| return switch (widget.alignment) { | ||
| DrawerAlignment.start => AlignmentDirectional.centerEnd, | ||
| DrawerAlignment.end => AlignmentDirectional.centerStart, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically, if you want things even shorter, I think you could even do:
AlignmentDirectional get _drawerInnerAlignment => switch(...)
Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like using that format for my own projects, but I've been using return switch(...) here to follow the Flutter Style Guide:
only use
=>when everything, including the function declaration, fits on a single line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh! That makes sense. Probably the rule is older than switch :P but still valid. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One rule down in the style guide there is also this one: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use--for-inline-callbacks-that-just-return-literals-or-switch-expressions
So, changing this to what Bernardo suggested would be within the rules of the style guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad that rule includes switch expressions!
I don't think it applies to class getters and methods, but I'll update focus_traversal.dart right now 🙂
| case CupertinoDatePickerMode.monthYear: | ||
| return _CupertinoDatePickerMonthYearState(dateOrder: dateOrder); | ||
| } | ||
| // ignore: no_logic_in_create_state, https://github.com/flutter/flutter/issues/70499 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ignore made more sense at its old location. Can it be moved back? Or at the very least above the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it needs to be placed right above the switch expression in order to suppress the warning.
But I was able to reposition the other comments; hopefully it's easier to follow now.
| final TextDirection? textDirection = configuration.textDirection; | ||
| assert(textDirection != null); | ||
| final double start; | ||
| final double shadowDirection; // -1 for ltr, 1 for rtl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was lost in the refactoring, but it is kinda useful. Can you preserve it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
| AxisDirection.down => Offset(0, position.pixels), | ||
| AxisDirection.up => Offset(0, -position.pixels), | ||
| AxisDirection.left => Offset(-position.pixels, 0), | ||
| AxisDirection.right => Offset( position.pixels, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit funky to me, consider instead:
| AxisDirection.right => Offset( position.pixels, 0), | |
| AxisDirection.right => Offset(position.pixels, 0), |
Or:
| AxisDirection.right => Offset( position.pixels, 0), | |
| AxisDirection.right => Offset(position.pixels, 0), |
| AxisDirection.down => Offset(0, scrollableState.position.pixels), | ||
| AxisDirection.up => Offset(0, -scrollableState.position.pixels), | ||
| AxisDirection.left => Offset(-scrollableState.position.pixels, 0), | ||
| AxisDirection.right => Offset( scrollableState.position.pixels, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here as above: https://github.com/flutter/flutter/pull/143634/files#r1496378734
loic-sharma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome improvements, thanks for the contribution!
Manual roll Flutter from 5129806 to efee280 (47 revisions) Manual roll requested by dit@google.com flutter/flutter@5129806...efee280 2024-02-22 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf5c003085fd to 7eeb697687d5 (16 revisions) (flutter/flutter#143911) 2024-02-22 katelovett@google.com Update PR template for dart fix (flutter/flutter#143879) 2024-02-22 katelovett@google.com Re-use methods to calculate leading and trailing garbage in RenderSliverMultiBoxAdaptor (flutter/flutter#143884) 2024-02-21 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Make impeller goldens test blocking. (#143864)" (flutter/flutter#143896) 2024-02-21 jonahwilliams@google.com [Impeller] Make impeller goldens test blocking. (flutter/flutter#143864) 2024-02-21 jonahwilliams@google.com Disable color filter sepia test for Impeller. (flutter/flutter#143861) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 52ffcaadea41 to bf5c003085fd (12 revisions) (flutter/flutter#143875) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4128895d79a1 to 52ffcaadea41 (1 revision) (flutter/flutter#143862) 2024-02-21 katelovett@google.com Deprecate redundant itemExtent in RenderSliverFixedExtentBoxAdaptor methods (flutter/flutter#143412) 2024-02-21 reidbaker@google.com Add aab as alias for appbundle (flutter/flutter#143855) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from e16f43eeaaa4 to 4128895d79a1 (1 revision) (flutter/flutter#143856) 2024-02-21 engine-flutter-autoroll@skia.org Roll Packages from 8bba41b to 48048f6 (2 revisions) (flutter/flutter#143853) 2024-02-21 tessertaha@gmail.com Update `hourMinuteTextStyle` defaults for Material 3 Time Picker (flutter/flutter#143749) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 93063f61943a to e16f43eeaaa4 (2 revisions) (flutter/flutter#143827) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from ed49634486e9 to 93063f61943a (1 revision) (flutter/flutter#143826) 2024-02-21 tessertaha@gmail.com `CalendarDatePicker` doesn't announce selected date on desktop (flutter/flutter#143583) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9100d326475a to ed49634486e9 (2 revisions) (flutter/flutter#143824) 2024-02-21 tessertaha@gmail.com Add `timeSelectorSeparatorColor` and `timeSelectorSeparatorTextStyle` for Material 3 Time Picker (flutter/flutter#143739) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from efc69946cb1e to 9100d326475a (2 revisions) (flutter/flutter#143820) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 700250436e3f to efc69946cb1e (2 revisions) (flutter/flutter#143816) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3557277c575c to 700250436e3f (1 revision) (flutter/flutter#143814) 2024-02-21 jonahwilliams@google.com more fixes to unstable impeller goldens. (flutter/flutter#143811) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from cb12a8cc97a1 to 3557277c575c (2 revisions) (flutter/flutter#143807) 2024-02-21 kevmoo@users.noreply.github.com [flutter_tools] enable wasm compile on beta channel (flutter/flutter#143779) 2024-02-21 simonfv@gmail.com Fix initialization of time in repeat on AnimationController (flutter/flutter#142887) 2024-02-21 jonahwilliams@google.com Disable debug banner to stabilize impeller goldens. (flutter/flutter#143794) 2024-02-21 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (#143281)" (flutter/flutter#143801) 2024-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add UI Benchmarks (#143542)" (flutter/flutter#143798) 2024-02-20 31859944+LongCatIsLooong@users.noreply.github.com Avoid applying partial dartfixes on CI (flutter/flutter#143551) 2024-02-20 bernaferrari2@gmail.com Add UI Benchmarks (flutter/flutter#143542) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ae2c10e8071 to cb12a8cc97a1 (1 revision) (flutter/flutter#143791) 2024-02-20 nate.w5687@gmail.com Implement `_suspendedNode` fix (flutter/flutter#143556) 2024-02-20 xubaolin@oppo.com Change `ItemExtentBuilder`'s return value nullable (flutter/flutter#142428) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 27828054f07a to 1ae2c10e8071 (6 revisions) (flutter/flutter#143783) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from e16a260265ad to 27828054f07a (1 revision) (flutter/flutter#143769) 2024-02-20 jonahwilliams@google.com [gold] Always provide host ABI to gold config (flutter/flutter#143621) 2024-02-20 andrewrkolos@gmail.com instead of exiting the tool, print a warning when using --flavor with an incompatible device (flutter/flutter#143735) 2024-02-20 nate.w5687@gmail.com Implementing `switch` expressions: everything in `flutter/lib/src/` (flutter/flutter#143634) 2024-02-20 31859944+LongCatIsLooong@users.noreply.github.com Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (flutter/flutter#143281) 2024-02-20 34871572+gmackall@users.noreply.github.com Delete local.properties that shouldn't have been pushed (flutter/flutter#143774) 2024-02-20 polinach@google.com Clean leaks. (flutter/flutter#142818) 2024-02-20 36861262+QuncCccccc@users.noreply.github.com Introduce tone-based surfaces and accent color add-ons - Part 2 (flutter/flutter#138521) 2024-02-20 greg@zulip.com Explain when and why to use CrossAxisAlignment.baseline (flutter/flutter#143632) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from a41da3701923 to e16a260265ad (2 revisions) (flutter/flutter#143763) ...
This pull request fixes #143803 by taking advantage of Dart's null-aware operators. And unlike `switch` expressions ([9 PRs](#143634) and counting), the Flutter codebase is already fantastic when it comes to null-aware coding. After refactoring the entire repo, all the changes involving `?.` and `??` can fit into a single pull request.
) Manual roll Flutter from 5129806 to efee280 (47 revisions) Manual roll requested by dit@google.com flutter/flutter@5129806...efee280 2024-02-22 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf5c003085fd to 7eeb697687d5 (16 revisions) (flutter/flutter#143911) 2024-02-22 katelovett@google.com Update PR template for dart fix (flutter/flutter#143879) 2024-02-22 katelovett@google.com Re-use methods to calculate leading and trailing garbage in RenderSliverMultiBoxAdaptor (flutter/flutter#143884) 2024-02-21 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Make impeller goldens test blocking. (#143864)" (flutter/flutter#143896) 2024-02-21 jonahwilliams@google.com [Impeller] Make impeller goldens test blocking. (flutter/flutter#143864) 2024-02-21 jonahwilliams@google.com Disable color filter sepia test for Impeller. (flutter/flutter#143861) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 52ffcaadea41 to bf5c003085fd (12 revisions) (flutter/flutter#143875) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4128895d79a1 to 52ffcaadea41 (1 revision) (flutter/flutter#143862) 2024-02-21 katelovett@google.com Deprecate redundant itemExtent in RenderSliverFixedExtentBoxAdaptor methods (flutter/flutter#143412) 2024-02-21 reidbaker@google.com Add aab as alias for appbundle (flutter/flutter#143855) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from e16f43eeaaa4 to 4128895d79a1 (1 revision) (flutter/flutter#143856) 2024-02-21 engine-flutter-autoroll@skia.org Roll Packages from 8bba41b to 48048f6 (2 revisions) (flutter/flutter#143853) 2024-02-21 tessertaha@gmail.com Update `hourMinuteTextStyle` defaults for Material 3 Time Picker (flutter/flutter#143749) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 93063f61943a to e16f43eeaaa4 (2 revisions) (flutter/flutter#143827) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from ed49634486e9 to 93063f61943a (1 revision) (flutter/flutter#143826) 2024-02-21 tessertaha@gmail.com `CalendarDatePicker` doesn't announce selected date on desktop (flutter/flutter#143583) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9100d326475a to ed49634486e9 (2 revisions) (flutter/flutter#143824) 2024-02-21 tessertaha@gmail.com Add `timeSelectorSeparatorColor` and `timeSelectorSeparatorTextStyle` for Material 3 Time Picker (flutter/flutter#143739) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from efc69946cb1e to 9100d326475a (2 revisions) (flutter/flutter#143820) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 700250436e3f to efc69946cb1e (2 revisions) (flutter/flutter#143816) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3557277c575c to 700250436e3f (1 revision) (flutter/flutter#143814) 2024-02-21 jonahwilliams@google.com more fixes to unstable impeller goldens. (flutter/flutter#143811) 2024-02-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from cb12a8cc97a1 to 3557277c575c (2 revisions) (flutter/flutter#143807) 2024-02-21 kevmoo@users.noreply.github.com [flutter_tools] enable wasm compile on beta channel (flutter/flutter#143779) 2024-02-21 simonfv@gmail.com Fix initialization of time in repeat on AnimationController (flutter/flutter#142887) 2024-02-21 jonahwilliams@google.com Disable debug banner to stabilize impeller goldens. (flutter/flutter#143794) 2024-02-21 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (#143281)" (flutter/flutter#143801) 2024-02-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add UI Benchmarks (#143542)" (flutter/flutter#143798) 2024-02-20 31859944+LongCatIsLooong@users.noreply.github.com Avoid applying partial dartfixes on CI (flutter/flutter#143551) 2024-02-20 bernaferrari2@gmail.com Add UI Benchmarks (flutter/flutter#143542) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ae2c10e8071 to cb12a8cc97a1 (1 revision) (flutter/flutter#143791) 2024-02-20 nate.w5687@gmail.com Implement `_suspendedNode` fix (flutter/flutter#143556) 2024-02-20 xubaolin@oppo.com Change `ItemExtentBuilder`'s return value nullable (flutter/flutter#142428) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 27828054f07a to 1ae2c10e8071 (6 revisions) (flutter/flutter#143783) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from e16a260265ad to 27828054f07a (1 revision) (flutter/flutter#143769) 2024-02-20 jonahwilliams@google.com [gold] Always provide host ABI to gold config (flutter/flutter#143621) 2024-02-20 andrewrkolos@gmail.com instead of exiting the tool, print a warning when using --flavor with an incompatible device (flutter/flutter#143735) 2024-02-20 nate.w5687@gmail.com Implementing `switch` expressions: everything in `flutter/lib/src/` (flutter/flutter#143634) 2024-02-20 31859944+LongCatIsLooong@users.noreply.github.com Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (flutter/flutter#143281) 2024-02-20 34871572+gmackall@users.noreply.github.com Delete local.properties that shouldn't have been pushed (flutter/flutter#143774) 2024-02-20 polinach@google.com Clean leaks. (flutter/flutter#142818) 2024-02-20 36861262+QuncCccccc@users.noreply.github.com Introduce tone-based surfaces and accent color add-ons - Part 2 (flutter/flutter#138521) 2024-02-20 greg@zulip.com Explain when and why to use CrossAxisAlignment.baseline (flutter/flutter#143632) 2024-02-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from a41da3701923 to e16a260265ad (2 revisions) (flutter/flutter#143763) ...

This PR is the 9ᵗʰ step in the journey to solve issue #136139 and make the entire Flutter repo more readable.
(previous pull requests: #139048, #139882, #141591, #142279, #142634, #142793, #143293, #143496)
I did a pass through all of
packages/flutter/lib/src/and found an abundance ofswitchstatements to improve. Whereas #143496 focused on in-depth refactoring, this PR is full of simple, straightforward changes. (I ended up making some more complicated changes inrendering/and will file those separately after this PR is done.)Pre-launch Checklist
///).