Sitelet https://github.com/flutter/flutter/pull/191510
Skip to content

[flutter_tools] migrate getBuildInfo() to typed OptionDescriptors and BuildInfoOptionsBundle - #191510

Merged
auto-submit[bot] merged 12 commits into
flutter:masterfrom
kevmoo:get-build-info-bundle
Aug 25, 2026
Merged

auto-submit[bot] merged 12 commits into
flutter:masterfrom
kevmoo:get-build-info-bundle

Conversation

@kevmoo

@kevmoo kevmoo commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Second step in the type-safe, declarative CLI argument refactor for flutter_tools (following flutter/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.

@github-actions github-actions Bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 21, 2026
Comment thread packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated
@kevmoo

kevmoo commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors 'FlutterCommand' to retrieve command-line arguments using typed option descriptors ('BuildInfoOptions', 'CommonOptions', and 'WebOptions') via 'getValue', replacing direct 'argParser' lookups. It also introduces the 'BuildInfoOptions' class and its corresponding option bundle, along with unit tests to verify default values and historical aliases. Feedback suggests refactoring a redundant 'getValue' call for the code size directory, using literal strings instead of confusing constants for front-end and gen-snapshot aliases, and adding documentation comments to the newly introduced public members in 'BuildInfoOptions' to comply with the style guide.

Comment thread packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated
Comment thread packages/flutter_tools/lib/src/runner/options/common_options.dart
Comment thread packages/flutter_tools/lib/src/runner/options/common_options.dart
Comment thread packages/flutter_tools/lib/src/runner/options/common_options.dart
@kevmoo
kevmoo marked this pull request as ready for review August 21, 2026 23:33
@kevmoo kevmoo added the CICD Run CI/CD label Aug 21, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors FlutterCommand to retrieve command-line options using typed option descriptors via getValue instead of directly querying argParser.options. It introduces BuildInfoOptions and BuildInfoOptionsBundle to centralize these descriptors, along with corresponding unit tests. The review feedback suggests removing redundant .toList() calls on getValue results and adding documentation comments to the new public static fields in BuildInfoOptions to comply with the style guide.

Comment thread packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated
Comment thread packages/flutter_tools/lib/src/runner/options/common_options.dart

@harryterkelsen harryterkelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 25, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Aug 25, 2026
Merged via the queue into flutter:master with commit ba57163 Aug 25, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 25, 2026
pull Bot pushed a commit to Budda0ne/flutter that referenced this pull request Sep 4, 2026
…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**.
@kevmoo
kevmoo deleted the get-build-info-bundle branch September 4, 2026 22:32
pull Bot pushed a commit to ZainCheung/flutter that referenced this pull request Sep 8, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants