Repository navigation
[flutter_tools] refactor CLI argument architecture with typed option descriptors and bundles (PoC) - #191018
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a type-safe command-line option and flag registration system for Flutter commands, refactoring the web build command to use the new OptionBundle and OptionDescriptor architecture. The review feedback highlights opportunities to simplify the output directory path retrieval by removing redundant fallback logic. Additionally, several style guide violations were identified, including missing documentation on public option descriptors and the need for more descriptive error messages when conflicting option descriptors are registered.
… vertical slice - Introduce OptionDescriptor and OptionBundle abstractions in src/runner/options/ - Add SafeArgResults extension for type-safe option access without raw strings - Define CommonBuildOptionsBundle and WebOptionsBundle for cohesive option grouping - Encapsulate web compile parameters in WebBuildSpecification struct - Refactor BuildWebCommand to declare optionBundles and eliminate 130+ lines of imperative setup boilerplate - Update WebBuilder.buildWeb to take WebBuildSpecification - Update unit tests in build_web_test.dart and compile_web_test.dart
… remove null-coalescing checks
…on registry and bundles Replaces OptionDescriptor<dynamic> with OptionDescriptor<Object?> across OptionDescriptor, OptionBundle, CommonOptions, SafeArgResults, FlutterCommand, and WebOptions bundles.
- Simplify outputDirectoryPath by removing redundant stringArg fallback - Add contextual conflict error messages in OptionDescriptor with safe null handling
74fd956 to
2d04ad6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors command-line option registration and parsing in flutter_tools by introducing type-safe OptionDescriptor and OptionBundle classes, migrating the build web command to use these bundles. The review feedback identifies a regression where several options are no longer dynamically hidden under standard --help output because the descriptors are now static constants. To resolve this, the reviewer suggests introducing an isHidden check in OptionBundle and overriding it in the respective bundles. Additionally, the reviewer recommends removing unused verboseHelp parameters in BuildModeOptionsBundle and CommonBuildOptionsBundle to simplify the constructors.
… OptionBundle and OptionDescriptor - Add `verboseOnly` parameter to `OptionDescriptor` and subclasses to declaratively mark advanced/internal options. - Propagate `command.verboseHelp` through `OptionBundle.register()` to automatically hide verbose options when verbose help is not requested. - Mark `enable-experiment`, `dump-info`, `minify-js`, `minify-wasm`, `enable-wasm-deferred-loading`, and `no-frequency-based-minification` as `verboseOnly: true` to restore exact parity with legacy CLI help behavior. - Support `aliases` on `MultiOptionDescriptor`. - Add unit tests in `option_descriptor_test.dart` and expand `build_web_test.dart` to verify option visibility under both standard and verboseHelp modes.
…n defaults - Drop redundant `verboseHelp` from all `OptionBundle` constructors, enabling compile-time `const` bundle singletons. - Forward `defaultsTo` in `MultiOptionDescriptor.addTo` to ensure non-empty defaults register with `ArgParser`. - Use `const` descriptor lists across web option bundles. - Simplify descriptor conflict duplicate check to use `identical(existing, this)`. - Add unit tests in `option_descriptor_test.dart` verifying `MultiOptionDescriptor` custom `defaultsTo` propagation.
|
autosubmit label was removed for flutter/flutter/191018, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…s_test style compliance
- Adopt super-parameter syntax required super.verboseHelp in BuildSubCommand. - Use enum dot shorthands and when guard clause in _resolveTargetResults. - Extract _computeEffectiveHide helper to deduplicate visibility logic across option descriptors.
…Descriptor - Eliminates single-use effectiveHide local variables across all OptionDescriptor addTo methods.
Roll Flutter from c2437523d308 to 65c9a8dc60bc (195 revisions) flutter/flutter@c243752...65c9a8d 2026-08-22 engine-flutter-autoroll@skia.org Roll ICU from d578f2e8b7bd to 8cc91d9b6ab9 (1 revision) (flutter/flutter#191542) 2026-08-22 engine-flutter-autoroll@skia.org Roll Skia from 666a9b3d5cf0 to ad2106c0bb64 (3 revisions) (flutter/flutter#191527) 2026-08-22 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from KaOq3EE4qJ9fnaaaK... to 0iCv10IlKfiilEBOU... (flutter/flutter#191524) 2026-08-22 bkonyi@google.com [flutter_tools] Add tests for negative lookahead regex in test runner and batch entrypoints (flutter/flutter#191438) 2026-08-22 engine-flutter-autoroll@skia.org Roll Skia from 0c37868737fa to 666a9b3d5cf0 (2 revisions) (flutter/flutter#191518) 2026-08-21 10456171+caroqliu@users.noreply.github.com Revert "[input] Migrate fuchsia.ui.pointerinjector to TouchSource (#190855) (flutter/flutter#191509) 2026-08-21 30870216+gaaclarke@users.noreply.github.com Fixes windows gallery benchmarks by forcing mobile layout (flutter/flutter#191507) 2026-08-21 bkonyi@google.com [flutter_tools] Restrict WebAssetServer source resolution to source map extensions (flutter/flutter#191501) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from f6900c5b8439 to 0c37868737fa (2 revisions) (flutter/flutter#191504) 2026-08-21 bkonyi@google.com Refactor `FlutterDevice.connect` and VM service discovery (flutter/flutter#191221) 2026-08-21 bkonyi@google.com [flutter_tools] Fix crash when migrating flow-style exclude lists in analysis_options.yaml (flutter/flutter#191269) 2026-08-21 269567208+reidbaker-agent@users.noreply.github.com [rules] Add packages/flutter_tools/gradle/AGENTS.md rules (flutter/flutter#191486) 2026-08-21 kevmoo@users.noreply.github.com [flutter_tools] refactor CLI argument architecture with typed option descriptors and bundles (PoC) (flutter/flutter#191018) 2026-08-21 bkonyi@google.com tools: Extract Dart SDK to temp directory before moving to final location (flutter/flutter#191263) 2026-08-21 1961493+harryterkelsen@users.noreply.github.com [web] Move CanvasKit fragment shader classes to canvaskit/fragment_shader.dart (flutter/flutter#191451) 2026-08-21 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from GCQlmt6h-esJsNubS... to ic6GjOSn-KN508XyK... (flutter/flutter#191485) 2026-08-21 bkonyi@google.com [Widget Preview] Isolate PageStorage scope in widget preview group expansion tile (flutter/flutter#191378) 2026-08-21 bkonyi@google.com [flutter_tools] Deprecate --build and --no-build flags on flutter run (flutter/flutter#191358) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from 70988bed1b3b to f6900c5b8439 (2 revisions) (flutter/flutter#191481) 2026-08-21 engine-flutter-autoroll@skia.org Roll Packages from 1785501 to 252bb33 (6 revisions) (flutter/flutter#191480) 2026-08-21 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 20IJas24bZiNmCZTK... to GCQlmt6h-esJsNubS... (flutter/flutter#191415) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from 2ba6971bd0d1 to 70988bed1b3b (2 revisions) (flutter/flutter#191476) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from 1d5f72537ba6 to 2ba6971bd0d1 (2 revisions) (flutter/flutter#191473) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from 2c25efd2e369 to 1d5f72537ba6 (1 revision) (flutter/flutter#191472) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from 09b1b810850a to 2c25efd2e369 (9 revisions) (flutter/flutter#191470) 2026-08-21 flar@google.com [Impeller] fix position of cached single glyph text shadows (flutter/flutter#191325) 2026-08-21 engine-flutter-autoroll@skia.org Roll Skia from abdf8821f313 to 09b1b810850a (15 revisions) (flutter/flutter#191458) 2026-08-21 me@bnsaed.com Document that programmatic TextEditingController changes do not run input formatters (flutter/flutter#190166) 2026-08-21 30870216+gaaclarke@users.noreply.github.com Adds new gallery benchmarks to windows (skia and impeller) (flutter/flutter#191454) 2026-08-21 chris@bracken.jp iOS: Deprecate FlutterEngine.isGpuDisabled (flutter/flutter#191393) 2026-08-21 154381524+flutteractionsbot@users.noreply.github.com Revert: [web] Unskip decoration image lerp tests (flutter/flutter#191462) 2026-08-20 awolff@google.com android_hardware_smoke_test: Improve reliability (flutter/flutter#191374) 2026-08-20 77467499+wilyan09007@users.noreply.github.com Don't size or offset the Android platform view before it is laid out (flutter/flutter#190895) 2026-08-20 15619084+vashworth@users.noreply.github.com [iOS][add2app] Skip building SwiftPM plugins when generating CocoaPods artifacts (flutter/flutter#190736) 2026-08-20 33794642+FelixMittermeier@users.noreply.github.com Optimize JSONMessageCodec UTF-8 conversion (flutter/flutter#190529) 2026-08-20 bkonyi@google.com [FML] Replace deprecated wstring_convert with Win32 APIs (flutter/flutter#191394) 2026-08-20 victorsanniay@gmail.com SliverFillRemaining extends beyond viewport size when fillOverscroll is true (flutter/flutter#191236) 2026-08-20 bkonyi@google.com [flutter_tools] Update argParser usageLineLength when --wrap-column is passed (flutter/flutter#191264) 2026-08-20 bkonyi@google.com [flutter_tools] Fix UNC path resolution in depfile parsing on Windows (flutter/flutter#191265) 2026-08-20 1961493+harryterkelsen@users.noreply.github.com [web] Unskip decoration image lerp tests (flutter/flutter#191426) 2026-08-20 bkonyi@google.com Do not inject 'type' and 'method' into service extension responses (flutter/flutter#190946) 2026-08-20 bkonyi@google.com [flutter_tools] Fix crash in symbolize command on stream error (flutter/flutter#191273) 2026-08-20 bkonyi@google.com [flutter_tools] Prevent deletion of shared native asset hooks outputs as stale (flutter/flutter#191272) 2026-08-20 bkonyi@google.com [flutter_tools] Support package wildcard assets in app pubspec (flutter/flutter#191266) 2026-08-20 108678139+manu-sncf@users.noreply.github.com Add SliverClipRect and SliverClipRRect (flutter/flutter#179003) 2026-08-20 bkonyi@google.com [flutter_tools] Implement Diagnostics extension slice and doctor integration (flutter/flutter#191162) ...
… BuildInfoOptionsBundle (flutter#191510) Second step in the type-safe, declarative CLI argument refactor for `flutter_tools` (following [flutter#191018](flutter#191018)). This PR migrates `getBuildInfo()` in `FlutterCommand` to use typed option descriptors and `SafeArgResults`, eliminating 21 defensive string-based `argParser.options.containsKey('...')` queries. ### Key Changes 1. **`BuildInfoOptionsBundle` (`lib/src/runner/options/common_options.dart`)**: - Encapsulates options specific to `BuildInfo` creation (`track-widget-creation`, `analyze-size`, `code-size-directory`, `obfuscate`, `split-debug-info`, `android-gradle-daemon`, `android-project-arg`, `android-project-cache-dir`, `android-skip-build-dependency-validation`, `performance-measurement-file`, `flavor`, `codesign`, `frontend-server-starter-path`, `initialize-from-dill`, `assume-initialize-from-dill-up-to-date`, `extra-front-end-options`, `extra-gen-snapshot-options`). - Grouped into declarative `const` option descriptors maintaining 100% backward compatibility for names, abbreviations, aliases, help strings, and defaults. 2. **`getBuildInfo()` Refactor (`lib/src/runner/flutter_command.dart`)**: - Replaced all 21 imperative string lookups with type-safe `wasParsed()` / `getValue()` calls via `SafeArgResults`. 3. **Testing & Verification**: - Added unit test suite `test/general.shard/runner/options/build_info_options_bundle_test.dart` verifying all descriptors register properly. - `dart analyze` across `packages/flutter_tools`: **0 issues**. - Runner and build unit tests (`flutter_command_test.dart`, `build_test.dart`): **250/250 passing**.
…ter#191760) Third step in the type-safe, declarative CLI argument refactor for `flutter_tools` (following flutter#191018 and flutter#191510). This PR introduces typed enum option descriptors and defaulted string/enum descriptors, and migrates existing Web and Common options to use them. ### Key Changes 1. **`EnumOptionDescriptor<T extends Enum>` & `DefaultedEnumOptionDescriptor<T extends Enum>` (`lib/src/runner/options/option_descriptor.dart`)**: - Automatically derives `allowed` values via `values.map(...)`. - Supports `nameMapper`, `valueParser`, and automatic `CliEnum` mapping for `allowed` and `allowedHelp`. - Returns strongly-typed `T?` or `T` from `getValue()`, eliminating manual `fromCliName()` parsing and casting. 2. **`DefaultedStringOptionDescriptor` (`lib/src/runner/options/option_descriptor.dart`)**: - Requires concrete non-null `defaultsTo`. - Statically types `getValue()` to return non-nullable `String`. - Removes ad-hoc `getValueOrDefault()` helper from `StringOptionDescriptor`. 3. **Migration of Existing Options**: - Migrated `WebOptions.pwaStrategy` from `static final StringOptionDescriptor` to `const EnumOptionDescriptor<ServiceWorkerStrategy>`. - Migrated `CommonOptions.target` from `StringOptionDescriptor` to `const DefaultedStringOptionDescriptor`. - Updated `build_web.dart` to pass `getValue(WebOptions.pwaStrategy)` directly without manual `fromCliName()` conversion. ### Verification - Whole-package `dart analyze` across `packages/flutter_tools`: **0 issues**. - Unit and command tests (`option_descriptor_test.dart`, `build_info_options_bundle_test.dart`, `build_web_test.dart`): **55/55 tests passing**.
…criptor (flutter#192307) Migrates `createDebuggingOptions()` in `RunCommandBase` (`lib/src/commands/run.dart`) and associated option registration helpers in `FlutterCommand` (`lib/src/runner/flutter_command.dart`) to typed `OptionDescriptor` instances, continuing the CLI argument architecture modernization started in flutter#191018, flutter#191510, and flutter#191760. ## Summary of Changes * **Typed Debugging Option Descriptors (`DebuggingOptionDescriptors` in `common_options.dart`)**: * Defined 35+ strongly-typed descriptors covering engine flags (`enableImpeller`, `enableFlutterGpu`, `enableVulkanValidation`, `enableEmbedderApi`, `enableHcpp`), tracing/profiling (`traceStartup`, `traceSystrace`, `traceToFile`, `endlessTraceBuffer`, `profileMicrotasks`, `traceSkia`, `traceAllowlist`, `traceSkiaAllowlist`, `enableDartProfiling`, `profileStartup`, `cacheStartupProfile`, `verboseSystemLogs`, `purgePersistentCache`), VM service/DevTools/DDS (`dds`, `ddsPort`, `disableDds`, `enableDevTools`, `devToolsServerAddress`, `vmserviceOutFile`, `disableServiceAuthCodes`, `disableServiceOriginCheck`, `startPaused`, `dartFlags`, `ipv6`), and runner configuration (`route`, `enableSoftwareRendering`, `skiaDeterministicRendering`, `dartEntrypointArgs`, `uninstallFirst`, `iosProfileDebugger`, `useTestFonts`, `adbLogFiltering`, `testFlag`). * **Web Server & Debugging Descriptors (`WebOptions` in `web_options.dart`)**: * Added typed descriptors for `webHeader`, `webHostname`, `webPort`, `webTlsCertPath`, `webTlsCertKeyPath`, `webServerDebugProtocol`, `webServerDebugBackendProtocol`, `webServerDebugInjectedClientProtocol`, `webAllowExposeUrl`, `webRunHeadless`, `webBrowserDebugPort`, `webEnableExpressionEvaluation`, `webLaunchUrl`, `webBrowserFlags`, and `crossOriginIsolation`. * **`ArgParserDescriptorExtension` & Automatic `Expando` Registry (`option_descriptor.dart`)**: * Added `addDescriptor` / `addDescriptors` extension methods on `ArgParser` for concise option registration. * Replaced explicit `Map<String, OptionDescriptor>? registry` parameter plumbing across `OptionDescriptor.addTo()`, `OptionBundle.register()`, and `FlutterCommand._optionRegistry` with an internal `Expando<Map<String, OptionDescriptor<Object?>>>` on `ArgParser`, ensuring automatic diamond-bundle de-duplication and conflict detection across all registration paths. * **Tri-State `bool?` Semantics**: * Used `NullableFlagOptionDescriptor` (`defaultsTo: null`) for flags whose omitted state is semantically distinct from `false` (`enable-impeller`, `enable-flutter-gpu`, `enable-hcpp`, `cross-origin-isolation`, `ios-profile-debugger`), allowing `getValue(...)` to preserve `bool?` without manual string lookups. * **Eliminated Untyped Lookups in `createDebuggingOptions()`**: * Replaced 35+ untyped `argParser.options.containsKey('...')`, `boolArg('...')`, `stringArg('...')`, and `stringsArg('...')` lookups in `RunCommandBase.createDebuggingOptions()`, `webDevServerConfigCore()`, and `FlutterCommand` helpers (`usesWebOptions`, `addDdsOptions`, `addDevToolsOptions`, `explicitEnableHcpp`, `ddsPort`, `devToolsServerAddress`) with type-safe `getValue(...)`, `wasParsed(...)`, and `hasOption(...)` calls. ## 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] All existing and new tests are passing.
Exploring a type-safe, declarative argument parsing foundation for
flutter_tools, validated end-to-end againstflutter build web.Today,
flutter_toolssubcommands rely heavily on imperative option registration methods scattered acrossFlutterCommand(~48uses*/add*methods) and loose string-based argument extraction (stringArg('foo'),boolArg('bar')), which leads to high boilerplate, runtime string typos, and difficulty refactoring CLI flags.Key Changes
Typed Option Descriptors (
lib/src/runner/options/option_descriptor.dart):OptionDescriptor<T>,StringOptionDescriptor,FlagOptionDescriptor,NullableFlagOptionDescriptor, andMultiOptionDescriptor.constdeclarations owning their name, abbreviations, defaults, help text, allowed values, andverboseOnlydynamic visibility.Domain Option Bundles (
lib/src/runner/options/option_bundle.dart):OptionBundleallows cohesive grouping of related options into reusable compile-timeconstunits with automatic section header separators in--helpoutput (viatitleandsubBundles).command.verboseHelpdown to all child descriptors during registration.common_options.dart(CommonBuildOptionsBundle,BuildModeOptionsBundle,DartCompileOptionsBundle).Type-Safe Argument Access (
lib/src/runner/options/safe_arg_results.dart):T getValue<T>(OptionDescriptor<T> descriptor)onFlutterCommandthat leverages Dart type inference to return non-nullablebool,bool?,List<String>, orString?without type casting or null-coalescing noise.wasProvided()(aliasedwasParsed()) for explicit presence queries.Vertical Slice Validation on
BuildWebCommand:BuildWebCommandwith 4 declarativeconstbundles.WebCoreOptionsBundle,WebJsOptionsBundle, andWebWasmOptionsBundleinlib/src/web/web_options.dart.Verification
dart analyzeacrosspackages/flutter_tools: 0 issues.option_descriptor_test.dart,build_web_test.dart,compile_web_test.dart): 51/51 tests passing.