Improve Phone validation

This commit will improve the Phone rule in the following ways:

* Upgrade its validation engine;

* Increase the number of tests;

* Do not validate phone numbers from other regions.

The last item is a possible bug with "libphonenumber-for-php", which I
have already reported.

Signed-off-by: Henrique Moody <henriquemoody@gmail.com>

library/Rules/AbstractAge.php

@@ -10,6 +10,8 @@ declare(strict_types=1);
 namespace Respect\Validation\Rules;
 
 use Respect\Validation\Helpers\CanValidateDateTime;
+use Respect\Validation\Result;
+use Respect\Validation\Rules\Core\Standard;
 
 use function date;
 use function date_parse_from_format;
@@ -17,7 +19,7 @@ use function is_scalar;
 use function strtotime;
 use function vsprintf;
 
-abstract class AbstractAge extends AbstractRule
+abstract class AbstractAge extends Standard
 {
     use CanValidateDateTime;
 
@@ -32,25 +34,18 @@ abstract class AbstractAge extends AbstractRule
         $this->baseDate = (int) date('Ymd') - $this->age * 10000;
     }
 
-    public function validate(mixed $input): bool
+    public function evaluate(mixed $input): Result
     {
+        $parameters = ['age' => $this->age];
         if (!is_scalar($input)) {
-            return false;
+            return Result::failed($input, $this, $parameters);
         }
 
         if ($this->format === null) {
-            return $this->isValidWithoutFormat((string) $input);
+            return new Result($this->isValidWithoutFormat((string) $input), $input, $this, $parameters);
         }
 
-        return $this->isValidWithFormat($this->format, (string) $input);
-    }
-
-    /**
-     * @return array<string, int>
-     */
-    public function getParams(): array
-    {
-        return ['age' => $this->age];
+        return new Result($this->isValidWithFormat($this->format, (string) $input), $input, $this, $parameters);
     }
 
     private function isValidWithoutFormat(string $dateTime): bool

library/Rules/Phone.php

@@ -11,14 +11,15 @@ namespace Respect\Validation\Rules;
 
 use libphonenumber\NumberParseException;
 use libphonenumber\PhoneNumberUtil;
-use Respect\Validation\Exceptions\ComponentException;
+use Respect\Validation\Exceptions\InvalidRuleConstructorException;
 use Respect\Validation\Exceptions\MissingComposerDependencyException;
 use Respect\Validation\Message\Template;
+use Respect\Validation\Result;
+use Respect\Validation\Rules\Core\Standard;
 use Sokil\IsoCodes\Database\Countries;
 
 use function class_exists;
 use function is_scalar;
-use function sprintf;
 
 #[Template(
     '{{name}} must be a valid telephone number',
@@ -30,7 +31,7 @@ use function sprintf;
     '{{name}} must not be a valid telephone number for country {{countryName|trans}}',
     self::TEMPLATE_FOR_COUNTRY,
 )]
-final class Phone extends AbstractRule
+final class Phone extends Standard
 {
     public const TEMPLATE_FOR_COUNTRY = '__for_country__';
     public const TEMPLATE_INTERNATIONAL = '__international__';
@@ -63,35 +64,34 @@ final class Phone extends AbstractRule
         $countries ??= new Countries();
         $this->country = $countries->getByAlpha2($countryCode);
         if ($this->country === null) {
-            throw new ComponentException(sprintf('Invalid country code %s', $countryCode));
+            throw new InvalidRuleConstructorException('Invalid country code %s', $countryCode);
         }
     }
 
