Fix double validation and state leaks in Attributes recursion

Recursive validation for nested objects guarded against validating a
nested object twice by checking whether the property already
carried #[Attributes] directly on the property. That guard did not
recognise the rule inside a wrapper, and the state backing the
recursion was never released, producing five defects:

- #[NullOr(new Attributes())] on a class-typed property validated
  the nested object twice and reported every nested failure twice. This
  was the documented way of validating a nullable nested object before
  recursion existed, so upgrading silently duplicated messages.
- The same object held by two sibling properties failed as a circular
  reference, because visited objects accumulated for the whole traversal
  instead of the current path.
- An Attributes instance could only be used once: nothing ever cleared
  the visited objects, so every evaluation after the first failed.
- A union type whose value satisfied more than one of its class members
  recursed once per member, and the second one reported the object it
  had just visited as a circular reference.
- A custom validator attribute with cyclic internal state made wrapped-rule
  detection recurse indefinitely.

Detect the rule anywhere inside a property's attributes rather than only at
the top level, so a wrapped Attributes suppresses implicit recursion as an
explicit one does. Make the rule immutable and give each recursion level its
own instance carrying the path to the object being evaluated, so visited
objects represent the path from the root rather than every object ever seen.
Collapse union recursion into a single Given over the disjunction of its
class members. Track the validators visited while inspecting a wrapped rule,
preventing a cyclic validator graph from overflowing the stack.

The detection only runs for properties whose type can hold an object, so
attributes with large arguments, such as #[In], no longer pay for it.

Author: alganet

docs/validators/Attributes.md

@@ -91,10 +91,18 @@ When a property's type is a class (named, union, or intersection type), `Attribu
 - **Untyped properties** (no type declaration, or builtin types like `string`): are never recursively validated.
 - **Array properties**: `Attributes` **does not** recursively validate objects inside arrays. To validate each element, use the `#[Each]` attribute on the property (e.g., `#[Each(new Attributes())]`).
 
+The implicit recursion is skipped whenever the property's own attributes already contain an `Attributes` rule, so the nested object is never validated twice. That holds for `#[Attributes]` written directly on the property and for an `Attributes` wrapped in another rule, like `#[NullOr(new Attributes())]`, `#[Each(new Attributes())]`, or `#[Given(new Instance(Address::class), new Attributes())]`. It holds even when the wrapper applies the rule to something other than the property itself, as in `#[Property('street', new Attributes())]`, so writing `Attributes` by hand always puts you in full control of what gets validated.
+
+Any other attribute on the property is combined with the implicit recursion instead of replacing it, so `#[Instance(Address::class)] public Address $address` still validates the nested object's own attributes.
+
 ### Circular references
 
 When a nested object graph contains a cycle (e.g., `$a->next = $b`, `$b->next = $a`), `Attributes` detects the revisit and fails with the `TEMPLATE_CIRCULAR_REFERENCE` template. This prevents infinite recursion and stack overflow.
 
+Detection is per *path*: only objects between the root and the property being validated count as a revisit. The same object reachable from two sibling properties (e.g., `$order->billing` and `$order->shipping` holding one `Address`) is not a cycle, and is validated on each path. The cost of validating an object graph is therefore proportional to how many distinct paths it has, not to how many objects it has, which is worth keeping in mind for graphs where many properties point at the same deeply nested objects.
+
+Detection covers the recursion `Attributes` performs on its own. An `Attributes` you write yourself in an attribute, such as `#[NullOr(new Attributes())]`, starts a traversal of its own and knows nothing about the objects already visited, so a cycle reached through it recurses infinitely.
+
 Note that circular reference detection only works for direct object references. If a cycle passes through an array (e.g., `$a->items = [$b]`, `$b->parent = $a`), `Attributes` cannot track the reference and the validation will recurse infinitely, causing a stack overflow.
 
 ## Templates

src/Validators/Attributes.php

@@ -19,6 +19,7 @@
 use ReflectionNamedType;
 use ReflectionObject;
 use ReflectionProperty;
+use ReflectionType;
 use ReflectionUnionType;
 use Respect\Fluent\Attributes\Composable;
 use Respect\Parameter\Resolver;
@@ -28,6 +29,8 @@
 use Respect\Validation\Validator;
 use Respect\Validation\Validators\Core\Reducer;
 
+use function count;
+use function is_array;
 use function spl_object_id;
 
 #[Composable(without: [All::class, Key::class, Property::class, Not::class, UndefOr::class])]
@@ -37,15 +40,16 @@
     '{{subject}} must contain a circular reference',
     Attributes::TEMPLATE_CIRCULAR_REFERENCE,
 )]
-final class Attributes implements Validator
+final readonly class Attributes implements Validator
 {
     public const string TEMPLATE_CIRCULAR_REFERENCE = '__circular_reference__';
 
     /** @var array<int, true> */
-    private array $visited = [];
+    private array $path;
 
-    public function __construct(private readonly Resolver|null $resolver = null)
+    public function __construct(private Resolver|null $resolver = null)
     {
+        $this->path = self::rootPath();
     }
 
     public function evaluate(mixed $input): Result
@@ -57,19 +61,24 @@ public function evaluate(mixed $input): Result
         }
 
         $objectId = spl_object_id($input);
-        if (isset($this->visited[$objectId])) {
+        if (isset($this->path[$objectId])) {
             return Result::failed($input, $this, [], self::TEMPLATE_CIRCULAR_REFERENCE)->withId($id);
         }
 
-        $this->visited[$objectId] = true;
+        $path = $this->path + [$objectId => true];
+        $child = clone ($this, ['path' => $path]);
 
         $reflection = new ReflectionObject($input);
-        $validators = [...$this->getClassValidators($reflection), ...$this->getPropertyValidators($reflection)];
-        if ($validators === []) {
-            return (new AlwaysValid())->evaluate($input)->withId($id);
-        }
+        $validators = [...$child->getClassValidators($reflection), ...$child->getPropertyValidators($reflection)];
+        $rule = $validators === [] ? new AlwaysValid() : new Reducer(...$validators);
+
+        return $rule->evaluate($input)->withId($id);
+    }
 
-        return (new Reducer(...$validators))->evaluate($input)->withId($id);
+    /** @return array<int, true> */
+    private static function rootPath(): array
+    {
+        return [];
     }
 
     /** @return array<Validator> */
@@ -110,46 +119,105 @@ private function getPropertyValidators(ReflectionObject $reflection): array
     private function getPropertyInnerValidators(ReflectionProperty $property): array
     {
         $propertyValidators = [];
-        $hasExplicitAttributes = false;
         foreach ($property->getAttributes(Validator::class, ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
             if ($attribute->getName() === self::class) {
-                $propertyValidator = $this;
-            } else {
-                $propertyValidator = $this->instantiateAttribute($attribute);
+                $propertyValidators[] = $this;
+
+                continue;
             }
 
-            $hasExplicitAttributes = $hasExplicitAttributes || $propertyValidator === $this;
-            $propertyValidators[] = $propertyValidator;
+            $propertyValidators[] = $this->instantiateAttribute($attribute);
         }
 
-        if ($hasExplicitAttributes) {
+        $recursion = $this->getRecursionValidator($property->getType());
+        if ($recursion === null) {
             return $propertyValidators;
         }
 
-        $type = $property->getType();
-        if ($type instanceof ReflectionNamedType) {
-            if (!$type->isBuiltin()) {
-                $propertyValidators[] = $this;
+        foreach ($propertyValidators as $propertyValidator) {
+            if (self::containsSelf($propertyValidator)) {
+                return $propertyValidators;
             }
         }
 
+        $propertyValidators[] = $recursion;
+
+        return $propertyValidators;
+    }
+
+    private function getRecursionValidator(ReflectionType|null $type): Validator|null
+    {
+        if ($type instanceof ReflectionNamedType) {
+            return $type->isBuiltin() ? null : $this;
+        }
+
         if ($type instanceof ReflectionIntersectionType) {
-            $propertyValidators[] = $this;
+            return $this;
         }
 
-        if ($type instanceof ReflectionUnionType) {
-            foreach ($type->getTypes() as $innerType) {
-                if (!$innerType instanceof ReflectionNamedType || $innerType->isBuiltin()) {
-                    continue;
+        if (!$type instanceof ReflectionUnionType) {
+            return null;
+        }
+
+        $instances = [];
+        foreach ($type->getTypes() as $innerType) {
+            if (!$innerType instanceof ReflectionNamedType || $innerType->isBuiltin()) {
+                continue;
+            }
+
+            /** @var class-string $class */
+            $class = $innerType->getName();
+            $instances[] = new Instance($class);
+        }
+
+        if ($instances === []) {
+            return null;
+        }
+
+        return new Given(count($instances) === 1 ? $instances[0] : new AnyOf(...$instances), $this);
+    }
+
+    private static function containsSelf(mixed $value): bool
+    {
+        $visited = [];
+
+        return self::containsSelfIn($value, $visited);
+    }
+
+    /** @param array<int, true> $visited */
+    private static function containsSelfIn(mixed $value, array &$visited): bool
+    {
+        if ($value instanceof self) {
+            return true;
+        }
+
+        if (is_array($value)) {
+            foreach ($value as $item) {
+                if (self::containsSelfIn($item, $visited)) {
+                    return true;
                 }
+            }
+
+            return false;
+        }
 
-                /** @var class-string $class */
-                $class = $innerType->getName();
-                $propertyValidators[] = new Given(new Instance($class), $this);
+        if (!$value instanceof Validator) {
+            return false;
+        }
+
+        $objectId = spl_object_id($value);
+        if (isset($visited[$objectId])) {
+            return false;
+        }
+
+        $visited[$objectId] = true;
+        foreach ((new ReflectionObject($value))->getProperties() as $property) {
+            if ($property->isInitialized($value) && self::containsSelfIn($property->getValue($value), $visited)) {
+                return true;
             }
         }
 
-        return $propertyValidators;
+        return false;
     }
 
     /** @return array<ReflectionProperty> */

tests/feature/Validators/AttributesTest.php

@@ -17,6 +17,7 @@
 use Respect\Validation\Test\Stubs\WithIntersectionTypeNested;
 use Respect\Validation\Test\Stubs\WithNestedAttributes;
 use Respect\Validation\Test\Stubs\WithUnionTypeNested;
+use Respect\Validation\Test\Stubs\WithWrappedAttributesOnNested;
 
 test('Default', catchAll(
     fn() => v::attributes()->assert(new WithAttributes('', '2024-06-23', 'john.doe@gmail.com')),
@@ -118,6 +119,14 @@
         ->and($messages)->toBe(['address' => '`.address.street` must be defined']),
 ));
 
+test('Recursive: wrapped Attributes on nested property is not duplicated', catchAll(
+    fn() => v::attributes()->assert(new WithWrappedAttributesOnNested(new NestedAddress('', 'Springfield'))),
+    fn(string $message, string $fullMessage, array $messages) => expect()
+        ->and($message)->toBe('`.address.street` must be defined')
+        ->and($fullMessage)->toBe('- `.address.street` must be defined')
+        ->and($messages)->toBe(['street' => '`.address.street` must be defined']),
+));
+
 test('Recursive: intersection type with invalid nested object property', catchAll(
     fn() => v::attributes()->assert(new WithIntersectionTypeNested('John Doe', new NestedWithAttributes('', 'Springfield'))),
     fn(string $message, string $fullMessage, array $messages) => expect()

tests/src/Stubs/CyclicValidator.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * SPDX-License-Identifier: MIT
+ * SPDX-FileCopyrightText: (c) Respect Project Contributors
+ * SPDX-FileContributor: Alexandre Gomes Gaigalas <alganet@gmail.com>
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+use Attribute;
+use Respect\Validation\Result;
+use Respect\Validation\Validator;
+use Respect\Validation\Validators\AlwaysValid;
+
+#[Attribute(Attribute::TARGET_PROPERTY)]
+final class CyclicValidator implements Validator
+{
+    /** @var array{self, string} */
+    private array $children;
+
+    public function __construct()
+    {
+        $this->children = [$this, 'cyclic'];
+    }
+
+    public function evaluate(mixed $input): Result
+    {
+        return (new AlwaysValid())->evaluate($input);
+    }
+}

tests/src/Stubs/WithCyclicValidator.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * SPDX-License-Identifier: MIT
+ * SPDX-FileCopyrightText: (c) Respect Project Contributors
+ * SPDX-FileContributor: Alexandre Gomes Gaigalas <alganet@gmail.com>
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+final class WithCyclicValidator
+{
+    public function __construct(
+        #[CyclicValidator]
+        public NestedAddress $address,
+    ) {
+    }
+}

tests/src/Stubs/WithOverlappingUnionTypeNested.php

@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * SPDX-License-Identifier: MIT
+ * SPDX-FileCopyrightText: (c) Respect Project Contributors
+ * SPDX-FileContributor: Alexandre Gomes Gaigalas <alganet@gmail.com>
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+final class WithOverlappingUnionTypeNested
+{
+    public function __construct(
+        public NestedWithAttributes|Nested $address,
+    ) {
+    }
+}

tests/src/Stubs/WithSharedNested.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * SPDX-License-Identifier: MIT
+ * SPDX-FileCopyrightText: (c) Respect Project Contributors
+ * SPDX-FileContributor: Alexandre Gomes Gaigalas <alganet@gmail.com>
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+final class WithSharedNested
+{
+    public function __construct(
+        public NestedAddress $billing,
+        public NestedAddress $shipping,
+    ) {
+    }
+}

tests/src/Stubs/WithWrappedAttributesOnNested.php

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * SPDX-License-Identifier: MIT
+ * SPDX-FileCopyrightText: (c) Respect Project Contributors
+ * SPDX-FileContributor: Alexandre Gomes Gaigalas <alganet@gmail.com>
+ */
+
+declare(strict_types=1);
+
+namespace Respect\Validation\Test\Stubs;
+
+use Respect\Validation\Validators as Rule;
+
+final class WithWrappedAttributesOnNested
+{
+    public function __construct(
+        #[Rule\NullOr(new Rule\Attributes())]
+        public NestedAddress|null $address = null,
+    ) {
+    }
+}