Fix an issue where required rules could not be combined with optional (…

…#844)
spatie · Oct 4, 2024 · c620967 · c620967
1 parent cf86d7a
commit c620967
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/src/RuleInferrers/AttributesRuleInferrer.php b/src/RuleInferrers/AttributesRuleInferrer.php
@@ -3,6 +3,7 @@
 namespace Spatie\LaravelData\RuleInferrers;
 
 use Spatie\LaravelData\Attributes\Validation\Present;
+use Spatie\LaravelData\Attributes\Validation\Sometimes;
 use Spatie\LaravelData\Support\DataProperty;
 use Spatie\LaravelData\Support\Validation\PropertyRules;
 use Spatie\LaravelData\Support\Validation\RequiringRule;
@@ -29,6 +30,10 @@ public function handle(
                     $rules->removeType(RequiringRule::class);
                 }
 
+                if ($rule instanceof RequiringRule && $rules->hasType(Sometimes::class)) {
+                    $rules->removeType(Sometimes::class);
+                }
+
                 $rules->add(
                     ...$this->rulesDenormalizer->execute($rule)
                 );

diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php
@@ -1082,18 +1082,26 @@ class CollectionClassC extends Data
         ->assertRules(
             [
                 'someData' => [
-                    'sometimes',
                     'array',
                     'required_without:someOtherData',
                 ],
                 'someOtherData' => [
-                    'sometimes',
                     'array',
                     'required_without:someData',
                 ],
             ],
-            []
-        );
+        )
+        ->assertOk([
+            'someData' => [["string" => "Hello World"]],
+        ])
+        ->assertOk([
+            'someOtherData' => [["string" => "Hello World"]],
+        ])
+        ->assertOk([
+            'someData' => [["string" => "Hello World"]],
+            'someOtherData' => [["string" => "Hello World"]],
+        ])
+        ->assertErrors([]);
 });
 
 it('supports required without validation for nullable collections', function () {
@@ -2429,3 +2437,28 @@ public function __construct(
         'some_property' => 1,
     ]);
 })->skip('Validation problem, fix in v5');
+
+it('will remove a sometimes rule generated by an Optional type when manually requiring something', function () {
+    $dataClass = new class () extends Data {
+        #[RequiredWith('otherProperty')]
+        public string|Optional $property;
+
+        public string|null $otherProperty;
+    };
+
+    DataValidationAsserter::for($dataClass)
+        ->assertRules([
+            'property' => ['string', 'required_with:otherProperty'],
+            'otherProperty' => ['nullable', 'string'],
+        ], ['property' => 'Hello World', 'otherProperty' => 'Hello World'])
+        ->assertOk([
+            'property' => 'Hello World',
+            'otherProperty' => 'Hello World',
+        ])
+        ->assertOk([
+            'otherProperty' => null,
+        ])
+        ->assertErrors([
+            'otherProperty' => 'Hello World',
+        ]);
+});