-    public function validate(mixed $input): bool
+    public function evaluate(mixed $input): Result
     {
+        $parameters = ['countryName' => $this->country?->getName()];
+        $template = $this->country === null ? self::TEMPLATE_INTERNATIONAL : self::TEMPLATE_FOR_COUNTRY;
         if (!is_scalar($input)) {
-            return false;
+            return Result::failed($input, $this, $parameters, $template);
         }
 
+        return new Result($this->isValidPhone((string) $input), $input, $this, $parameters, $template);
+    }
+
+    private function isValidPhone(string $input): bool
+    {
         try {
-            return PhoneNumberUtil::getInstance()->isValidNumber(
-                PhoneNumberUtil::getInstance()->parse((string) $input, $this->country?->getAlpha2())
-            );
-        } catch (NumberParseException $e) {
-            return false;
+            $phoneNumberUtil = PhoneNumberUtil::getInstance();
+            $phoneNumberObject = $phoneNumberUtil->parse($input, $this->country?->getAlpha2());
+            if ($this->country === null) {
+                return $phoneNumberUtil->isValidNumber($phoneNumberObject);
+            }
+
+            return $phoneNumberUtil->getRegionCodeForNumber($phoneNumberObject) === $this->country->getAlpha2();
+        } catch (NumberParseException) {
         }
-    }
 
-    /**
-     * @return array<string, mixed>
-     */
-    public function getParams(): array
-    {
-        return ['countryName' => $this->country?->getName()];
-    }
-
-    protected function getStandardTemplate(mixed $input): string
-    {
-        return $this->country ? self::TEMPLATE_FOR_COUNTRY : self::TEMPLATE_INTERNATIONAL;
+        return false;
     }
 }

library/Rules/Size.php

@@ -13,6 +13,8 @@ use Psr\Http\Message\StreamInterface;
 use Psr\Http\Message\UploadedFileInterface;
 use Respect\Validation\Exceptions\ComponentException;
 use Respect\Validation\Message\Template;
+use Respect\Validation\Result;
+use Respect\Validation\Rules\Core\Standard;
 use SplFileInfo;
 
 use function filesize;
@@ -37,7 +39,7 @@ use function sprintf;
     '{{name}} must not be lower than {{maxSize}}',
     self::TEMPLATE_GREATER,
 )]
-final class Size extends AbstractRule
+final class Size extends Standard
 {
     public const TEMPLATE_LOWER = '__lower__';
     public const TEMPLATE_GREATER = '__greater__';
@@ -55,7 +57,18 @@ final class Size extends AbstractRule
         $this->maxValue = $maxSize ? $this->toBytes((string) $maxSize) : null;
     }
 
