Skip to content

Commit

Permalink
Improve Phone validation
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@henriquemoody
henriquemoody committed Mar 25, 2024
1 parent 92b196e commit ae369c4
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 55 deletions.
44 changes: 22 additions & 22 deletions library/Rules/Phone.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@

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',
Expand All @@ -30,7 +31,7 @@
'{{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__';
Expand Down Expand Up @@ -63,35 +64,34 @@ public function __construct(?string $countryCode = null, ?Countries $countries =
$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);
}

try {
return PhoneNumberUtil::getInstance()->isValidNumber(
PhoneNumberUtil::getInstance()->parse((string) $input, $this->country?->getAlpha2())
);
} catch (NumberParseException $e) {
return false;
}
return new Result($this->isValidPhone((string) $input), $input, $this, $parameters, $template);
}

/**
* @return array<string, mixed>
*/
public function getParams(): array
private function isValidPhone(string $input): bool
{
return ['countryName' => $this->country?->getName()];
}
try {
$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) {
}

protected function getStandardTemplate(mixed $input): string
{
return $this->country ? self::TEMPLATE_FOR_COUNTRY : self::TEMPLATE_INTERNATIONAL;
return false;
}
}
50 changes: 44 additions & 6 deletions tests/integration/rules/phone.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]
130 changes: 103 additions & 27 deletions tests/unit/Rules/PhoneTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,55 +10,131 @@
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');
self::assertValidInput(new Phone(), $input);
}

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('"abc" must be a valid telephone number for country Brazil');
#[Test]
#[DataProvider('providerForInvalidInputWithoutCountryCode')]
public function shouldValidateInvalidInputWithoutCountryCode(mixed $input): void
{
self::assertInvalidInput(new Phone(), $input);
}

$phoneValidator->assert('abc');
#[Test]
#[DataProvider('providerForValidInputWithCountryCode')]
public function shouldValidateValidInputWithCountryCode(string $countryCode, mixed $input): void
{
self::assertValidInput(new Phone($countryCode), $input);
}

public function testThrowsExceptionForInternationalNumbers(): void
#[Test]
#[DataProvider('providerForInvalidInputWithCountryCode')]
public function shouldValidateInvalidInputWithCountryCode(string $countryCode, mixed $input): void
{
$phoneValidator = new Phone();
self::assertInvalidInput(new Phone($countryCode), $input);
}

$this->expectException(ValidationException::class);
$this->expectExceptionMessage('"abc" must be a valid telephone number');
/** @return array<array{mixed}> */
public static function providerForValidInputWithoutCountryCode(): array
{
return [
['+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'],
];
}

$phoneValidator->assert('abc');
/** @return array<array{mixed}> */
public static function providerForInvalidInputWithoutCountryCode(): array
{
return [
['+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 iterable<array{Phone, mixed}> */
public static function providerForValidInput(): iterable
/** @return array<array{string, mixed}> */
public static function providerForValidInputWithCountryCode(): 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
['BR', '+55 11 91111 1111'],
['BR', '11 91111 1111'],
['BR', '+5511911111111'],
['BR', '11911111111'],
['NL', '+31 10 408 1775'],
];
}

/** @return iterable<array{Phone, mixed}> */
public static function providerForInvalidInput(): iterable
/** @return array<array{string, mixed}> */
public static function providerForInvalidInputWithCountryCode(): array
{
return [
[new Phone(), '+1-650-253-00-0'],
[new Phone('BR'), '+1 11 91111 1111'], // invalid + code for BR
['BR', '+1 11 91111 1111'],
['BR', '+1 650 253 00 00'],
['US', '+31 10 408 1775'],
];
}
}

0 comments on commit ae369c4

Please sign in to comment.