Looking at ways we aren't actually following the style guide by Piinks · Pull Request #172296 · flutter/flutter

Piinks · 2025-07-17T16:21:07Z

Prompted by Gemini output that has been given our style guide: flutter/packages#9627 (comment)

Started looking at =>, but also looked to see where else we may be out of compliance with the style guide - or the style guide may not longer be correct. Feedback welcome. :)

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Piinks · 2025-07-17T16:21:52Z

-refactoring code much harder. (These are commonly used in Flutter's codebase today, but that is almost
-always a mistake. When you are editing a file that uses those features, aim to reduce the number of
-tests using them while you're there.)
+Avoid using test-global variables or other state shared between tests. They make writing tests easier


The setUp and tearDown methods are used in many places currently.
Should we fix that? Or remove this specification from the style guide?

I'm a big believer in stateless or near-stateless test fixtures, and have been chipping away at the pattern in flutter/packages. I often point people to https://abseil.io/tips/122 in reviews (I forgot we had this here).

I still think this is good advice, personally (and will continue to advocate for it even if it's not in the guide).

Piinks · 2025-07-17T16:23:00Z

 have side-effects that might change how other tests run.)


-### Prefer more test files, avoid long test files


In packages/flutter alone, 528 test files (out of 942) are over 200 lines in length.
The average line length for a test file in package/flutter is 1,438 lines, so we are clearly not following this.

We could adjust this rule to a more realistic line length, just remove it, or break up test files (which would be a lot of them).

I think a version of this rule with a much longer threshold would be a good one. Definitely the point about keeping each test file running quickly, for iteration when developing on it, is a compelling one for me — but to the extent that correlates with the number of lines of code, I think it kicks in at more like 1000 to 2000 lines than 100 to 200.

Piinks · 2025-07-17T16:23:27Z

-suite. (It also makes developing the tests faster because you can run the test file faster.)
-
-
-### Avoid using `pumpAndSettle`


This is being used in multiple places.
Should we remove usage? Or remove the rule?

I think this is a good rule — there sometimes are real regressions where something starts taking an extra frame, and it's good for tests to catch that. This is one of the rules in the Flutter style guide that I regularly link to when reviewing PRs in Zulip.

It's also a rule where it's understandable that people will sometimes feel like letting things slide instead, so it's no surprise that there are a fair number of places where it's currently not followed.

Piinks · 2025-07-17T16:24:14Z

 (for example, prefer `index` over `i`, but prefer `x` over `horizontalPosition`).


-### Avoid anonymous parameter names


Also being used in multiple places.
Should we remove usage or remove the rule?

Dart supporting wildcard variables now is something newer that we may be showing off as well.

I think the new Dart feature doesn't really affect the reasoning given for this rule. But personally I don't find that reasoning very compelling — this isn't a rule I would choose.

(This rule's reasoning also seems very closely related to the choice of "always annotate types" vs. "omit obvious types". We currently have the former, but personally I'd prefer the latter, and I gather many people agree; I know there's been work toward switching to the latter.)

Piinks · 2025-07-17T16:25:49Z

 line length of 100.


-### Consider using `=>` for short functions and methods


This and the following rule make for a complex decision around using =>. See following comment for full suggested adjustment.

(This rule and output from Gemini in flutter/packages prompted this audit 🙂 - flutter/packages#9627 (comment))

Piinks · 2025-07-17T16:26:50Z

-
-
-### Use braces for long functions and methods
+### Use braces for long functions and methods instead of `=>`


Proposal: Keep it simple.
We already have several violations of existing rules, most likely due to formatter.

stuartmorgan-g · 2025-07-17T17:05:19Z

-### Use braces for long functions and methods
+### Use braces for long functions and methods instead of `=>`

 Use a block (with braces) when a body would wrap onto more than one line (as opposed to using `=>`; the cases where you can use `=>` are discussed in the previous two guidelines).


If we remove the above, this line would need to change.

Piinks · 2026-02-06T01:30:20Z

Done here, this went into the style updates doc and discussion in Flutter Style Updates

Companion PR: #181934 Design doc: [Flutter Style Updates](https://docs.google.com/document/d/1Ao6RZmI4F8VSCpN-89o0Y6bZXhYb1-qgVGV5tALxm48/edit?tab=t.0) Closes #180607 Also closing #172296 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Companion PR: flutter#181934 Design doc: [Flutter Style Updates](https://docs.google.com/document/d/1Ao6RZmI4F8VSCpN-89o0Y6bZXhYb1-qgVGV5tALxm48/edit?tab=t.0) Closes flutter#180607 Also closing flutter#172296 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Looking at ways we aren't actually following the style guide

1ce67c3

github-actions Bot added c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. d: docs/ flutter/flutter/docs, for contributors labels Jul 17, 2025

Piinks commented Jul 17, 2025

View reviewed changes

stuartmorgan-g reviewed Jul 17, 2025

View reviewed changes

Piinks closed this Feb 6, 2026

Piinks mentioned this pull request Feb 6, 2026

Updates to style guide #181985

Merged

9 tasks

		have side-effects that might change how other tests run.)


		### Prefer more test files, avoid long test files

		suite. (It also makes developing the tests faster because you can run the test file faster.)


		### Avoid using `pumpAndSettle`

		(for example, prefer `index` over `i`, but prefer `x` over `horizontalPosition`).


		### Avoid anonymous parameter names

		line length of 100.


		### Consider using `=>` for short functions and methods



		### Use braces for long functions and methods
		### Use braces for long functions and methods instead of `=>`

Uh oh!

Conversation

Piinks commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Piinks Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Piinks commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Piinks commented Jul 17, 2025 •

edited

Loading

Piinks Jul 17, 2025 •

edited

Loading