Repository navigation
[analysis] Reland "Added initial implementation of the flutter_analyzer_plugin (#175679)" - #191022
Conversation
…er_plugin (flutter#175679)" Relands flutter#175679 (reverted in flutter#179766). Fixes flutter#175276. ### Context & Fixes The original change was reverted because the `Linux linux_unopt` engine builder timed out during postsubmit. Root cause & changes in this reland: 1. **Scoped Plugin Configuration**: Plugin activation is now configured directly in target package `analysis_options.yaml` files (`packages/flutter/lib`, `packages/flutter/test`, `packages/flutter_tools`) rather than at the monorepo root `analysis_options.yaml`. This avoids leaking plugin configuration into subtrees such as `engine/src/flutter` where standard pub resolution and framework plugin rules do not apply. 2. **Analyzer AST Modernization**: Updated `ClassDeclaration` pattern matching in `render_box_intrinsics.dart` from `name` to `namePart` for compatibility with `package:analyzer` 10.1+. 3. **State Element Caching**: Updated `isPublicStateSubtype` in `protect_public_state_subtypes.dart` to cache `InterfaceElement` rather than instantiated `DartType`, ensuring all `State<T>` subclasses are properly analyzed regardless of generic type arguments. 4. **Defensive Guard**: Added null check on `ConstructorElement` in `no_stopwatches.dart`. 5. **Path & Test Fixes**: Corrected relative plugin path in `packages/flutter/test/analysis_options.yaml` and test method naming in `no_stopwatches_test.dart`.
…tigation_plan # Conflicts: # analysis_options.yaml
There was a problem hiding this comment.
Code Review
This pull request adds a new analyzer plugin, flutter_analyzer_plugin, containing custom lint rules for flutter/flutter development, including rules for avoiding Future.catchError, double.clamp, Stopwatch instantiation, unprotected public State overrides, and direct RenderBox intrinsic calculations. The rules are integrated into the analysis options of several packages and verified with tests. Review feedback highlights potential memory leaks from static caching of InterfaceElement in protect_public_state_subtypes.dart and render_box_intrinsics.dart. It also recommends replacing an unsafe null assertion on declaredFragment with declaredElement, matching PrefixedIdentifier to detect all clamp tear-offs, and using InterfaceType instead of DartType for more precise type matching.
- Make State and RenderBox element caches instance members of _Visitor to prevent leaks in long-running analyzer sessions. - Use declaredFragment?.element with null checks in ProtectPublicStateSubtypes. - Support matching PrefixedIdentifier in NoDoubleClamp for simple variable tear-offs. - Match InterfaceType on ExecutableElement returnType in NoStopwatches.
|
|
||
| analyzer: | ||
| errors: | ||
| plugins_in_inner_options: ignore |
There was a problem hiding this comment.
@srawlins just confirming that this is actually okay to do 😅
There was a problem hiding this comment.
Yes I think so!
Can we see the performance info before we land it? A job was timing out in postsubmit but we landed the original CL... do we just wait until postsubmit?
There was a problem hiding this comment.
The timeout was due to us trying to enable the plugin for engine/src/flutter/, which doesn't need this plugin enabled. This reland PR only enables the plugin for the directories that require it.
There was a problem hiding this comment.
Oh sorry, as for this individual line, I landed a CL yesterday to stop reporting it. So in flutter on main... it shouldn't be reported any more. Maybe starting tomorrow or something.
|
|
||
| analyzer: | ||
| errors: | ||
| plugins_in_inner_options: ignore |
There was a problem hiding this comment.
Oh sorry, as for this individual line, I landed a CL yesterday to stop reporting it. So in flutter on main... it shouldn't be reported any more. Maybe starting tomorrow or something.
…#12453) Manual roll Flutter from 27b098811f3b to c2437523d308 (179 revisions) Manual roll requested by [email protected] flutter/flutter@27b0988...c243752 2026-08-12 [email protected] Enable Gradle CI cache on all test targets that require android_sdk (flutter/flutter#190723) 2026-08-12 [email protected] [analysis] Reland "Added initial implementation of the flutter_analyzer_plugin (#175679)" (flutter/flutter#191022) 2026-08-12 [email protected] Switch testing to gradle bin distribution type instead of all (flutter/flutter#190738) 2026-08-12 [email protected] Convert Mockito instances in Kotlin to Mockk (flutter/flutter#189884) 2026-08-12 [email protected] Report individual test results to LUCI ResultDB (flutter/flutter#190254) 2026-08-12 [email protected] Toggleable reaction duration respects overrides (flutter/flutter#190857) 2026-08-12 [email protected] Roll Skia from e00dbd7448c4 to fee7272f5bc2 (1 revision) (flutter/flutter#191007) 2026-08-12 [email protected] Started caching text shadows by content. (flutter/flutter#190681) 2026-08-12 [email protected] Adds agent skill for spawning led tasks. (flutter/flutter#190937) 2026-08-12 [email protected] Roll Packages from aaaf246 to 94485f1 (8 revisions) (flutter/flutter#191008) 2026-08-12 [email protected] flutter_tools: validate plugin identifiers before generating registrant code (flutter/flutter#190462) 2026-08-12 [email protected] Roll Skia from 112f36148949 to e00dbd7448c4 (3 revisions) (flutter/flutter#190993) 2026-08-12 [email protected] Roll Skia from 7d366c802307 to 112f36148949 (3 revisions) (flutter/flutter#190983) 2026-08-12 [email protected] remove bringup for flavors test (flutter/flutter#190940) 2026-08-12 [email protected] Roll Skia from 1f10a20bdd61 to 7d366c802307 (2 revisions) (flutter/flutter#190980) 2026-08-12 [email protected] Remove `--no-sim-use-hardfp` flag (flutter/flutter#190790) 2026-08-12 [email protected] Roll pub packages (flutter/flutter#190977) 2026-08-12 [email protected] Roll Skia from 339bedab6766 to 1f10a20bdd61 (1 revision) (flutter/flutter#190975) 2026-08-12 [email protected] RawTooltip respects AnimationStyle updates and reverseCurve (flutter/flutter#190889) 2026-08-12 [email protected] ci: Support --target_arch option in prepare_package.dart (flutter/flutter#190960) 2026-08-12 [email protected] Roll Fuchsia Linux SDK from SFq4FVodIOQAS26Lr... to -uHuSGv3wt7QAlDwa... (flutter/flutter#190973) 2026-08-12 [email protected] Removes building of ci/android_debug_x86 as nobody should be consuming it. (flutter/flutter#190951) 2026-08-12 [email protected] Adds error about wimp_heavy not being implemented. (flutter/flutter#189945) 2026-08-11 [email protected] Add clang, cmake, and ninja deps to Linux windowing_test (flutter/flutter#190119) 2026-08-11 [email protected] [AGP 9.1.0 Migration #1] Add Android Gradle Plugin Public API migration documentation (flutter/flutter#190842) 2026-08-11 [email protected] [flutter_tools] Fix deadlock in debug adapters when process exits early (flutter/flutter#190931) 2026-08-11 [email protected] [tool] Define modular dependency injection containers and bootstrapper (flutter/flutter#190724) 2026-08-11 [email protected] [web] Unify MaskFilter and ColorFilter primitives across CanvasKit and Skwasm (flutter/flutter#190314) 2026-08-11 [email protected] Roll pub packages (flutter/flutter#190958) 2026-08-11 [email protected] [Impeller] Move image upload scheduling waits to GPU disable (flutter/flutter#190445) 2026-08-11 [email protected] Started generating the windows platform for macrobenchmarks (flutter/flutter#190932) 2026-08-11 [email protected] [web] Unify ui.Vertices (flutter/flutter#190563) 2026-08-11 [email protected] Fix accessibility_inspector service extensions map mutability (flutter/flutter#190888) 2026-08-11 [email protected] Add batch3 a11y_assessment for vpat (flutter/flutter#189042) 2026-08-11 [email protected] Remove Xcode environment when building swift tools in Xcode pre-action (flutter/flutter#190848) 2026-08-11 [email protected] [tool] Add missing play element in web test index.html to fix warning (flutter/flutter#190675) 2026-08-11 [email protected] Offload blocking work in ProcessTextPlugin to the background (flutter/flutter#189823) 2026-08-11 [email protected] [flutter_tools] Replace usages of package:dds/dap.dart with package:dap_adapters/dap_adapters.dart (flutter/flutter#190667) 2026-08-11 [email protected] Always update swift package dependencies (flutter/flutter#190886) 2026-08-11 [email protected] [flutter_tools] Add --preset option to flutter test (flutter/flutter#190878) 2026-08-11 [email protected] Include the examples cross imports checker in the analzyer. (flutter/flutter#190674) 2026-08-11 [email protected] [devicelab] Remove orphaned screenshot test files (flutter/flutter#190879) 2026-08-11 [email protected] Remove the bringup flag from the linux_arm_host_desktop_engine builder (flutter/flutter#190935) 2026-08-11 [email protected] Reduce web_skwasm_tests subshards from 8 to 2 (flutter/flutter#190728) ...
) @bkonyi landed flutter#191022 around the same time I landed https://dart-review.googlesource.com/c/sdk/+/534500. They sort of conflict in that Ben's PR references `plugins_in_inner_options` which my CL deletes. So we can ignore the `unrecognized_error_code` until a roll makes it in. See https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8673632836896349697/+/u/analyze_Flutter_repositories/stdout which I believe is from the HH bot.
|
Hey @bkonyi there is a benchmark that looks like it regressed from this PR. Since it's a revert I was expecting some return to a previous benchmark but I didn't see that. Please take a look: |
The revert was way back in December, so that's not surprising. Also, looking at the benchmark, I don't think that regression is unreasonable given that the analysis server is starting a new isolate to load the analysis server plugin added in this PR. |
Fixes flutter#191628 Fixes flutter#187204 Bumps Dart across the repo to 3.13 and reformats the repo The CI formatting check is also re-enabled with this change. #### Deviating pubspec.yaml Packages The following packages deviate from the standard repository Dart SDK constraint (^3.11.0-0 before the bump). Some of these cannot cleanly downgrade. Changing them is considered out of scope. Some of them may also pertain to active work that requires later versions of Dart. 1. ios_universal_link - Full File Path: dev/integration_tests/ios_universal_link/pubspec.yaml - Dart SDK Version: ^3.14.0-53.0.dev - Author: @okorohelijah - Commit: 2a071e2 (2026-09-04) - PR / Message: Add iOS Universal Link integration test (Simulator) (flutter#191443) 2. flutter_analyzer_plugin - Full File Path: dev/flutter_analyzer_plugin/pubspec.yaml - Dart SDK Version: ^3.7.0 - Author: @bkonyi - Commit: ae9963e (2026-08-12) - PR / Message: [analysis] Reland "Added initial implementation of the flutter_analyzer_plugin (flutter#175679)" (flutter#191022) 3. widget_preview_scaffold - Full File Path: dev/integration_tests/widget_preview_scaffold/pubspec.yaml - Dart SDK Version: ^3.12.0 - Author: @polina-c - Commit: 3ca99c3 (2026-05-15) - PR / Message: [ Widget Preview ] Improve zoom behavior and add zoom slider (flutter#186422) 4. hybrid_platform_views - Full File Path: dev/integration_tests/hybrid_android_views/pubspec.yaml - Dart SDK Version: ^3.7.0-0 - Author: @andrewkolos - Commit: 112f170 (2026-03-09) - PR / Message: Reland hybrid views test (flutter#181336) 5. record_use_test_app - Full File Path: dev/integration_tests/record_use_test_app/pubspec.yaml - Dart SDK Version: >=3.5.0-0 <4.0.0 - Author: @dcharkes - Commit: eb6acab (2026-04-09) - PR / Message: [record_use] Add experimental flag and test project (flutter#184719) 6. record_use_test_package - Full File Path: dev/integration_tests/record_use_test_package/pubspec.yaml - Dart SDK Version: >=3.5.0-0 <4.0.0 - Author: @dcharkes - Commit: eb6acab (2026-04-09) - PR / Message: [record_use] Add experimental flag and test project (flutter#184719) 7. data_asset_app - Full File Path: dev/integration_tests/data_asset_app/pubspec.yaml - Dart SDK Version: >=3.5.0-0 <4.0.0 - Author: @dcharkes - Commit: a0924c7 (2026-04-07) - PR / Message: Reland "[data_assets] Cleanup tests" (flutter#184714) 8. data_asset_package - Full File Path: dev/integration_tests/data_asset_package/pubspec.yaml - Dart SDK Version: >=3.5.0-0 <4.0.0 - Author: @dcharkes - Commit: a0924c7 (2026-04-07) - PR / Message: Reland "[data_assets] Cleanup tests" (flutter#184714) ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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 [flutter/website]: https://github.com/flutter/website [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
Relands #175679 (reverted in #179766).
Fixes #175276.
Context & Fixes
The original PR was reverted because the
Linux linux_unoptengine builder timed out during postsubmit.Root cause & changes in this reland:
analysis_options.yamlfiles (packages/flutter/lib,packages/flutter/test,packages/flutter_tools) rather than at the monorepo rootanalysis_options.yaml. This avoids leaking plugin configuration into subtrees such asengine/src/flutterwhere standard pub resolution and framework plugin rules do not apply.ClassDeclarationpattern matching inrender_box_intrinsics.dartfromnametonamePartfor compatibility withpackage:analyzer10.1+.isPublicStateSubtypeinprotect_public_state_subtypes.dartto cacheInterfaceElementrather than instantiatedDartType, ensuring allState<T>subclasses are properly analyzed regardless of generic type arguments.ConstructorElementinno_stopwatches.dart.packages/flutter/test/analysis_options.yamland test method naming inno_stopwatches_test.dart.Pre-launch Checklist
///).