Ver código fonte

Support invokable rules correctly (closes #629), support Laravel 10 ValidationRule

shalvah 2 anos atrás
pai
commit
ea341869b4

+ 1 - 1
.github/ISSUE_TEMPLATE/bug_report.md

@@ -9,7 +9,7 @@ assignees: ''
 
 !!IMPORTANT!!
 
-READ BEFORE OPENING AN ISSUE:
+READ BEFORE OPENING AN ISSUE (and then DELETE before writing your issue):
 
 - If you don't provide enough information in your issue for me to reproduce (such as version and any custom configuration), I will close it. I can't keep wasting my time investigating issues which no longer exist.
 - Check the Troubleshooting Guide! https://scribe.knuckles.wtf/laravel/troubleshooting Some solutions to common problems are detailed there.

+ 1 - 1
composer.json

@@ -39,7 +39,7 @@
         "brianium/paratest": "^6.0",
         "dms/phpunit-arraysubset-asserts": "^0.4",
         "laravel/legacy-factories": "^1.3.0",
-        "laravel/lumen-framework": "^8.0|^9.0",
+        "laravel/lumen-framework": "^8.0|^9.0|^10.0",
         "league/fractal": "^0.20",
         "nikic/fast-route": "^1.3",
         "orchestra/testbench": "^6.0|^7.0|^8.0",

+ 38 - 23
src/Extracting/ParsesValidationRules.php

@@ -2,8 +2,8 @@
 
 namespace Knuckles\Scribe\Extracting;
 
-use Illuminate\Contracts\Validation\InvokableRule;
 use Illuminate\Contracts\Validation\Rule;
+use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
@@ -207,7 +207,13 @@ trait ParsesValidationRules
             return true;
         }
 
-        if ($rule instanceof Rule || $rule instanceof InvokableRule) {
+        if ($rule instanceof Rule || $rule instanceof ValidationRule) {
+            if (method_exists($rule, 'invokable')) {
+                // Laravel wraps InvokableRule instances in an InvokableValidationRule class,
+                // so we must retrieve the original rule
+                $rule = $rule->invokable();
+            }
+
             if (method_exists($rule, 'docs')) {
                 $customData = call_user_func_array([$rule, 'docs'], []) ?: [];
 
@@ -469,31 +475,40 @@ trait ParsesValidationRules
                     $parameterData['description'] .= ' Must not be one of ' . w::getListOfValuesAsFriendlyHtmlString($arguments) . ' ';
                     break;
                 case 'required_if':
-                    $parameterData['description'] .= ' ' . $this->getDescription(
-                            $rule, [':other' => "<code>{$arguments[0]}</code>", ':value' => w::getListOfValuesAsFriendlyHtmlString(array_slice($arguments, 1))]
-                        ) . ' ';
+                    $parameterData['description'] .= sprintf(
+                        " This field is required when <code>{$arguments[0]}</code> is %s. ",
+                        w::getListOfValuesAsFriendlyHtmlString(array_slice($arguments, 1))
+                    );
                     break;
                 case 'required_unless':
-                    $parameterData['description'] .= ' ' . $this->getDescription(
-                            $rule, [':other' => "<code>" . array_shift($arguments) . "</code>", ':values' => w::getListOfValuesAsFriendlyHtmlString($arguments)]
-                        ) . ' ';
+                    $parameterData['description'] .= sprintf(
+                        " This field is required unless <code>{$arguments[0]}</code> is in %s. ",
+                        w::getListOfValuesAsFriendlyHtmlString(array_slice($arguments, 1))
+                    );
                     break;
                 case 'required_with':
-                    $parameterData['description'] .= ' ' . $this->getDescription(
-                            $rule, [':values' => w::getListOfValuesAsFriendlyHtmlString($arguments)]
-                        ) . ' ';
+                    $parameterData['description'] .= sprintf(
+                        " This field is required when %s is present. ",
+                        w::getListOfValuesAsFriendlyHtmlString($arguments)
+                    );
                     break;
                 case 'required_without':
-                    $description = $this->getDescription(
-                            $rule, [':values' => w::getListOfValuesAsFriendlyHtmlString($arguments)]
-                        ) . ' ';
-                    $parameterData['description'] .= str_replace('must be present', 'is not present', $description);
+                    $parameterData['description'] .= sprintf(
+                        " This field is required when %s is not present. ",
+                        w::getListOfValuesAsFriendlyHtmlString($arguments)
+                    );
                     break;
                 case 'required_with_all':
+                    $parameterData['description'] .= sprintf(
+                        " This field is required when %s are present. ",
+                        w::getListOfValuesAsFriendlyHtmlString($arguments, "and")
+                    );
+                    break;
                 case 'required_without_all':
-                    $parameterData['description'] .= ' ' . $this->getDescription(
-                            $rule, [':values' => w::getListOfValuesAsFriendlyHtmlString($arguments, "and")]
-                        ) . ' ';
+                    $parameterData['description'] .= sprintf(
+                    " This field is required when none of %s are present. ",
+                    w::getListOfValuesAsFriendlyHtmlString($arguments, "and")
+                    );
                     break;
                 case 'accepted_if':
                     $parameterData['type'] = 'boolean';
@@ -501,10 +516,10 @@ trait ParsesValidationRules
                     $parameterData['setter'] = fn() => true;
                     break;
                 case 'same':
+                    $parameterData['description'] .= " The value and <code>{$arguments[0]}</code> must match.";
+                    break;
                 case 'different':
-                    $parameterData['description'] .= ' ' . $this->getDescription(
-                            $rule, [':other' => "<code>{$arguments[0]}</code>"]
-                        ) . ' ';
+                    $parameterData['description'] .= " The value and <code>{$arguments[0]}</code> must be different.";
                     break;
 
                 default:
@@ -769,8 +784,8 @@ trait ParsesValidationRules
             $description = str_replace($placeholder, $argument, $description);
         }
 
-        // For rules that validate subfields
-        $description = str_replace("The :attribute field ", "This field ", $description);
+        // Laravel 10 added `field` to its messages: https://github.com/laravel/framework/pull/45974
+        $description = str_replace("The :attribute field ", "The value ", $description);
 
         $description = preg_replace("/(?!<\W):attribute\b/", "value", $description);
 

+ 58 - 4
tests/Unit/ValidationRuleParsingTest.php

@@ -11,6 +11,9 @@ use Knuckles\Scribe\Tests\BaseLaravelTest;
 use Knuckles\Scribe\Tools\DocumentationConfig;
 use Knuckles\Scribe\Tests\Fixtures;
 
+$invokableRulesSupported = interface_exists(\Illuminate\Contracts\Validation\InvokableRule::class);
+$laravel10Rules = version_compare(Application::VERSION, '10.0', '>=');
+
 class ValidationRuleParsingTest extends BaseLaravelTest
 {
     private $strategy;
@@ -499,13 +502,23 @@ class ValidationRuleParsingTest extends BaseLaravelTest
     public function can_parse_custom_rule_classes()
     {
         $ruleset = [
-            // The page number. Example: 1
-            'custom_rule' => ['bail', 'required', new DummyWithDocsValidationRule],
+            'param1' => ['bail', 'required', new DummyWithDocsValidationRule],
         ];
 
+        global $invokableRulesSupported;
+        if ($invokableRulesSupported) {
+            $ruleset['param2'] = [new DummyInvokableValidationRule];
+        }
+        global $laravel10Rules;
+        if ($laravel10Rules) {
+            $ruleset['param3'] = [new DummyL10ValidationRule];
+        }
+
         $results = $this->strategy->parse($ruleset);
-        $this->assertEquals(true, $results['custom_rule']['required']);
-        $this->assertEquals('This is a dummy test rule.', $results['custom_rule']['description']);
+        $this->assertEquals(true, $results['param1']['required']);
+        $this->assertEquals('This is a dummy test rule.', $results['param1']['description']);
+        if (isset($results['param2'])) $this->assertEquals('This rule is invokable.', $results['param2']['description']);
+       if (isset($results['param3'])) $this->assertEquals('This is a custom rule.', $results['param3']['description']);
     }
 
     /** @test */
@@ -577,3 +590,44 @@ class DummyWithDocsValidationRule implements \Illuminate\Contracts\Validation\Ru
         ];
     }
 }
+
+if ($invokableRulesSupported) {
+    class DummyInvokableValidationRule implements \Illuminate\Contracts\Validation\InvokableRule
+    {
+        public function __invoke($attribute, $value, $fail)
+        {
+            if (strtoupper($value) !== $value) {
+                $fail(':attribute must be uppercase.');
+            }
+        }
+
+        public function docs()
+        {
+
+            return [
+                'description' => 'This rule is invokable.',
+            ];
+        }
+    }
+}
+
+if ($laravel10Rules) {
+// Laravel 10 deprecated the previous Rule and InvokableRule classes for a single interface
+    // (https://github.com/laravel/framework/pull/45954)
+    class DummyL10ValidationRule implements \Illuminate\Contracts\Validation\ValidationRule
+    {
+        public function validate(string $attribute, mixed $value, \Closure $fail): void
+        {
+            if (strtoupper($value) !== $value) {
+                $fail('The :attribute must be an attribute.');
+            }
+        }
+
+        public static function docs()
+        {
+            return [
+                'description' => 'This is a custom rule.',
+            ];
+        }
+    }
+}