[all] Omit obvious local types #10511
Conversation
stuartmorgan-g
added
override: no versioning needed
stuartmorgan-g
requested review from
LongCatIsLooong,
LouiseHsu,
MitchellGoodwin,
bkonyi,
bparrishMines,
camsim99,
domesticmouse,
gmackall,
hellohuanlin,
jesswrd,
mboetger,
mdebbar,
okorohelijah,
parlough,
reidbaker,
robert-ancell,
tarrinneal,
vashworth and
yjbanov
as code owners
github-actions
bot
added
p: google_fonts
triage-framework
|
@tarrinneal or @bparrishMines please review the non-automated commits (3620b2c, f231a2f, d700b45). Everyone else, please feel free to ignore this PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new linter rules to prefer type inference for obvious local variables and then applies these changes across the entire repository. The changes are extensive but mostly automated, resulting in more concise code by removing explicit type annotations where they are redundant. The modifications to analysis_options.yaml are correct and the subsequent code fixes align with the new linting rules. Overall, this is a solid stylistic improvement for the codebase.
chunhtai
left a comment
chunhtai
left a comment
There was a problem hiding this comment.
LGTM for go_router and go_router_builder part
|
Hm, that's unfortunate. From the |
Ah good to know! I will bump the framework too, I think it is still on 3.8 |
|
.. or 3.10. We're headed that way to adopt dot shorthands in the framework. Checking in with @loic-sharma |
I would not expect this issue to affect anything in the framework tree; it's specific to the fact that we run |
|
Ok. Touched base with Loïc, we want to publish guidelines/style guide for use of dot shorthands, so we won't bump to 3.10 until we have been able to finalize and communicate those. |
parlough
left a comment
parlough
left a comment
There was a problem hiding this comment.
package:mustache_template changes look good to me! Thanks :D
|
Oh so nice! 👏 |
|
Since there's a window of opportunity here where everything passed without any other PRs landing (and since unlike flutter/flutter we don't have a way to close the flutter/packages tree manually), I'm going to seize the moment and land this now rather than tomorrow. |
flutter/packages@e019cf9...cc3dca6 2025-11-24 [email protected] [all] Omit obvious local types (flutter/packages#10511) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Makes the analysis options changes described in flutter/flutter#178827: - Adding [omit_obvious_local_variable_types](https://dart.dev/tools/linter-rules/omit_obvious_local_variable_types) - Adding [specify_nonobvious_local_variable_types](https://dart.dev/tools/linter-rules/specify_nonobvious_local_variable_types) - Adding [specify_nonobvious_property_types](https://dart.dev/tools/linter-rules/specify_nonobvious_property_types) - Adding [type_annotate_public_apis](https://dart.dev/tools/linter-rules/type_annotate_public_apis) - Removing [always_specify_types](https://dart.dev/tools/linter-rules/always_specify_types) After those changes, makes the following repo-wide changes: - `dart fix --apply` in all packages and in script/tool/ - `dart format` in all packages and in script/tool/ - `update-excerpts` repo tooling command to update excerpts based on the changes to their sources Also updates the min Flutter/Dart SDK version to 3.35/3.9 for the following packages, to avoid `analyze` failures in the `N-2` legacy analysis run due to what appears to be a 3.9 change in what the Dart analyzer continues to be an obvious local type in loop iterations: - go_router - google_fonts - google_identity_services_web - google_maps_flutter_web - local_auth_platform_interface - metrics_center - multicast_dns - pigeon - rfw - shared_preferences - two_dimensional_scrollables - vector_graphics_compiler - mustache_template - path_parsing Because this is causing a significant amount of format churn already, I took this opportunity to update the repository tooling to a min Dart SDK of 3.8 (the N-2 stable version, so the earliest version we need the tooling to support) to pick up the new format style, so the amount of automated formatter change is higher in script/tool/ than in the packages. This does contain two manual changes (other than the repo tooling min version): - flutter@d700b45 changes `dynamic` to `Object?` in a few places where `dynamic` caused analyzer warnings under the new rule set. - Updates the repo tooling to ignore `.dart_tool/` when looking for unexpected local `analysis_options.yaml` files, to fix issues running the repo tool's `analyze` command locally based on recent changes in `dart` behavior. This does not include any CHANGELOG or version updates; even though we normally version any changes to production code, mass automated changes like this aren't worth the churn of releasing. This includes changes to lib/example/main.dart and to README.md excerpts; while the style changes will be user-visible on pub.dev, it's fine for those changes to wait for the next release of each package. Part of flutter/flutter#178827
flutter/packages@e019cf9...cc3dca6 2025-11-24 [email protected] [all] Omit obvious local types (flutter/packages#10511) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@e019cf9...cc3dca6 2025-11-24 [email protected] [all] Omit obvious local types (flutter/packages#10511) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
4 participants
Makes the analysis options changes described in flutter/flutter#178827:
After those changes, makes the following repo-wide changes:
dart fix --applyin all packages and in script/tool/dart formatin all packages and in script/tool/update-excerptsrepo tooling command to update excerpts based on the changes to their sourcesAlso updates the min Flutter/Dart SDK version to 3.35/3.9 for the following packages, to avoid
analyzefailures in theN-2legacy analysis run due to what appears to be a 3.9 change in what the Dart analyzer continues to be an obvious local type in loop iterations:Because this is causing a significant amount of format churn already, I took this opportunity to update the repository tooling to a min Dart SDK of 3.8 (the N-2 stable version, so the earliest version we need the tooling to support) to pick up the new format style, so the amount of automated formatter change is higher in script/tool/ than in the packages.
This does contain two manual changes (other than the repo tooling min version):
dynamictoObject?in a few places wheredynamiccaused analyzer warnings under the new rule set..dart_tool/when looking for unexpected localanalysis_options.yamlfiles, to fix issues running the repo tool'sanalyzecommand locally based on recent changes indartbehavior.This does not include any CHANGELOG or version updates; even though we normally version any changes to production code, mass automated changes like this aren't worth the churn of releasing. This includes changes to lib/example/main.dart and to README.md excerpts; while the style changes will be user-visible on pub.dev, it's fine for those changes to wait for the next release of each package.
Part of flutter/flutter#178827