Implementing `switch` expressions: everything in `flutter/lib/src/` #143634

nate-thegrate · 2024-02-17T07:28:04Z

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 of switch statements 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 in rendering/ and will file those separately after this PR is done.)

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

flutter-dashboard · 2024-02-18T23:47:03Z

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.

bernaferrari · 2024-02-19T18:46:08Z

packages/flutter/lib/src/material/drawer.dart

  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,
+    };
  }


Theoretically, if you want things even shorter, I think you could even do:

AlignmentDirectional get _drawerInnerAlignment => switch(...)

Right?

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.

oh! That makes sense. Probably the rule is older than switch :P but still valid. Thanks

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.

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 🙂

goderbauer · 2024-02-20T18:27:54Z

packages/flutter/lib/src/cupertino/date_picker.dart

-      case CupertinoDatePickerMode.monthYear:
-        return _CupertinoDatePickerMonthYearState(dateOrder: dateOrder);
-    }
+    // ignore: no_logic_in_create_state, https://github.com/flutter/flutter/issues/70499


This ignore made more sense at its old location. Can it be moved back? Or at the very least above the comment?

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.

goderbauer · 2024-02-20T18:29:32Z

packages/flutter/lib/src/cupertino/route.dart

    final TextDirection? textDirection = configuration.textDirection;
    assert(textDirection != null);
-    final double start;
-    final double shadowDirection; // -1 for ltr, 1 for rtl.


This comment was lost in the refactoring, but it is kinda useful. Can you preserve it?

This is one of those borderline cases as far as whether a comment is useful: in my opinion, the switch expression makes it easy to tell that the value of shadowDirection is 1 for rtl and -1 for ltr.

packages/flutter/lib/src/material/ink_well.dart

packages/flutter/lib/src/services/raw_keyboard_fuchsia.dart

Co-authored-by: Loïc Sharma <[email protected]>

loic-sharma · 2024-02-20T19:29:14Z

packages/flutter/lib/src/widgets/scrollable.dart

+      AxisDirection.down  => Offset(0,  position.pixels),
+      AxisDirection.up    => Offset(0, -position.pixels),
+      AxisDirection.left  => Offset(-position.pixels, 0),
+      AxisDirection.right => Offset( position.pixels, 0),


This looks a bit funky to me, consider instead:

Suggested change

AxisDirection.right => Offset( position.pixels, 0),

AxisDirection.right => Offset(position.pixels, 0),

Or:

Suggested change

AxisDirection.right => Offset( position.pixels, 0),

AxisDirection.right => Offset(position.pixels, 0),

loic-sharma · 2024-02-20T19:29:41Z

packages/flutter/lib/src/widgets/scrollable.dart

+    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),


Same comment here as above: https://github.com/flutter/flutter/pull/143634/files#r1496378734

loic-sharma

Awesome improvements, thanks for the contribution!

