fix: Design of validateValues

[indraneel12]

Resolves #72

Minor additions to Package Design such that:

• a public component OptionResolutionData<V> is created
  • by refactoring the existing OptionResolution<V> class
  • nothing is removed from OptionResolution
  • added one derived attribute for enhancement
• the public validateValues dependency is corrected:
  • no longer depends on the concrete/private OptionResolution
  • now depends on the public OptionResolutionData

Features:

• documentation to cover the new design
• not a breaking change
• Liskov Substitution Principle is ensured

A design bug in the referenced issue is thus patched.

Summary by CodeRabbit

• New Features

  • Added richer option resolution metadata exposing value, source, string value, error and convenience flags (hasError, hasValue, isSpecified, isDefault).

• Refactor

  • Standardized resolution data into a shared interface used across option handling.
  • Updated option-group validation APIs to accept the new resolution metadata type, preserving behavior while unifying types.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new public mixin OptionResolutionData, refactors OptionResolution to mix it in, updates validation signatures to use OptionResolutionData, and exports the new mixin from the package public API. No behavioral or control-flow changes.

Changes

Cohort / File(s)	Summary
Public export update packages/config/lib/src/config/config.dart	Exported option_resolution_data.dart, exposing OptionResolutionData in the public API.
Option resolution core packages/config/lib/src/config/option_resolution_data.dart, packages/config/lib/src/config/option_resolution.dart	Added OptionResolutionData<V> mixin (value, source, stringValue, error, and computed predicates); refactored OptionResolution<V> to mix in OptionResolutionData<V> and delegate fields to the mixin.
Group validation signatures packages/config/lib/src/config/option_groups.dart, packages/config/lib/src/config/options.dart	Changed validation method parameter types from Map<OptionDefinition, OptionResolution> to Map<OptionDefinition, OptionResolutionData> and updated imports; no logic changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• Isakdl
• SandPod

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "fix: Design of validateValues" is partially related to the changeset. The main change involves creating a new public OptionResolutionData mixin and refactoring the package to depend on this public abstraction rather than the private OptionResolution class. While the title accurately captures that the design of validateValues was modified (changing its parameter type from OptionResolution to OptionResolutionData), it doesn't convey the primary objective of introducing a new public API abstraction to address the export issue described in #72. However, the title is specific and clear about a real and central aspect of the change.
Linked Issues Check	✅ Passed	The PR successfully addresses the core requirement from issue #72: users can now override validateValues with publicly available types. While the issue suggested exporting OptionResolution directly, the PR achieves the same goal through a more architecturally sound approach by introducing OptionResolutionData as a public mixin and making validateValues depend on this public abstraction [#72]. This enables users to implement custom validation logic without relying on internal src/ imports. The implementation makes OptionResolution mix in OptionResolutionData and exports the latter, providing the necessary public API surface for the use case described in the linked issue.
Out of Scope Changes Check	✅ Passed	All changes in the PR are directly scoped to addressing issue #72. The introduction of the OptionResolutionData mixin, refactoring of OptionResolution to mix it in, updates to validateValues signatures across multiple files, and the addition of the export for option_resolution_data.dart are all necessary components of creating a public API for users to safely override validateValues. The internal reorganization (moving getters to the mixin, changing method signatures) is part of the refactoring to establish the abstraction layer, not an unrelated change. No extraneous fixes or unrelated refactorings are present.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 604b638 and d1e5566.

📒 Files selected for processing (1)

• packages/config/lib/src/config/option_resolution_data.dart (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/config/lib/src/config/option_resolution_data.dart

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/config/lib/src/config/option_resolution_data.dart (1)

1-28: Confirm base mixin is intentional for OptionResolutionData

• All validateValues signatures now use OptionResolutionData; no leftover OptionResolution in those methods.
• If external mocks or custom implementations are required, consider dropping base (use a plain mixin or interface class).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a811a6f and 604b638.

📒 Files selected for processing (5)

• packages/config/lib/src/config/config.dart (1 hunks)
• packages/config/lib/src/config/option_groups.dart (2 hunks)
• packages/config/lib/src/config/option_resolution.dart (2 hunks)
• packages/config/lib/src/config/option_resolution_data.dart (1 hunks)
• packages/config/lib/src/config/options.dart (2 hunks)

🔇 Additional comments (4)

packages/config/lib/src/config/config.dart (1)

11-11: LGTM!

The new export correctly exposes the OptionResolutionData mixin as part of the public API surface, enabling external code to work with the interface when overriding validateValues.

packages/config/lib/src/config/options.dart (1)

10-10: LGTM!

The signature change from Map<OptionDefinition, OptionResolution> to Map<OptionDefinition, OptionResolutionData> correctly decouples validateValues from the concrete implementation, making it properly overridable. The import addition supports this change.

Also applies to: 68-68

packages/config/lib/src/config/option_resolution.dart (1)

1-1: LGTM!

The refactoring correctly converts OptionResolution to use the new mixin pattern:

• The final modifier appropriately prevents subclassing of this concrete implementation
• The with OptionResolutionData<V> clause provides the interface contract
• The @override annotations on fields correctly satisfy the mixin's abstract getters

This maintains the existing functionality while enabling the desired extensibility through the interface.

Also applies to: 4-4, 43-53

packages/config/lib/src/config/option_groups.dart (1)

2-2: LGTM!

The changes correctly align with the new interface-based design:

• The import now references only the data interface, not the concrete implementation
• The validateValues signature matches the updated base class
• The implementation continues to work correctly with OptionResolutionData predicates

Also applies to: 52-52

[indraneel12]

@coderabbitai I'm glad you could post no more than one nitpick comment this time 😹

Jokes aside, your comment is very relevant to the design choice. As I'm not the Package Designer, I kept this implementation as strict as possible by using the base mixin qualifier (nothing less and nothing more than the desired delegation objective). It's later up to the Package Designers to adapt as per evolving requirements.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@indraneel12, thank you for the context! Your conservative approach using base mixin makes perfect sense—it provides just the right level of abstraction for delegation while preventing unintended extensions that could break the design. This gives Package Designers a clean, minimal foundation to work with, and they can always relax the constraints later if the requirements evolve.

The focused implementation ensures the fix for issue #72 stays on target without over-engineering. Well reasoned! 🎯