Fix constructor breaking with format as input #102
Conversation
| $options = array_merge($format, $options); | ||
| } elseif (null !== $format) { | ||
| $options['value'] = $format; | ||
| } |
There was a problem hiding this comment.
This looks indeed somehow wrong. Maybe it's done on purpose? cc @jderusse
Asking because it's a bc break if it may be useful in a case.
There was a problem hiding this comment.
It kinda looks to me to be some BC code to some REALLY old SF versions (2.x maybe).
I've gone through the code trying to find any use for it but could not (tests passing without it also suggests it wasn't used anywhere).
But yeah some extra confirmation might be good
| @@ -54,12 +54,6 @@ class PhoneNumber extends Constraint | |||
| */ | |||
| public function __construct($format = null, $type = null, string $defaultRegion = null, string $regionPath = null, string $message = null, array $groups = null, $payload = null, array $options = []) | |||
| { | |||
| if (\is_array($format)) { | |||
| $options = array_merge($format, $options); | |||
| } elseif (null !== $format) { | |||
There was a problem hiding this comment.
- The first
ifseems to be (unfortunately) an important BC break. We are going to keep it besides it's not great. - The $format var is therefore not
string|array|nullbutint|array|null. - I really think the
$options['value']part is to drop because thegetDefaultOptionis not defined. - We should trigger a deprecation if an options array is passed instead of the format.
- Finally we should add some doc to the docbloc to make the usage more clear and maybe rename the format
$formatOrOptions.
edit: my bad, I was able to do it.
This is related to odolbeau#102 that remove the part that I add here.
This is related to odolbeau#102 that remove the part that I add here.
|
I squashed your work. Added the part I was talking about in my last comment. PHPStan failures are unrelated. (seems to be an issue from the image we use) Everything is ready to be merged, except this failure 😬 . |
Fixes #101
Also using the constructor in tests so that doesn't happen again