-
Notifications
You must be signed in to change notification settings - Fork 11.5k
[12.x] Feat: add validation rule builders for all rules #56405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 12.x
Are you sure you want to change the base?
Changes from all commits
326a7f3
bc6cec1
a325245
6678996
04ce509
2464874
e963dba
0405fb3
560a4e1
30c2137
de1c150
c167ec2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Accepted implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'accepted'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class AcceptedIf implements Stringable | ||
{ | ||
public function __construct( | ||
protected string $anotherField, | ||
protected string|null|int|float $value, | ||
) { | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'accepted_if:'.$this->anotherField.','.($this->value ?? 'null'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class ActiveUrl implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'active_url'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Alpha implements Stringable | ||
{ | ||
protected bool $isAscii = false; | ||
|
||
public function ascii(): static | ||
{ | ||
$this->isAscii = true; | ||
|
||
return $this; | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'alpha'.($this->isAscii ? ':ascii' : ''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice if we had a base rule where we could just pass the rule and its arguments and it would generate the string for us There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't do it, I think isn't necessary. Like this, I feel it simple and straightforward, and maybe even easier to expand if it's in its own class. But I could be wrong. I'm open to make changes obviously. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class AlphaDash implements Stringable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, these rules should inherit from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree there are duplicated classes, but after thinking about it before the PR, I felt that since they're pretty simple and have few properties/methods, it's just easier to follow if we keep them like that. Do you think using inheritance would make it as equally readable and easier to extend as is now? If so, help me develop it. How would you approach it? |
||
{ | ||
protected bool $isAscii = false; | ||
|
||
public function ascii(): static | ||
{ | ||
$this->isAscii = true; | ||
|
||
return $this; | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'alpha_dash'.($this->isAscii ? ':ascii' : ''); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class AlphaNum implements Stringable | ||
{ | ||
protected bool $isAscii = false; | ||
|
||
public function ascii(): static | ||
{ | ||
$this->isAscii = true; | ||
|
||
return $this; | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'alpha_num'.($this->isAscii ? ':ascii' : ''); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
|
||
use function Illuminate\Support\enum_value; | ||
|
||
class ArrayRule implements Stringable | ||
class Arr implements Stringable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure, this isn't a breaking change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say it isn't a breaking change since I checked it and it is only being used in the I renamed it from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation—that makes sense 👍🏻 Just wanted to make sure :) |
||
{ | ||
/** | ||
* The accepted keys. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Ascii implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'ascii'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Bail implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'bail'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,35 @@ | ||||||||||||||
<?php | ||||||||||||||
|
||||||||||||||
namespace Illuminate\Validation\Rules; | ||||||||||||||
|
||||||||||||||
use InvalidArgumentException; | ||||||||||||||
use Stringable; | ||||||||||||||
|
||||||||||||||
class Between implements Stringable | ||||||||||||||
{ | ||||||||||||||
protected int|float $min; | ||||||||||||||
protected int|float $max; | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Create a new rule instance. | ||||||||||||||
* | ||||||||||||||
* @param int|float $min | ||||||||||||||
* @param int|float $max | ||||||||||||||
* | ||||||||||||||
* @throws InvalidArgumentException | ||||||||||||||
*/ | ||||||||||||||
public function __construct(int|float $min, int|float $max) | ||||||||||||||
{ | ||||||||||||||
if ($min > $max) { | ||||||||||||||
throw new InvalidArgumentException('The min value cannot be greater than the max value.'); | ||||||||||||||
} | ||||||||||||||
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about this?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO I think it's better to throw an error than silently swapping the values. This could cause unwanted successful validations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would that happen? Is there any use case for explicitly wanting to validate an impossible range? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the point is throwing the exception so the user knows that the min value cannot be greater than the max. If we just swap the values the user won't see an exception so won't see that something is wrong! I think we need to prevent invalid states from being processed unnoticed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would agree with you if the values were ambiguous. However, in this case, we know exactly what the user wanted. So, throwing an exception here is entirely unnecessary. |
||||||||||||||
|
||||||||||||||
$this->min = $min; | ||||||||||||||
$this->max = $max; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public function __toString(): string | ||||||||||||||
{ | ||||||||||||||
return "between:{$this->min},{$this->max}"; | ||||||||||||||
} | ||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Boolean implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'boolean'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Confirmed implements Stringable | ||
{ | ||
protected ?string $confirmationField = null; | ||
|
||
public function customField(string $confirmationField) | ||
{ | ||
$this->confirmationField = $confirmationField; | ||
|
||
return $this; | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'confirmed'.($this->confirmationField ? ":{$this->confirmationField}" : ''); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class CurrentPassword implements Stringable | ||
{ | ||
protected ?string $guard = null; | ||
|
||
public function guard(string $guard): static | ||
{ | ||
$this->guard = $guard; | ||
|
||
return $this; | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'current_password'.($this->guard ? ":{$this->guard}" : ''); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use InvalidArgumentException; | ||
use Stringable; | ||
|
||
class Decimal implements Stringable | ||
{ | ||
public function __construct( | ||
protected int $minPlaces, | ||
protected ?int $maxPlaces, | ||
) { | ||
if (! is_null($this->maxPlaces) && $this->minPlaces > $this->maxPlaces) { | ||
throw new InvalidArgumentException('The min places value cannot be greater than the max places value.'); | ||
} | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
if (is_null($this->maxPlaces)) { | ||
return "decimal:{$this->minPlaces}"; | ||
} | ||
|
||
return "decimal:{$this->minPlaces},{$this->maxPlaces}"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Declined implements Stringable | ||
{ | ||
public function __toString(): string | ||
{ | ||
return 'declined'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class DeclinedIf implements Stringable | ||
{ | ||
public function __construct( | ||
protected string $anotherField, | ||
protected string|null|int|float $value | ||
) { | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return 'declined_if:'.$this->anotherField.','.($this->value ?? 'null'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Different implements Stringable | ||
{ | ||
public function __construct(protected string $field) | ||
{ | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return "different:{$this->field}"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class Digits implements Stringable | ||
{ | ||
public function __construct(protected int $length) | ||
{ | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return "digits:{$this->length}"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
namespace Illuminate\Validation\Rules; | ||
|
||
use Stringable; | ||
|
||
class DigitsBetween implements Stringable | ||
{ | ||
public function __construct( | ||
protected int $min, | ||
protected int $max, | ||
) { | ||
} | ||
|
||
public function __toString(): string | ||
{ | ||
return "digits_between:{$this->min},{$this->max}"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason, why
$this->ascii
cannot be set via the constructor like in other rules?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a solid reason, basically I felt that
Rule::alpha()->ascii()
is more verbose thanRule::alpha(true)
.I know you can do it like this:
Rule::alpha(ascii: true)
, but the chained way forces you to read it. Instead, in this way -Rule::alpha(true)
, is not mandatory to specify the parameter name, and is less readable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, but that begs the question why you allow this in other rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there are rules that need their parameters to be defined it when constructing the object. But there are others, like the one we are looking at, the Alpha one, where is optional, therefore makes more sense to chain it after creating the object than in the constructor. I think it's fine this way, don't you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, can't agree. There are other places in the framework which don't throw exceptions when they should, but not here.