…/src/` (flutter/flutter#143634)

Manual roll Flutter from 5129806 to efee280 (47 revisions) Manual roll requested by [email protected] flutter/flutter@5129806...efee280 2024-02-22 [email protected] Roll Flutter Engine from bf5c003085fd to 7eeb697687d5 (16 revisions) (flutter/flutter#143911) 2024-02-22 [email protected] Update PR template for dart fix (flutter/flutter#143879) 2024-02-22 [email protected] 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 [email protected] [Impeller] Make impeller goldens test blocking. (flutter/flutter#143864) 2024-02-21 [email protected] Disable color filter sepia test for Impeller. (flutter/flutter#143861) 2024-02-21 [email protected] Roll Flutter Engine from 52ffcaadea41 to bf5c003085fd (12 revisions) (flutter/flutter#143875) 2024-02-21 [email protected] Roll Flutter Engine from 4128895d79a1 to 52ffcaadea41 (1 revision) (flutter/flutter#143862) 2024-02-21 [email protected] Deprecate redundant itemExtent in RenderSliverFixedExtentBoxAdaptor methods (flutter/flutter#143412) 2024-02-21 [email protected] Add aab as alias for appbundle (flutter/flutter#143855) 2024-02-21 [email protected] Roll Flutter Engine from e16f43eeaaa4 to 4128895d79a1 (1 revision) (flutter/flutter#143856) 2024-02-21 [email protected] Roll Packages from 8bba41b to 48048f6 (2 revisions) (flutter/flutter#143853) 2024-02-21 [email protected] Update `hourMinuteTextStyle` defaults for Material 3 Time Picker (flutter/flutter#143749) 2024-02-21 [email protected] Roll Flutter Engine from 93063f61943a to e16f43eeaaa4 (2 revisions) (flutter/flutter#143827) 2024-02-21 [email protected] Roll Flutter Engine from ed49634486e9 to 93063f61943a (1 revision) (flutter/flutter#143826) 2024-02-21 [email protected] `CalendarDatePicker` doesn't announce selected date on desktop (flutter/flutter#143583) 2024-02-21 [email protected] Roll Flutter Engine from 9100d326475a to ed49634486e9 (2 revisions) (flutter/flutter#143824) 2024-02-21 [email protected] Add `timeSelectorSeparatorColor` and `timeSelectorSeparatorTextStyle` for Material 3 Time Picker (flutter/flutter#143739) 2024-02-21 [email protected] Roll Flutter Engine from efc69946cb1e to 9100d326475a (2 revisions) (flutter/flutter#143820) 2024-02-21 [email protected] Roll Flutter Engine from 700250436e3f to efc69946cb1e (2 revisions) (flutter/flutter#143816) 2024-02-21 [email protected] Roll Flutter Engine from 3557277c575c to 700250436e3f (1 revision) (flutter/flutter#143814) 2024-02-21 [email protected] more fixes to unstable impeller goldens. (flutter/flutter#143811) 2024-02-21 [email protected] Roll Flutter Engine from cb12a8cc97a1 to 3557277c575c (2 revisions) (flutter/flutter#143807) 2024-02-21 [email protected] [flutter_tools] enable wasm compile on beta channel (flutter/flutter#143779) 2024-02-21 [email protected] Fix initialization of time in repeat on AnimationController (flutter/flutter#142887) 2024-02-21 [email protected] 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 [email protected] Avoid applying partial dartfixes on CI (flutter/flutter#143551) 2024-02-20 [email protected] Add UI Benchmarks (flutter/flutter#143542) 2024-02-20 [email protected] Roll Flutter Engine from 1ae2c10e8071 to cb12a8cc97a1 (1 revision) (flutter/flutter#143791) 2024-02-20 [email protected] Implement `_suspendedNode` fix (flutter/flutter#143556) 2024-02-20 [email protected] Change `ItemExtentBuilder`'s return value nullable (flutter/flutter#142428) 2024-02-20 [email protected] Roll Flutter Engine from 27828054f07a to 1ae2c10e8071 (6 revisions) (flutter/flutter#143783) 2024-02-20 [email protected] Roll Flutter Engine from e16a260265ad to 27828054f07a (1 revision) (flutter/flutter#143769) 2024-02-20 [email protected] [gold] Always provide host ABI to gold config (flutter/flutter#143621) 2024-02-20 [email protected] instead of exiting the tool, print a warning when using --flavor with an incompatible device (flutter/flutter#143735) 2024-02-20 [email protected] Implementing `switch` expressions: everything in `flutter/lib/src/` (flutter/flutter#143634) 2024-02-20 [email protected] Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (flutter/flutter#143281) 2024-02-20 [email protected] Delete local.properties that shouldn't have been pushed (flutter/flutter#143774) 2024-02-20 [email protected] Clean leaks. (flutter/flutter#142818) 2024-02-20 [email protected] Introduce tone-based surfaces and accent color add-ons - Part 2 (flutter/flutter#138521) 2024-02-20 [email protected] Explain when and why to use CrossAxisAlignment.baseline (flutter/flutter#143632) 2024-02-20 [email protected] 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 [email protected] flutter/flutter@5129806...efee280 2024-02-22 [email protected] Roll Flutter Engine from bf5c003085fd to 7eeb697687d5 (16 revisions) (flutter/flutter#143911) 2024-02-22 [email protected] Update PR template for dart fix (flutter/flutter#143879) 2024-02-22 [email protected] 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 [email protected] [Impeller] Make impeller goldens test blocking. (flutter/flutter#143864) 2024-02-21 [email protected] Disable color filter sepia test for Impeller. (flutter/flutter#143861) 2024-02-21 [email protected] Roll Flutter Engine from 52ffcaadea41 to bf5c003085fd (12 revisions) (flutter/flutter#143875) 2024-02-21 [email protected] Roll Flutter Engine from 4128895d79a1 to 52ffcaadea41 (1 revision) (flutter/flutter#143862) 2024-02-21 [email protected] Deprecate redundant itemExtent in RenderSliverFixedExtentBoxAdaptor methods (flutter/flutter#143412) 2024-02-21 [email protected] Add aab as alias for appbundle (flutter/flutter#143855) 2024-02-21 [email protected] Roll Flutter Engine from e16f43eeaaa4 to 4128895d79a1 (1 revision) (flutter/flutter#143856) 2024-02-21 [email protected] Roll Packages from 8bba41b to 48048f6 (2 revisions) (flutter/flutter#143853) 2024-02-21 [email protected] Update `hourMinuteTextStyle` defaults for Material 3 Time Picker (flutter/flutter#143749) 2024-02-21 [email protected] Roll Flutter Engine from 93063f61943a to e16f43eeaaa4 (2 revisions) (flutter/flutter#143827) 2024-02-21 [email protected] Roll Flutter Engine from ed49634486e9 to 93063f61943a (1 revision) (flutter/flutter#143826) 2024-02-21 [email protected] `CalendarDatePicker` doesn't announce selected date on desktop (flutter/flutter#143583) 2024-02-21 [email protected] Roll Flutter Engine from 9100d326475a to ed49634486e9 (2 revisions) (flutter/flutter#143824) 2024-02-21 [email protected] Add `timeSelectorSeparatorColor` and `timeSelectorSeparatorTextStyle` for Material 3 Time Picker (flutter/flutter#143739) 2024-02-21 [email protected] Roll Flutter Engine from efc69946cb1e to 9100d326475a (2 revisions) (flutter/flutter#143820) 2024-02-21 [email protected] Roll Flutter Engine from 700250436e3f to efc69946cb1e (2 revisions) (flutter/flutter#143816) 2024-02-21 [email protected] Roll Flutter Engine from 3557277c575c to 700250436e3f (1 revision) (flutter/flutter#143814) 2024-02-21 [email protected] more fixes to unstable impeller goldens. (flutter/flutter#143811) 2024-02-21 [email protected] Roll Flutter Engine from cb12a8cc97a1 to 3557277c575c (2 revisions) (flutter/flutter#143807) 2024-02-21 [email protected] [flutter_tools] enable wasm compile on beta channel (flutter/flutter#143779) 2024-02-21 [email protected] Fix initialization of time in repeat on AnimationController (flutter/flutter#142887) 2024-02-21 [email protected] 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 [email protected] Avoid applying partial dartfixes on CI (flutter/flutter#143551) 2024-02-20 [email protected] Add UI Benchmarks (flutter/flutter#143542) 2024-02-20 [email protected] Roll Flutter Engine from 1ae2c10e8071 to cb12a8cc97a1 (1 revision) (flutter/flutter#143791) 2024-02-20 [email protected] Implement `_suspendedNode` fix (flutter/flutter#143556) 2024-02-20 [email protected] Change `ItemExtentBuilder`'s return value nullable (flutter/flutter#142428) 2024-02-20 [email protected] Roll Flutter Engine from 27828054f07a to 1ae2c10e8071 (6 revisions) (flutter/flutter#143783) 2024-02-20 [email protected] Roll Flutter Engine from e16a260265ad to 27828054f07a (1 revision) (flutter/flutter#143769) 2024-02-20 [email protected] [gold] Always provide host ABI to gold config (flutter/flutter#143621) 2024-02-20 [email protected] instead of exiting the tool, print a warning when using --flavor with an incompatible device (flutter/flutter#143735) 2024-02-20 [email protected] Implementing `switch` expressions: everything in `flutter/lib/src/` (flutter/flutter#143634) 2024-02-20 [email protected] Changing `TextPainter.getOffsetForCaret` implementation to remove the logarithmic search (flutter/flutter#143281) 2024-02-20 [email protected] Delete local.properties that shouldn't have been pushed (flutter/flutter#143774) 2024-02-20 [email protected] Clean leaks. (flutter/flutter#142818) 2024-02-20 [email protected] Introduce tone-based surfaces and accent color add-ons - Part 2 (flutter/flutter#138521) 2024-02-20 [email protected] Explain when and why to use CrossAxisAlignment.baseline (flutter/flutter#143632) 2024-02-20 [email protected] Roll Flutter Engine from a41da3701923 to e16a260265ad (2 revisions) (flutter/flutter#143763) ...

This pull request is step 12 in the journey to make this entire repository more readable. (previous PRs: #139048, #139882, #141591, #142279, #142634, #142793, #143293, #143496, #143634, #143812, #144580) We're getting close to the end! ð��

…/src/` (flutter/flutter#143634)

nate-thegrate added 2 commits February 16, 2024 23:27

refactor performLayout()

0b5eb1e

so many switch expressions

9d84cb6

nate-thegrate added 10 commits February 17, 2024 00:33

fix variable syntax

c61c880

was missing a comma 😑

5809f66

remove unused variable

b941524

move the // ignore comment to a better spot

1d11356

revert some scrolling changes

75123f4

re-implement some scroll changes

d8e5a7d

re-implement scrollable.dart

b3698d5

re-implement scrollable_helpers.dart

c6a0003

re-implement *some* SingleChildScrollView changes

9ea128a

fix SingleChildScrollView problem

fbccf06

nate-thegrate marked this pull request as ready for review February 17, 2024 20:49

nate-thegrate added 2 commits February 18, 2024 16:08

DRY switch statements

941a800

revert changes to lib/src/rendering/

871b053

revert non-switch-expression changes

e91c5a8

bernaferrari reviewed Feb 19, 2024

View reviewed changes

=> for inline methods

ea2e2e4

goderbauer reviewed Feb 20, 2024

View reviewed changes

relocate comments

beef7ab

loic-sharma reviewed Feb 20, 2024

View reviewed changes

packages/flutter/lib/src/services/raw_keyboard_fuchsia.dart Outdated Show resolved Hide resolved

Apply suggestions from code review

a1bc67b

Co-authored-by: Loïc Sharma <[email protected]>

loic-sharma reviewed Feb 20, 2024

View reviewed changes

loic-sharma approved these changes Feb 20, 2024

View reviewed changes

make switch expressions a bit less funky

ebd16a1

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 20, 2024

auto-submit bot merged commit 7a4c246 into flutter:master Feb 20, 2024

nate-thegrate deleted the switch-expressions-9 branch February 20, 2024 21:07

nate-thegrate mentioned this pull request Feb 21, 2024

Implementing null-aware operators throughout the repository #143804

Merged

8 tasks

engine-flutter-autoroll mentioned this pull request Feb 21, 2024

Roll Flutter from 5129806e6c63 to 3403ca413f6e (35 revisions) flutter/packages#6170

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 21, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

c06f7c0

…/src/` (flutter/flutter#143634)

engine-flutter-autoroll mentioned this pull request Feb 21, 2024

Roll Flutter from 5129806e6c63 to 4aecbf916b87 (36 revisions) flutter/packages#6171

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 21, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

5977ba3

…/src/` (flutter/flutter#143634)

engine-flutter-autoroll mentioned this pull request Feb 21, 2024

Roll Flutter from 5129806e6c63 to bfeaf5a7f2c6 (37 revisions) flutter/packages#6172

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 21, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

79aae95

…/src/` (flutter/flutter#143634)

engine-flutter-autoroll mentioned this pull request Feb 21, 2024

Roll Flutter from 5129806e6c63 to b09a015ff567 (38 revisions) flutter/packages#6174

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 21, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

7b452f2

…/src/` (flutter/flutter#143634)

engine-flutter-autoroll mentioned this pull request Feb 22, 2024

Manual roll Flutter from 5129806e6c63 to efee280cfa93 (47 revisions) flutter/packages#6180

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

b500549

…/src/` (flutter/flutter#143634)

HansMuller mentioned this pull request Mar 7, 2024

Flutter Framework Team: Notable Commits #121415

Closed

nate-thegrate mentioned this pull request Mar 23, 2024

Implementing switch expressions in flutter_tools/ #145632

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024

7a4c246 Implementing switch expressions: everything in `flutter/lib…

febdff7

…/src/` (flutter/flutter#143634)

nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Jul 12, 2024

nate-thegrate mentioned this pull request Oct 9, 2024

Style Guide updates #152525

Merged

	AxisDirection.right => Offset( position.pixels, 0),
	AxisDirection.right => Offset(position.pixels, 0),

Implementing switch expressions: everything in flutter/lib/src/ #143634

Implementing switch expressions: everything in flutter/lib/src/ #143634

Uh oh!

Conversation

nate-thegrate commented Feb 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Feb 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implementing `switch` expressions: everything in `flutter/lib/src/` #143634

Implementing `switch` expressions: everything in `flutter/lib/src/` #143634

nate-thegrate commented Feb 17, 2024 •

edited

Loading