Type fixes (#49)

* refactor: allowed modifiers to optionally return a modified rule or not * docs: typed `excludeIf` correctly * test: added explicit check of both modifier flows * docs: fixed dimensions typehint * test: switch to gate facade
sourcetoad · Nov 5, 2024 · 7aebad8 · 7aebad8
1 parent 02c06f8
commit 7aebad8
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 31 deletions.
diff --git a/src/Rule.php b/src/Rule.php
@@ -339,7 +339,7 @@ public static function digitsBetween(int $min, int $max): string
      * like *3/2* or a float like *1.5*.
      *
      * @link https://laravel.com/docs/11.x/validation#rule-dimensions
-     * @param array<string, int|float> $constraints
+     * @param array<string, int|float|string> $constraints
      */
     public static function dimensions(array $constraints = []): Dimensions
     {

diff --git a/src/RuleSet.php b/src/RuleSet.php
@@ -404,15 +404,15 @@ public function digitsBetween(int $min, int $max): self
      * pass a callback which accepts a {@see \Illuminate\Validation\Rules\Dimensions} instance.
      *
      * @link https://laravel.com/docs/11.x/validation#rule-dimensions
-     * @param array<string, int|float> $constraints
-     * @param ?callable(\Illuminate\Validation\Rules\Dimensions): void $modifier
+     * @param array<string, int|float|string> $constraints
+     * @param ?callable(\Illuminate\Validation\Rules\Dimensions): (\Illuminate\Validation\Rules\Dimensions|void) $modifier
      */
     public function dimensions(array $constraints = [], ?callable $modifier = null): self
     {
         $rule = Rule::dimensions($constraints);
 
         if ($modifier) {
-            $modifier($rule);
+            $rule = $this->modify($rule, $modifier);
         }
 
         return $this->rule($rule);
@@ -473,14 +473,14 @@ public function endsWith(string ...$value): self
      *
      * @link https://laravel.com/docs/11.x/validation#rule-enum
      * @param class-string $type
-     * @param ?callable(\Illuminate\Validation\Rules\Enum): void $modifier
+     * @param ?callable(\Illuminate\Validation\Rules\Enum): (\Illuminate\Validation\Rules\Enum|void) $modifier
      */
     public function enum(string $type, ?callable $modifier = null): self
     {
         $rule = Rule::enum($type);
 
         if ($modifier) {
-            $modifier($rule);
+            $rule = $this->modify($rule, $modifier);
         }
 
         return $this->rule($rule);
@@ -502,7 +502,7 @@ public function exclude(): self
      * methods if a true boolean is passed in or the passed in closure returns true.
      *
      * @link https://laravel.com/docs/11.x/validation#rule-exclude-if
-     * @param callable|bool $callback
+     * @param bool|callable(): bool $callback
      */
     public function excludeIf(mixed $callback): self
     {
@@ -562,14 +562,14 @@ public function excludeWithout(string $anotherField): self
      * {@see RuleSet::rule} or pass a callback which accepts an {@see \Illuminate\Validation\Rules\Exists} instance.
      *
      * @link https://laravel.com/docs/11.x/validation#rule-exists
-     * @param ?callable(\Illuminate\Validation\Rules\Exists): void $modifier
+     * @param ?callable(\Illuminate\Validation\Rules\Exists): (\Illuminate\Validation\Rules\Exists|void) $modifier
      */
     public function exists(string $table, string $column = 'NULL', ?callable $modifier = null): self
     {
         $rule = Rule::exists($table, $column);
 
         if ($modifier) {
-            $modifier($rule);
+            $rule = $this->modify($rule, $modifier);
         }
 
         return $this->rule($rule);
@@ -951,14 +951,14 @@ public function numeric(): self
      * use {@see Rule::password} with {@see RuleSet::rule}, or pass a callback which accepts a {@see Password} instance.
      *
      * @link https://laravel.com/docs/11.x/validation#validating-passwords
-     * @param ?callable(\Illuminate\Validation\Rules\Password): void $modifier
+     * @param ?callable(\Illuminate\Validation\Rules\Password): (\Illuminate\Validation\Rules\Password|void) $modifier
      */
     public function password(?int $size = null, ?callable $modifier = null): self
     {
         $rule = Rule::password($size);
 
         if ($modifier) {
-            $modifier($rule);
+            $rule = $this->modify($rule, $modifier);
         }
 
         return $this->rule($rule);
@@ -1294,14 +1294,14 @@ public function ulid(): self
      * {@see RuleSet::rule} or pass a callback which accepts a {@see \Illuminate\Validation\Rules\Unique} instance.
      *
      * @link https://laravel.com/docs/11.x/validation#rule-unique
-     * @param ?callable(\Illuminate\Validation\Rules\Unique): void $modifier
+     * @param ?callable(\Illuminate\Validation\Rules\Unique): (\Illuminate\Validation\Rules\Unique|void) $modifier
      */
     public function unique(string $table, string $column = 'NULL', ?callable $modifier = null): self
     {
         $rule = Rule::unique($table, $column);
 
         if ($modifier) {
-            $modifier($rule);
+            $rule = $this->modify($rule, $modifier);
         }
 
         return $this->rule($rule);
@@ -1353,4 +1353,16 @@ protected static function getDefinedRuleSets(): Contracts\DefinedRuleSets
     {
         return resolve(Contracts\DefinedRuleSets::class);
     }
+
+    /**
+     * @param T $rule
+     * @param callable(T): (T|void) $modifier
+     * @return T
+     * @template T
+     */
+    protected function modify($rule, callable $modifier)
+    {
+        /** @var T */
+        return $modifier($rule) ?: $rule;
+    }
 }
diff --git a/tests/Unit/DatabaseRuleTest.php b/tests/Unit/DatabaseRuleTest.php
@@ -9,6 +9,7 @@
 use Illuminate\Foundation\Testing\WithFaker;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Validator;
+use Illuminate\Validation\Rule as LaravelRule;
 use Illuminate\Validation\Rules\Exists;
 use Illuminate\Validation\Rules\Unique;
 use PHPUnit\Framework\Attributes\DataProvider;
@@ -204,6 +205,51 @@ public static function databaseSetupProvider(): array
                 ],
                 'fails' => true,
             ],
+            'not unique with modifier that does not return' => [
+                'createData' => function () {
+                    $email = $this->faker->email;
+                    DB::table('users')->insert([
+                        'name' => 'modified',
+                        'email' => $email,
+                        'password' => $this->faker->password,
+                    ]);
+
+                    return ['value' => $email];
+                },
+                'createRules' => fn() => [
+                    'value' => RuleSet::create()->unique(
+                        'users',
+                        'email',
+                        function (Unique $rule) {
+                            $rule->where('name', 'modified');
+                        }
+                    ),
+                ],
+                'fails' => true,
+            ],
+            'not unique with modifier that overwrites' => [
+                'createData' => function () {
+                    $email = $this->faker->email;
+                    DB::table('users')->insert([
+                        'name' => 'overwritten',
+                        'email' => $email,
+                        'password' => $this->faker->password,
+                    ]);
+
+                    return ['value' => $email];
+                },
+                'createRules' => fn() => [
+                    'value' => RuleSet::create()->unique(
+                        'invalid_users',
+                        'email',
+                        function () {
+                            return LaravelRule::unique('users', 'email')
+                                ->where('name', 'overwritten');
+                        }
+                    ),
+                ],
+                'fails' => true,
+            ],
             'unique with modifier' => [
                 'createData' => function () {
                     $email = $this->faker->email;

diff --git a/tests/Unit/RuleTest.php b/tests/Unit/RuleTest.php
@@ -9,12 +9,12 @@
 use Closure;
 use DateTime;
 use Illuminate\Auth\AuthManager;
-use Illuminate\Contracts\Auth\Access\Gate;
 use Illuminate\Contracts\Auth\Guard;
 use Illuminate\Contracts\Hashing\Hasher;
 use Illuminate\Contracts\Validation\InvokableRule;
 use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Foundation\Auth\User;
+use Illuminate\Support\Facades\Gate;
 use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
 use Illuminate\Validation\Rules\Dimensions;
@@ -40,7 +40,7 @@ protected function setUp(): void
     }
 
     #[DataProvider('ruleDataProvider')]
-    public function testRuleIntegration($data, Closure $rules, bool $fails, ?array $errors = null): void
+    public function testRuleIntegration(mixed $data, Closure $rules, bool $fails, ?array $errors = null): void
     {
         // Arrange
         if ($data instanceof Closure) {
@@ -478,7 +478,11 @@ public static function ruleDataProvider(): array
                 'data' => 'value-a',
                 'rules' => function () {
                     $mockUser = new User;
-                    $this->mockGateAllows(true, 'modify', [User::class, $mockUser, 'value-a']);
+
+                    Gate::expects('allows')
+                        ->once()
+                        ->with('modify', [User::class, $mockUser, 'value-a'])
+                        ->andReturn(true);
 
                     return RuleSet::create()->can('modify', User::class, $mockUser);
                 },
@@ -488,7 +492,11 @@ public static function ruleDataProvider(): array
                 'data' => 'value-b',
                 'rules' => function () {
                     $mockUser = new User;
-                    $this->mockGateAllows(false, 'modify', [User::class, $mockUser, 'value-b']);
+
+                    Gate::expects('allows')
+                        ->once()
+                        ->with('modify', [User::class, $mockUser, 'value-b'])
+                        ->andReturn(false);
 
                     return RuleSet::create()->can('modify', User::class, $mockUser);
                 },
@@ -3114,18 +3122,4 @@ public function getClientOriginalExtension(): string
             }
         };
     }
-
-    private function mockGateAllows(bool $return, ...$arguments): void
-    {
-        $gate = $this->mock(Gate::class);
-
-        $gate
-            ->expects('allows')
-            ->once()
-            ->with(...$arguments)
-            ->andReturn($return)
-            ->getMock();
-
-        $this->app->instance(Gate::class, $gate);
-    }
 }