-    public function validate(mixed $input): bool
+    public function evaluate(mixed $input): Result
+    {
+        return new Result(
+            $this->isValid($input),
+            $input,
+            $this,
+            ['minSize' => $this->minSize, 'maxSize' => $this->maxSize],
+            $this->getStandardTemplate()
+        );
+    }
+
+    private function isValid(mixed $input): bool
     {
         if ($input instanceof SplFileInfo) {
             return $this->isValidSize((float) $input->getSize());
@@ -76,18 +89,7 @@ final class Size extends AbstractRule
         return false;
     }
 
-    /**
-     * @return array<string, mixed>
-     */
-    public function getParams(): array
-    {
-        return [
-            'minSize' => $this->minSize,
-            'maxSize' => $this->maxSize,
-        ];
-    }
-
-    protected function getStandardTemplate(mixed $input): string
+    private function getStandardTemplate(): string
     {
         if (!$this->minValue) {
             return self::TEMPLATE_GREATER;

tests/integration/rules/phone.phpt

@@ -7,13 +7,51 @@ require 'vendor/autoload.php';
 
 use Respect\Validation\Validator as v;
 
-exceptionMessage(static fn() => v::phone()->check('123'));
-exceptionMessage(static fn() => v::not(v::phone())->check('+1 650 253 00 00'));
-exceptionFullMessage(static fn() => v::phone()->assert('(555)5555 555'));
-exceptionFullMessage(static fn() => v::not(v::phone())->assert('+55 11 91111 1111'));
+run([
+    'Default' => [v::phone(), '123'],
+    'Country-specific' => [v::phone('BR'), '+1 650 253 00 00'],
+    'Negative' => [v::not(v::phone()), '+55 11 91111 1111'],
+    'Default with name' => [v::phone()->setName('Phone'), '123'],
+    'Country-specific with name' => [v::phone('US')->setName('Phone'), '123'],
+]);
 ?>
 --EXPECT--
+Default
+⎺⎺⎺⎺⎺⎺⎺
 "123" must be a valid telephone number
-"+1 650 253 00 00" must not be a valid telephone number
-- "(555)5555 555" must be a valid telephone number
+- "123" must be a valid telephone number
+[
+    'phone' => '"123" must be a valid telephone number',
+]
+
+Country-specific
+⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
+"+1 650 253 00 00" must be a valid telephone number for country Brazil
+- "+1 650 253 00 00" must be a valid telephone number for country Brazil
+[
+    'phone' => '"+1 650 253 00 00" must be a valid telephone number for country Brazil',
+]
+
+Negative
+⎺⎺⎺⎺⎺⎺⎺⎺
+"+55 11 91111 1111" must not be a valid telephone number
 - "+55 11 91111 1111" must not be a valid telephone number
+[
+    'phone' => '"+55 11 91111 1111" must not be a valid telephone number',
+]
+
+Default with name
+⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
+Phone must be a valid telephone number
+- Phone must be a valid telephone number
+[
+    'Phone' => 'Phone must be a valid telephone number',
+]
+
+Country-specific with name
+⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺
+Phone must be a valid telephone number for country United States
+- Phone must be a valid telephone number for country United States
+[
+    'Phone' => 'Phone must be a valid telephone number for country United States',
+]

tests/unit/Rules/PhoneTest.php

@@ -10,55 +10,131 @@ declare(strict_types=1);
 namespace Respect\Validation\Rules;
 
 use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
-use Respect\Validation\Exceptions\ValidationException;
-use Respect\Validation\Test\RuleTestCase;
+use PHPUnit\Framework\Attributes\Test;
+use Respect\Validation\Test\TestCase;
 
 #[Group('rule')]
 #[CoversClass(Phone::class)]
-final class PhoneTest extends RuleTestCase
+final class PhoneTest extends TestCase
 {
-    public function testThrowsExceptionWithCountryName(): void
+    #[Test]
+    #[DataProvider('providerForValidInputWithoutCountryCode')]
+    public function shouldValidateValidInputWithoutCountryCode(mixed $input): void
     {
-        $phoneValidator = new Phone('BR');
-
-        $this->expectException(ValidationException::class);
-        $this->expectExceptionMessage('"abc" must be a valid telephone number for country Brazil');
-
-        $phoneValidator->assert('abc');
+        self::assertValidInput(new Phone(), $input);
     }
 
-    public function testThrowsExceptionForInternationalNumbers(): void
+    #[Test]
+    #[DataProvider('providerForInvalidInputWithoutCountryCode')]
+    public function shouldValidateInvalidInputWithoutCountryCode(mixed $input): void
     {
-        $phoneValidator = new Phone();
-
-        $this->expectException(ValidationException::class);
-        $this->expectExceptionMessage('"abc" must be a valid telephone number');
-
-        $phoneValidator->assert('abc');
+        self::assertInvalidInput(new Phone(), $input);
     }
 
-    /** @return iterable<array{Phone, mixed}> */
-    public static function providerForValidInput(): iterable
+    #[Test]
+    #[DataProvider('providerForValidInputWithCountryCode')]
+    public function shouldValidateValidInputWithCountryCode(string $countryCode, mixed $input): void
+    {
+        self::assertValidInput(new Phone($countryCode), $input);
+    }
+
+    #[Test]
+    #[DataProvider('providerForInvalidInputWithCountryCode')]
+    public function shouldValidateInvalidInputWithCountryCode(string $countryCode, mixed $input): void
+    {
+        self::assertInvalidInput(new Phone($countryCode), $input);
+    }
+
+    /** @return array<array{mixed}> */
+    public static function providerForValidInputWithoutCountryCode(): array
     {
         return [
-            [new Phone(), '+1 650 253 00 00'],
-            [new Phone(), '+7 (999) 999-99-99'],
-            [new Phone(), '+7(999)999-99-99'],
-            [new Phone(), '+7(999)999-9999'],
-            [new Phone('BR'), '+55 11 91111 1111'],
-            [new Phone('BR'), '11 91111 1111'], // no international prefix
-            [new Phone('BR'), '+5511911111111'], // no whitespace
-            [new Phone('BR'), '11911111111'], // no prefix, no whitespace
+            ['+1 650 253 00 00'],
+            ['+7 (999) 999-99-99'],
+            ['+7(999)999-99-99'],
+            ['+7(999)999-9999'],
+            ['+33(1)22 22 22 22'],
+            ['+1 650 253 00 00'],
+            ['+7 (999) 999-99-99'],
+            ['+7(999)999-99-99'],
+            ['+7(999)999-9999'],
         ];
     }
 
-    /** @return iterable<array{Phone, mixed}> */
-    public static function providerForInvalidInput(): iterable
+    /** @return array<array{mixed}> */
+    public static function providerForInvalidInputWithoutCountryCode(): array
     {
         return [
-            [new Phone(), '+1-650-253-00-0'],
-            [new Phone('BR'), '+1 11 91111 1111'], // invalid + code for BR
+            ['+1-650-253-00-0'],
+            ['33(020) 7777 7777'],
+            ['33(020)7777 7777'],
+            ['+33(020) 7777 7777'],
+            ['+33(020)7777 7777'],
+            ['03-6106666'],
+            ['036106666'],
+            ['+33(11) 97777 7777'],
+            ['+3311977777777'],
+            ['11977777777'],
+            ['11 97777 7777'],
+            ['(11) 97777 7777'],
+            ['(11) 97777-7777'],
+            ['555-5555'],
+            ['5555555'],
+            ['555.5555'],
+            ['555 5555'],
+            ['+1 (555) 555 5555'],
+            ['33(1)2222222'],
+            ['33(1)22222222'],
+            ['33(1)22 22 22 22'],
+            ['(020) 7476 4026'],
+            ['+5-555-555-5555'],
+            ['+5 555 555 5555'],
+            ['+5.555.555.5555'],
+            ['5-555-555-5555'],
+            ['5.555.555.5555'],
+            ['5 555 555 5555'],
+            ['555.555.5555'],
+            ['555 555 5555'],
+            ['555-555-5555'],
+            ['555-5555555'],
+            ['5(555)555.5555'],
+            ['+5(555)555.5555'],
+            ['+5(555)555 5555'],
+            ['+5(555)555-5555'],
+            ['+5(555)5555555'],
+            ['(555)5555555'],
+            ['(555)555.5555'],
+            ['(555)555-5555'],
+            ['(555) 555 5555'],
+            ['55555555555'],
+            ['5555555555'],
+            ['+33(1)2222222'],
+            ['+33(1)222 2222'],
+            ['+33(1)222.2222'],
+        ];
+    }
+
+    /** @return array<array{string, mixed}> */
+    public static function providerForValidInputWithCountryCode(): array
+    {
+        return [
+            ['BR', '+55 11 91111 1111'],
+            ['BR', '11 91111 1111'],
+            ['BR', '+5511911111111'],
+            ['BR', '11911111111'],
+            ['NL', '+31 10 408 1775'],
+        ];
+    }
+
+    /** @return array<array{string, mixed}> */
+    public static function providerForInvalidInputWithCountryCode(): array
+    {
+        return [
+            ['BR', '+1 11 91111 1111'],
+            ['BR', '+1 650 253 00 00'],
+            ['US', '+31 10 408 1775'],
         ];
     }
 }