Skip to content

Resolve validator dependencies in the factory instead of validators#1790

Open
henriquemoody wants to merge 1 commit into
Respect:mainfrom
henriquemoody:no_registry
Open

Resolve validator dependencies in the factory instead of validators#1790
henriquemoody wants to merge 1 commit into
Respect:mainfrom
henriquemoody:no_registry

Conversation

@henriquemoody

@henriquemoody henriquemoody commented Jun 12, 2026

Copy link
Copy Markdown
Member

A few validators (CountryCode, CurrencyCode, LanguageCode, Phone, SubdivisionCode, and Uuid) fetched their dependencies straight from the ContainerRegistry. That coupled every validator to a global registry and forced anyone providing a custom container to define those services, even when they only wanted to customize something unrelated.

Validators now receive their dependencies as optional constructor arguments. The new ContainerArgumentsResolver inspects a constructor and augments the given arguments with whatever the container can provide for the parameters they do not fill, skipping PHP internal classes (such as DateTimeImmutable) that are value-like and must never come from the container. NamespacedValidatorFactory and the Attributes validator share this resolver, so rules created through the fluent API and through PHP attributes both honor services defined in the container. The resolver hides behind the ArgumentsResolver interface, allowing clients to replace the resolution strategy entirely.

When no service is available, validators fall back to instantiating their default dependency directly, and they detect missing optional packages with class_exists() instead of container lookups. This makes the MissingComposerDependencyException accurate regardless of how bare the client's container is, but it also means the missing-package paths can no longer be simulated by swapping containers, so the tests doing that were removed.

Creating a rule that resolves services from the container costs about 1µs more than before, the price of querying the container and mapping named arguments at creation time. Rules without resolvable parameters pay around 0.1µs, and rules whose arguments already fill the constructor skip the parameter inspection entirely.

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.40%. Comparing base (436fc89) to head (9293a26).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1790      +/-   ##
============================================
- Coverage     99.41%   98.40%   -1.01%     
- Complexity     1023     1061      +38     
============================================
  Files           195      196       +1     
  Lines          2392     2448      +56     
============================================
+ Hits           2378     2409      +31     
- Misses           14       39      +25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR decouples several validators from the global ContainerRegistry by moving dependency resolution into a factory-level ArgumentsResolver, enabling optional service injection from a PSR-11 container while preserving sensible defaults when services/packages are absent.

Changes:

  • Introduces ArgumentsResolver plus a default ContainerArgumentsResolver that augments constructor arguments with container-provided services (with an “unresolvable types” denylist).
  • Updates NamespacedValidatorFactory and the Attributes validator to use the resolver so both fluent rules and PHP attributes can receive injected services.
  • Refactors CountryCode, CurrencyCode, LanguageCode, Phone, SubdivisionCode, and Uuid to accept optional dependencies and use class_exists() for missing-package detection; updates docs and tests accordingly.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/Validators/UuidTest.php Removes container-swap test for missing ramsey/uuid path.
