Remove Composite base class

[henriquemoody]

The Composite class didn't provide much value: just a constructor gathering validators and a getValidators() method that wasn't used.

Because the Validator interface is so simple (only an evaluate method), it's not always worth creating abstract base classes that only provide constructor logic. Each composite validator can implement the interface directly with minimal duplication.

Assisted-by: Opencode (GLM 4.7)
Assisted-by: Claude Code (Opus 4.5)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.21%. Comparing base (882f24b) to head (f5eec9f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #1653   +/-   ##
=========================================
  Coverage     99.20%   99.21%           
- Complexity      915      918    +3     
=========================================
  Files           191      190    -1     
  Lines          2146     2152    +6     
=========================================
+ Hits           2129     2135    +6     
  Misses           17       17           

☔ View full report in Codecov by Sentry.
📢 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]

Pull request overview

This pull request removes the Composite abstract base class and updates all composite validators (AllOf, AnyOf, Circuit, NoneOf, OneOf) to implement the Validator interface directly. The Composite class provided minimal value, only offering constructor logic and an unused getValidators() method.

Changes:

• Removed the Composite base class and its associated test files
• Updated five composite validators to implement Validator directly with minimal duplication
• Updated migration documentation to remove reference to Composite base class

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Validators/Core/Composite.php	Removed the abstract Composite base class
tests/unit/Validators/Core/CompositeTest.php	Removed tests for the now-deleted Composite class
tests/src/Validators/Core/ConcreteComposite.php	Removed test helper concrete implementation of Composite
src/Validators/AllOf.php	Now implements Validator directly with inline constructor and validators property
src/Validators/AnyOf.php	Now implements Validator directly with inline constructor and validators property
src/Validators/Circuit.php	Now implements Validator directly with inline constructor and validators property
src/Validators/NoneOf.php	Now implements Validator directly with inline constructor and validators property
src/Validators/OneOf.php	Now implements Validator directly with inline constructor and validators property
docs/migrating-from-v2-to-v3.md	Removed Composite from the list of available base classes

---

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

[alganet]

I'm not sure about this. That base class estabilished a common behavior across all logic-style composites (must have two arguments or more).

This is different from what we discussed in #1650. I said:

This "check for two arguments" exists in two places. In the function signature of Core\Composite and in the match body of this new validator. Is there a way we can abstract that so that is expressed in a single place instead?

I pointed out that the check for multiple parameters was spread out into two places: the new public Composite (via match block having a default > 1) and Core\Composite (via function args).

When you said that didn't went unnoticed, I was expecting a PR that solves that issue, not just the name collision.

With this change, this spread is further amplified to more classes. The abstract still exists "spiritually" (via the constructors having the exact same signature and implementation). If you ask an AI to refactor AllOf, AnyOf, etc for reusability, it will probably switch back to having a common abstract among them.

In broad strokes, what I was expecting was:

• A base composite class that accepts 0..N arguments.
• A subtype of that base composite that restricts the input to 2..N arguments.
• The logical composites subtyping the 2..N one.

This would provide a clear class hierarchy with more general on the root and more specific on the leaves. It would still preserve the same behavior we have today.

[henriquemoody]

I was talking about the naming collision, not so much the problem you're describing. I'm sorry I wasn't clear enough 😕

As I mentioned in the commit message, I don't see much the point on keeping that abstract class just for the construtor. If PHP didn't have construtor inheritance, I wouldn't miss it, to be honest, and some languages like Java, C++, and C# don't even support it. The Composite is just a void class, with no behavior, we're just using its construtor to reuse the same parameters. I see constructors as unique, pretty much like a function, because there's no contract you can check against, it's just an implementation.

You know the contract of array_map(), hence you use it, and if it fails it fails. You can't check if the class extends Composite and simply create the object assuming it has 1 or N parameters because each implementation could have its own construtor. It's different with a method in a class, in which you do have a contract, and you can assume that method X have the contract you're expecting.

I know this is controversial amongst different engineers, maybe we're just on opposite sides here.

[alganet]

Java, C++, and C#

We're PHP though!

we're just using its construtor to reuse the same parameters

And why is that bad? Reuse is good.

I see constructors as unique, pretty much like a function, because there's no contract you can check against, it's just an implementation.

I don't understand what you mean here. Tools like phpstan depend heavily on constructor signatures, and do consider inheritance dynamics.

PHP has traits, which are implementation only. The hierarchy mismatch I pointed out can be solved by using them as well.

I know this is controversial amongst different engineers, maybe we're just on opposite sides here.

Even on your side (constructors as unique pristine signatures), that doesn't solve the hierarchy mismatch I pointed out.

IMHO, that problem is more offensive to OOP theory than variadic signatures.

[henriquemoody]

I think we see the Composite type differently. For me it has no value because we don't treat a validator differently if they're a Composite instance or not. To me, creating a type to identify those validators is the same as creating a type to identify which validators deal with strings, arrays, etc. I don't see the value of it because they're all the same on the light of objects that use them: an instance of Validator.

We don't have a case for like Composite, and until we don't have it, I would say we don't need it.

Tools like phpstan depend heavily on constructor signatures, and do consider inheritance dynamics.

I think tools like PHPStan are a good example of what I'm saying, the way PHPStan checks if str_pad() was called correctly is the same way it checks if new DateTime() was called: it checks the signature. Constructors are not part of the "type" that defines a class. Amongst everything that class inherits that's exposed to the clients (public/protect properties/method) the construtor is only thing that you cannot guarantee that the contract of a child class is the same as the parent.

And why is that bad? Reuse is good.

Never said it's bad! I think Envelop is very valuable, but funny enough, we always overwrite its construtor -- it was designed for that.

[henriquemoody]

Closed due recent changes in #1650