Clean up avoid_type_to_string suppressions#186869
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. 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. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
78780ab to
790e522
Compare
There was a problem hiding this comment.
Code Review
This pull request replaces runtimeType.toString() calls with hardcoded strings or string interpolation to address the avoid_type_to_string lint. Feedback indicates that hardcoding class names results in a loss of dynamic behavior for subclasses. Furthermore, using string interpolation on Type objects may not effectively resolve the lint's underlying concerns regarding obfuscation and could still be flagged by the linter.
790e522 to
9ef677d
Compare
|
Addressed Gemini feedback in head |
9ef677d to
607ee2b
Compare
|
Addressed the review feedback in 607ee2b. The bot exceptions now use |
607ee2b to
4fc4d62
Compare
|
Updated in 4fc4d62 to match the maintainer feedback: the tool crash analytics path now keeps |
bkonyi
left a comment
There was a problem hiding this comment.
One last change, then this LGTM!
4fc4d62 to
29c127e
Compare
|
Addressed the last review thread in 29c127e: |
29c127e to
ddab037
Compare
|
Addressed the two output-initialization nits in ddab037: both bot exception toString implementations now initialize output as |
…11832) Manual roll requested by [email protected] flutter/flutter@701665b...2ba5420 2026-06-03 [email protected] Roll Packages from 818b310 to b11504f (8 revisions) (flutter/flutter#187511) 2026-06-03 [email protected] Add new file patterns for team-web labeler (flutter/flutter#187397) 2026-06-03 [email protected] Roll Skia from 279b17fe9fc1 to d625048c853a (12 revisions) (flutter/flutter#187483) 2026-06-03 [email protected] [SwiftPM] Fix prefer_initializing_formals lint (flutter/flutter#187502) 2026-06-03 [email protected] Roll Fuchsia Linux SDK from q27k7_um1GvVrySZS... to ap7MhLX4TdpWRrLS_... (flutter/flutter#187478) 2026-06-03 [email protected] [SwiftPM] Fix concurrent directory/file/symlink creation crashes (flutter/flutter#186953) 2026-06-03 [email protected] [Impeller] Fix positioning of text shadow masks (flutter/flutter#187460) 2026-06-02 [email protected] Remove Material imports from painting tests (flutter/flutter#186937) 2026-06-02 [email protected] Add android_hardware_smoke_test integration tests (flutter/flutter#187130) 2026-06-02 [email protected] Roll pub packages (flutter/flutter#187471) 2026-06-02 [email protected] [flutter tool] propagate analytics env to sub-tools (flutter/flutter#186780) 2026-06-02 [email protected] Adds macro for fragment shaders to support flutter <= 3.44 (flutter/flutter#187316) 2026-06-02 [email protected] Small clean-up in different java files under `engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/` (flutter/flutter#186631) 2026-06-02 [email protected] refactor(web): Unify ui.Path code for CanvasKit and Skwasm (flutter/flutter#187331) 2026-06-02 [email protected] Roll Packages from f5d50ca to 818b310 (2 revisions) (flutter/flutter#187441) 2026-06-02 [email protected] Allow selecting multi-digit device options (flutter/flutter#186184) 2026-06-02 [email protected] Improve error message for type mismatch in Navigator.pop and maybePop. (flutter/flutter#186571) 2026-06-02 [email protected] Remove semantics_tester import from material_button_test.dart (flutter/flutter#184807) 2026-06-02 [email protected] [flutter_tools] Refactor hostPlatform to use Abi.current() (flutter/flutter#185369) 2026-06-02 [email protected] Clean up avoid_type_to_string suppressions (flutter/flutter#186869) 2026-06-02 [email protected] Update Flutter documentation links in flutter_console.bat (flutter/flutter#187354) 2026-06-02 [email protected] Revert "[Impeller] Allow attaching specific texture mip levels and slices" (flutter/flutter#187445) 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 Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
…lutter#11832) Manual roll requested by [email protected] flutter/flutter@701665b...2ba5420 2026-06-03 [email protected] Roll Packages from 818b310 to b11504f (8 revisions) (flutter/flutter#187511) 2026-06-03 [email protected] Add new file patterns for team-web labeler (flutter/flutter#187397) 2026-06-03 [email protected] Roll Skia from 279b17fe9fc1 to d625048c853a (12 revisions) (flutter/flutter#187483) 2026-06-03 [email protected] [SwiftPM] Fix prefer_initializing_formals lint (flutter/flutter#187502) 2026-06-03 [email protected] Roll Fuchsia Linux SDK from q27k7_um1GvVrySZS... to ap7MhLX4TdpWRrLS_... (flutter/flutter#187478) 2026-06-03 [email protected] [SwiftPM] Fix concurrent directory/file/symlink creation crashes (flutter/flutter#186953) 2026-06-03 [email protected] [Impeller] Fix positioning of text shadow masks (flutter/flutter#187460) 2026-06-02 [email protected] Remove Material imports from painting tests (flutter/flutter#186937) 2026-06-02 [email protected] Add android_hardware_smoke_test integration tests (flutter/flutter#187130) 2026-06-02 [email protected] Roll pub packages (flutter/flutter#187471) 2026-06-02 [email protected] [flutter tool] propagate analytics env to sub-tools (flutter/flutter#186780) 2026-06-02 [email protected] Adds macro for fragment shaders to support flutter <= 3.44 (flutter/flutter#187316) 2026-06-02 [email protected] Small clean-up in different java files under `engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/` (flutter/flutter#186631) 2026-06-02 [email protected] refactor(web): Unify ui.Path code for CanvasKit and Skwasm (flutter/flutter#187331) 2026-06-02 [email protected] Roll Packages from f5d50ca to 818b310 (2 revisions) (flutter/flutter#187441) 2026-06-02 [email protected] Allow selecting multi-digit device options (flutter/flutter#186184) 2026-06-02 [email protected] Improve error message for type mismatch in Navigator.pop and maybePop. (flutter/flutter#186571) 2026-06-02 [email protected] Remove semantics_tester import from material_button_test.dart (flutter/flutter#184807) 2026-06-02 [email protected] [flutter_tools] Refactor hostPlatform to use Abi.current() (flutter/flutter#185369) 2026-06-02 [email protected] Clean up avoid_type_to_string suppressions (flutter/flutter#186869) 2026-06-02 [email protected] Update Flutter documentation links in flutter_console.bat (flutter/flutter#187354) 2026-06-02 [email protected] Revert "[Impeller] Allow attaching specific texture mip levels and slices" (flutter/flutter#187445) 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 Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
Fixes flutter#185310. Issue: Several `avoid_type_to_string` suppressions were either avoidable or undocumented. The Flutter tool crash analytics path still needs a best-effort concrete exception category for unknown error types, so replacing the type with a generic category loses useful crash signal. Fix: - Keeps tool crash analytics on `error.runtimeType.toString()` with a local comment explaining why that suppression is intentional. - Makes `PreparePackageException` and `UnpublishException` final and uses `$PreparePackageException` / `$UnpublishException` so their string output follows future class renames. - Leaves GitHub crash template sanitization on stable categories where privacy/sanitization matters. - Adds focused coverage for the bot exception `toString()` output. Tests: - `./bin/dart format --output=none --set-exit-if-changed packages/flutter_tools/lib/runner.dart packages/flutter_tools/test/general.shard/runner/runner_test.dart` - `./bin/flutter analyze dev/bots/prepare_package/common.dart dev/bots/unpublish_package.dart packages/flutter_tools/lib/runner.dart packages/flutter_tools/lib/src/reporting/github_template.dart dev/bots/test/prepare_package_test.dart dev/bots/test/unpublish_package_test.dart packages/flutter_tools/test/general.shard/runner/runner_test.dart packages/flutter_tools/test/general.shard/github_template_test.dart` - `./bin/flutter test packages/flutter_tools/test/general.shard/runner/runner_test.dart packages/flutter_tools/test/general.shard/github_template_test.dart` - `./bin/flutter test packages/flutter_tools/test/general.shard/github_template_test.dart` - `./bin/dart test dev/bots/test/prepare_package_test.dart` - `./bin/dart test dev/bots/test/unpublish_package_test.dart` - `git diff --check` Risk: Low. The PR removes avoidable suppressions, keeps the one remaining suppression explicit, and preserves useful crash analytics signal where the tool is not obfuscated.
Fixes #185310.
Issue: Several
avoid_type_to_stringsuppressions were either avoidable or undocumented. The Flutter tool crash analytics path still needs a best-effort concrete exception category for unknown error types, so replacing the type with a generic category loses useful crash signal.Fix:
error.runtimeType.toString()with a local comment explaining why that suppression is intentional.PreparePackageExceptionandUnpublishExceptionfinal and uses$PreparePackageException/$UnpublishExceptionso their string output follows future class renames.toString()output.Tests:
./bin/dart format --output=none --set-exit-if-changed packages/flutter_tools/lib/runner.dart packages/flutter_tools/test/general.shard/runner/runner_test.dart./bin/flutter analyze dev/bots/prepare_package/common.dart dev/bots/unpublish_package.dart packages/flutter_tools/lib/runner.dart packages/flutter_tools/lib/src/reporting/github_template.dart dev/bots/test/prepare_package_test.dart dev/bots/test/unpublish_package_test.dart packages/flutter_tools/test/general.shard/runner/runner_test.dart packages/flutter_tools/test/general.shard/github_template_test.dart./bin/flutter test packages/flutter_tools/test/general.shard/runner/runner_test.dart packages/flutter_tools/test/general.shard/github_template_test.dart./bin/flutter test packages/flutter_tools/test/general.shard/github_template_test.dart./bin/dart test dev/bots/test/prepare_package_test.dart./bin/dart test dev/bots/test/unpublish_package_test.dartgit diff --checkRisk: Low. The PR removes avoidable suppressions, keeps the one remaining suppression explicit, and preserves useful crash analytics signal where the tool is not obfuscated.