tests/unit/Validators/SubdivisionCodeTest.php Removes container-swap test for missing ISO Codes path.
tests/unit/Validators/PhoneTest.php Removes container-swap tests simulating missing isocodes/libphonenumber services.
tests/unit/Validators/LanguageCodeTest.php Removes container-swap test for missing ISO Codes path.
tests/unit/Validators/CurrencyCodeTest.php Removes container-swap test for missing ISO Codes path.
tests/unit/Validators/CountryCodeTest.php Removes container-swap test for missing ISO Codes path.
tests/unit/Validators/AttributesTest.php Adds tests asserting attribute validator dependency injection when a resolver is provided.
tests/unit/NamespacedRuleFactoryTest.php Adds tests covering container-based constructor injection behavior in the validator factory.
tests/unit/ContainerArgumentsResolverTest.php Adds unit tests for argument augmentation, denylist behavior, and internal class handling.
tests/src/Validators/InjectableWithPdo.php Adds a test validator attribute used to verify internal-class injection behavior (PDO).
tests/src/Validators/Injectable.php Adds a test validator attribute used to verify interface/class injection behavior (Transformer).
src/Validators/Uuid.php Removes ContainerRegistry usage; injects/creates UuidFactory and uses class_exists() for missing package detection.
src/Validators/SubdivisionCode.php Removes ContainerRegistry usage; injects/creates ISO Codes databases; uses class_exists() for missing package detection.
src/Validators/Phone.php Removes ContainerRegistry usage; injects/creates PhoneNumberUtil/Countries; uses class_exists() for missing package detection.
src/Validators/LanguageCode.php Removes ContainerRegistry usage; injects/creates Languages; uses class_exists() for missing package detection.
src/Validators/CurrencyCode.php Removes ContainerRegistry usage; injects/creates Currencies; uses class_exists() for missing package detection.
src/Validators/CountryCode.php Removes ContainerRegistry usage; injects/creates Countries; uses class_exists() for missing package detection.
src/Validators/Attributes.php Adds optional ArgumentsResolver to instantiate attribute validators with injected services when available.
src/NamespacedValidatorFactory.php Adds optional ArgumentsResolver hook to resolve constructor args before instantiation.
src/ContainerRegistry.php Registers ArgumentsResolver and wires it into the default ValidatorFactory creation.
src/ContainerArgumentsResolver.php New resolver that augments missing constructor parameters from a container, skipping denylisted types.
src/ArgumentsResolver.php New public interface enabling custom resolution strategies.
src-dev/Markdown/Linters/ValidatorHeaderLinter.php Updates linter exclusions for newly injectable/external dependency types.
src-dev/Commands/LintMixinCommand.php Updates mixin lint exclusions to match new injectable/external dependency types.
docs/validators/Attributes.md Documents resolver-driven injection behavior for attribute validators when created via fluent API.
docs/configuration.md Adds documentation for service injection and custom arguments resolver configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ContainerArgumentsResolver.php
@henriquemoody henriquemoody force-pushed the no_registry branch 4 times, most recently from 9cc0738 to 73f6d68 Compare June 12, 2026 15:49
A few validators (CountryCode, CurrencyCode, LanguageCode, Phone,
SubdivisionCode, and Uuid) fetched their dependencies straight from the
ContainerRegistry. That coupled every validator to a global registry and
forced anyone providing a custom container to define those services,
even when they only wanted to customize something unrelated.

Validators now receive their dependencies as optional constructor
arguments. The new ContainerArgumentsResolver inspects a constructor and
augments the given arguments with whatever the container can provide for
the parameters they do not fill, skipping PHP internal classes (such as
DateTimeImmutable) that are value-like and must never come from the
container. NamespacedValidatorFactory and the Attributes validator share
this resolver, so rules created through the fluent API and through PHP
attributes both honor services defined in the container. The resolver
hides behind the ArgumentsResolver interface, allowing clients to
replace the resolution strategy entirely.

When no service is available, validators fall back to instantiating
their default dependency directly, and they detect missing optional
packages with class_exists() instead of container lookups. This makes
the MissingComposerDependencyException accurate regardless of how bare
the client's container is, but it also means the missing-package paths
can no longer be simulated by swapping containers, so the tests doing
that were removed.

Creating a rule that resolves services from the container costs about
1µs more than before, the price of querying the container and mapping
named arguments at creation time. Rules without resolvable parameters
pay around 0.1µs, and rules whose arguments already fill the constructor
skip the parameter inspection entirely.
@alganet

alganet commented Jun 12, 2026

Copy link
Copy Markdown
Member

I believe the work done on Respect\Parameter is related to this idea. Have you seen it?

@henriquemoody

Copy link
Copy Markdown
Member Author

Respect\Parameter doesn't augment parameter, it will resolve all dependencies from a callable, but I agree that Respect\Parameter is where this logic should live, though. I'm going to send a PR with some changes there as soon as I can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants