Skip to content

[args] Add core OptionDescriptor types and ArgResults lookup (1/3) - #1012

Open
kevmoo wants to merge 6 commits into
mainfrom
args-typed
Open

kevmoo wants to merge 6 commits into
mainfrom
args-typed

Conversation

@kevmoo

@kevmoo kevmoo commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Adds core declarative OptionDescriptor<T> types and ArgParser/Option extensions while keeping args.dart and command_runner.dart public APIs unchanged and withholding the public package:args/typed.dart export until PR 3 (#1017).

…d bundles

- Add `package:args/typed.dart` (`OptionDescriptor`, `OptionBundle`, `ValueOptionBundle`, `CliEnum`, and typed extensions on `ArgResults`, `Command`, `ArgParser`, and `Option`).
- Add package-private `descriptorOf`, `isOptionRegistered`, and `rootOf` helpers in `lib/src/` to avoid external `Expando` registries, resolve `defaultsTo: null` option registration in `O(1)`, and automatically link subcommand `ArgResults` to top-level global results.
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

PR Health

Publish ✔️
Package Version Status Publish tag (post-merge)
package:args ⭐ 2.8.0-wip WIP (no publish necessary)
package:os_detect 2.0.4-preview.1 ready to publish os_detect-v2.0.4-preview.1
  • 2 already published.
  • 9 WIP (no publish necessary).

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

This check can be disabled by tagging the PR with skip-publish-check.

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
args Breaking 2.7.0 2.8.0-wip 2.8.0-wip ✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Unused Dependencies ✔️
Package Status
args ✔️ All dependencies utilized correctly.

For details on how to fix these, see dependency_validator.

This check can be disabled by tagging the PR with skip-unused-dependencies-check.

License Headers ✔️
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/collection/benchmark/legacy_quicksort.dart

This check can be disabled by tagging the PR with skip-license-check.

Coverage ✔️
File Coverage
pkgs/args/lib/src/arg_results.dart 💚 87 % ⬆️ 1 %
pkgs/args/lib/src/option.dart 💚 94 % ⬆️ 1 %
pkgs/args/lib/src/typed/option_descriptor.dart 💚 96 %
pkgs/args/lib/src/typed/typed_extensions.dart 💚 100 %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

…nd add verboseOnly/override hooks

- Validate `IntOptionDescriptor` and `DefaultedIntOptionDescriptor` at parse time in `Parser._validateAllowed` so invalid integer arguments throw `ArgParserException` during `ArgParser.parse()` (and `UsageException` in `CommandRunner.run()`).
- Remove `{ArgResults? globalResults}` plumbing from `OptionDescriptor`, `ValueOptionBundle.parse`, and `SafeArgResults` since `Parser.parse()` automatically links child `ArgResults` to the root `ArgResults`.
- Hoist `aliases` onto `OptionDescriptor<T>` and extract `_StringOptionDescriptorBase<R>` and `_FlagOptionDescriptorBase<R>`.
- Add `verboseOnly` and `{bool verboseHelp = false, bool? hide, String? help}` registration overrides to `OptionDescriptor.addTo`, `OptionBundle.register`, and `ArgParserBundleExtension`.
- Widen bulk registration methods to `Iterable<...>` and add `addDescriptor` / `addDescriptors` on `ArgParser`.
…gh OptionBundle

- Add `OptionDescriptor.isRegisteredOnParser(ArgParser)` and check
  `identical(descriptorOf(option), this)` in `isRegistered`,
  `wasProvided`, `_resolveTargetResults`, and `getValue` so same-name
  collisions (e.g. an imperative flag or distinct descriptor sharing an
  option name) never falsely match or prevent `OptionScope.any` from
  falling back to `globalResults`.
- Update `CommandSafeArgResults.hasOption` to check `argParser` and
  `runner?.argParser` before `argResults` is parsed, as well as post-parse
  `argResults` / `_effectiveResults`.
- Pass `bool? hide` through `OptionBundle.register`, `registerOnCommand`,
  `CommandSafeArgResults.registerOptionBundle` / `registerOptionBundles`,
  and `ArgParserBundleExtension.registerBundle` / `registerBundles`.
- Default `MultiOptionDescriptor.defaultsTo` to `const <String>[]`.
… and immutability

- Move descendant subcommand root-linking from `Parser.parse` into
  `newArgResults`, eliminating the cognitive complexity increase in
  `Parser.parse`.
- Guard `parserOf`, `rootOf`, `setRootOf`, and `descriptorOf` against
  custom `Fake` / `Mock` test doubles implementing `ArgResults` or `Option`.
- Add `Option.copyTo` on `TypedOptionExtension` to preserve descriptor
  identity and value validators when cloning `ArgParser`s, and add a
  metadata fallback in `OptionDescriptor.isRegisteredOnParser` for
  wholesale imperative `ArgParser` clones.
- Return `List<String>.unmodifiable` from `MultiOptionDescriptor.getValue`.
- Throw `FormatException` in `EnumOptionDescriptor` and
  `DefaultedEnumOptionDescriptor` when a custom `valueParser` rejects a
  non-null value.
- Prevent duplicate separator headers when `OptionBundle`s with a `title`
  are registered multiple times, and expand `test/typed_test.dart` to 31
  tests (342/342 total).
@kevmoo
kevmoo marked this pull request as ready for review October 17, 2026 01:08
@kevmoo
kevmoo requested a review from a team as a code owner October 17, 2026 01:08
…lts lookup (1/3)

Defer IntOptionDescriptor, EnumOptionDescriptor, CliEnum, OptionBundle,
Command extensions, and the public lib/typed.dart export to stacked PRs
2/3 and 3/3 so each slice can be reviewed independently.
@kevmoo kevmoo changed the title [args] Add package:args/typed.dart with typed option descriptors and bundles [args] Add core OptionDescriptor types and ArgResults lookup (1/3) Oct 17, 2026
@natebosch

Copy link
Copy Markdown
Member

IMO a change like this warrants a design discussion before we get to a PR review. This will take a significant effort to review.

@kevmoo

kevmoo commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

@natebosch – I just did a whole re-stack of this PR into smaller chunks.

I've been working with Harry and @bkonyi since the beginning on this w/ the Flutter repo.

It's all landed there with bells on. 😄

Let's follow-up in chat?

- Expose `value(ArgResults)`, `valueOrDefault(ArgResults)`, `wasParsed(ArgResults)`, and `isRegistered(ArgResults)` directly on `OptionDescriptor<T>` with strict registration verification.
- Remove `OptionScope`, `ArgResults._root`, and `SafeArgResults` so typed descriptors layer directly on `ArgParser` and `ArgResults`.
- Shorten concrete descriptor names (`FlagDescriptor`, `NullableFlagDescriptor`, `StringDescriptor`, `DefaultedStringDescriptor`, `MultiStringDescriptor`), omit `defaultsTo` on nullable descriptors, and add `TypedArgParserExtension` / `TypedOptionExtension` in `typed_extensions.dart`.
@kevmoo

kevmoo commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Updated the stack (#1012, #1016, #1017) based on review feedback:

  • Moved extraction directly onto OptionDescriptor<T> (descriptor.value(results) and descriptor.valueOrDefault(results)) and removed SafeArgResults.
  • Removed OptionScope and ArgResults root tracking; callers pass argResults! or globalResults! explicitly.
  • Shortened concrete names to *Descriptor (FlagDescriptor, NullableFlagDescriptor, StringDescriptor, DefaultedStringDescriptor, MultiStringDescriptor, etc.), removed nullable defaultsTo constructor parameters, and added hideNegatedUsage.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Package publishing

If you have publishing permissions, you can use the links below to publish the changes after merging this PR.

Package Version Status Publish tag (post-merge)
package:args ⭐ 2.8.0-wip WIP (no publish necessary)
package:os_detect 2.0.4-preview.1 ready to publish os_detect-v2.0.4-preview.1
  • 2 already published.
  • 9 WIP (no publish necessary).

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

ievdokdm pushed a commit to ievdokdm/flutter that referenced this pull request Nov 6, 2026
…lutter#196076)

Aligns `OptionDescriptor.addTo` (`{bool? hide, String? help}`) with
upcoming `package:args/typed.dart`
(dart-lang/core#1012) and removes the three
non-`const` `OptionDescriptor` factories in `DebuggingOptionDescriptors`
so every option lookup queries the exact descriptor instance registered
on `ArgParser`:

- **`DebuggingOptionDescriptors.devToolsServerAddress`**: Consolidates
[`devToolsServerAddress(...)` and
`devToolsServerAddressOption`](https://github.com/flutter/flutter/blob/1eecf014c5c47aada3a859fd05fa180a6ccee7dd/packages/flutter_tools/lib/src/runner/options/common_options.dart#L924-L945)
into a single `const` descriptor, passing
`devToolsServerAddressBaseHelp` via `addTo(..., help: ...)` when
`--devtools` is omitted so
[`FlutterCommand.devToolsServerAddress`](https://github.com/flutter/flutter/blob/1eecf014c5c47aada3a859fd05fa180a6ccee7dd/packages/flutter_tools/lib/src/runner/flutter_command.dart#L423-L431)
matches the registered instance.
- **`RunCommandBase._startPaused` & `DriveCommand._publishPort`**:
Replaces [`startPaused(...)` /
`startPausedOption`](https://github.com/flutter/flutter/blob/1eecf014c5c47aada3a859fd05fa180a6ccee7dd/packages/flutter_tools/lib/src/commands/run.dart#L84)
([queried at line
211](https://github.com/flutter/flutter/blob/1eecf014c5c47aada3a859fd05fa180a6ccee7dd/packages/flutter_tools/lib/src/commands/run.dart#L211))
with an instance-scoped `_startPaused` field on `RunCommandBase`, and
inlines [`publishPortOption(enabledByDefault:
false)`](https://github.com/flutter/flutter/blob/1eecf014c5c47aada3a859fd05fa180a6ccee7dd/packages/flutter_tools/lib/src/runner/options/common_options.dart#L893-L904)
as `static const _publishPort` on `DriveCommand`.

Part of flutter#194338

## 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.

<!-- 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/CONTRIBUTING.ai.md
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants