Rfeactor property rules and add a failing test

spatie · Feb 9, 2024 · e954107 · e954107
1 parent c34bbea
commit e954107
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 16 deletions.
diff --git a/src/Support/Validation/PropertyRules.php b/src/Support/Validation/PropertyRules.php
@@ -2,16 +2,16 @@
 
 namespace Spatie\LaravelData\Support\Validation;
 
-use Illuminate\Support\Collection;
+use Illuminate\Support\Arr;
 
 class PropertyRules
 {
-    /** @var Collection<\Spatie\LaravelData\Support\Validation\ValidationRule> */
-    protected Collection $rules;
-
-    public function __construct()
-    {
-        $this->rules = new Collection();
+    /**
+     * @param array<int, ValidationRule> $rules
+     */
+    public function __construct(
+        protected array $rules = []
+    ) {
     }
 
     public static function create(ValidationRule ...$rules): self
@@ -22,45 +22,55 @@ public static function create(ValidationRule ...$rules): self
     public function add(ValidationRule ...$rules): static
     {
         $this->removeType(...$rules);
-        $this->rules->push(...$rules);
+
+        array_push($this->rules, ...$rules);
 
         return $this;
     }
 
     public function prepend(ValidationRule ...$rules): static
     {
         $this->removeType(...$rules);
-        $this->rules->prepend(...$rules);
+
+        $this->rules = Arr::prepend($this->rules, ...$rules);
 
         return $this;
     }
 
     public function removeType(string|ValidationRule ...$classes): static
     {
-        $this->rules = $this->rules->reject(function (ValidationRule $rule) use ($classes) {
+        foreach ($this->rules as $i => $rule) {
             foreach ($classes as $class) {
                 if ($class instanceof RequiringRule && $rule instanceof RequiringRule) {
-                    return true;
+                    unset($this->rules[$i]);
+                    continue 2;
                 }
 
                 if ($rule instanceof $class) {
-                    return true;
+                    unset($this->rules[$i]);
+                    continue 2;
                 }
             }
+        }
 
-            return false;
-        })->values();
+        $this->rules = array_values($this->rules);
 
         return $this;
     }
 
     public function hasType(string $class): bool
     {
-        return $this->rules->contains(fn (ValidationRule $rule) => $rule instanceof $class);
+        foreach ($this->rules as $rule) {
+            if ($rule instanceof $class) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     public function all(): array
     {
-        return $this->rules->all();
+        return $this->rules;
     }
 }
diff --git a/tests/DataTest.php b/tests/DataTest.php
@@ -2,6 +2,9 @@
 
 use Illuminate\Contracts\Support\Responsable;
 use Illuminate\Validation\ValidationException;
+use Spatie\LaravelData\Attributes\DataCollectionOf;
+use Spatie\LaravelData\Attributes\Validation\RequiredUnless;
+use Spatie\LaravelData\Attributes\Validation\RequiredWith;
 use Spatie\LaravelData\Concerns\AppendableData;
 use Spatie\LaravelData\Concerns\BaseData;
 use Spatie\LaravelData\Concerns\ContextableData;

diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php
@@ -14,6 +14,7 @@
 use Illuminate\Validation\ValidationException;
 use Illuminate\Validation\Validator;
 
+use Spatie\LaravelData\Attributes\Validation\RequiredUnless;
 use function Pest\Laravel\mock;
 use function PHPUnit\Framework\assertFalse;
 
@@ -275,6 +276,19 @@ public static function rules(): array
     ]);
 });
 
+it('is possible to have multiple required rules', function () {
+    DataValidationAsserter::for(new class () extends Data {
+        #[RequiredUnless('is_required', false), RequiredWith('make_required')]
+        public string $property;
+        public string $make_required;
+        public bool $is_required;
+    })->assertRules([
+        'property' => ['string', 'required_unless:is_required', 'required_with:make_required'],
+        'make_required' => ['required', 'string'],
+        'is_required' => ['boolean'],
+    ]);
+})->skip('Add a new ruleinferrer to rule them all and make these cases better');
+
 it('it will take care of mapping', function () {
     DataValidationAsserter::for(new class () extends Data {
         #[MapInputName('some_property')]