From 007bfd424ef94109d2270af5b0e65011705413db Mon Sep 17 00:00:00 2001 From: Julien Gidel Date: Sat, 20 Nov 2021 22:34:12 +0100 Subject: [PATCH] Small contribution - code readability improvements (#306) * Some code improvements - tests same as beginning * isUTF8Invalid returns a boolean * Changes for tests passed * Revert change + scrutinizer to flaky * Null coalescing on token to return a null token * Psalm pass and PHPunit too (php 7.4) * Minor change to set lookahead to nullToken --- .scrutinizer.yml | 2 +- src/EmailLexer.php | 174 +++++++++++--------------- src/MessageIDParser.php | 1 - src/Parser/DomainLiteral.php | 29 +++-- src/Parser/DoubleQuote.php | 9 +- src/Parser/FoldingWhiteSpace.php | 14 ++- src/Parser/IDRightPart.php | 4 +- src/Parser/LocalPart.php | 23 ++-- src/Validation/DNSCheckValidation.php | 47 +++---- 9 files changed, 143 insertions(+), 160 deletions(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 5671790..448af0c 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -7,7 +7,7 @@ build: tests: override: - - command: 'vendor/bin/phpunit --coverage-clover=clover.xml --exclude-group slow' + command: 'vendor/bin/phpunit --coverage-clover=clover.xml --exclude-group flaky' coverage: file: 'clover.xml' format: 'clover' diff --git a/src/EmailLexer.php b/src/EmailLexer.php index 41e9ea9..85e1984 100644 --- a/src/EmailLexer.php +++ b/src/EmailLexer.php @@ -61,7 +61,7 @@ class EmailLexer extends AbstractLexer * * @var array */ - protected $charValue = array( + protected $charValue = [ '{' => self::S_OPENCURLYBRACES, '}' => self::S_CLOSECURLYBRACES, '(' => self::S_OPENPARENTHESIS, @@ -105,11 +105,29 @@ class EmailLexer extends AbstractLexer '?' => self::QUESTIONMARK, '#' => self::NUMBER_SIGN, 'ยก' => self::INVERT_EXCLAMATION, - ); + ]; - /** - * @var bool - */ + const INVALID_CHARS_REGEX = "/[^\p{S}\p{C}\p{Cc}]+/iu"; + + const VALID_UTF8_REGEX = '/\p{Cc}+/u'; + + const CATCHABLE_PATTERNS = [ + '[a-zA-Z]+[46]?', //ASCII and domain literal + '[^\x00-\x7F]', //UTF-8 + '[0-9]+', + '\r\n', + '::', + '\s+?', + '.', + ]; + + const NON_CATCHABLE_PATTERNS = [ + '[\xA0-\xff]+', + ]; + + const MODIFIERS = 'iu'; + + /** @var bool */ protected $hasInvalidTokens = false; /** @@ -137,23 +155,17 @@ class EmailLexer extends AbstractLexer */ public $lookahead; - /** - * @psalm-var array{value:'', type:null, position:0} - */ + /** @psalm-var array{value:'', type:null, position:0} */ private static $nullToken = [ 'value' => '', 'type' => null, 'position' => 0, ]; - /** - * @var string - */ + /** @var string */ private $accumulator = ''; - /** - * @var bool - */ + /** @var bool */ private $hasToRecord = false; public function __construct() @@ -162,24 +174,13 @@ class EmailLexer extends AbstractLexer $this->lookahead = null; } - /** - * @return void - */ - public function reset() + public function reset() : void { $this->hasInvalidTokens = false; parent::reset(); $this->previous = $this->token = self::$nullToken; } - /** - * @return bool - */ - public function hasInvalidTokens() - { - return $this->hasInvalidTokens; - } - /** * @param int $type * @throws \UnexpectedValueException @@ -187,7 +188,7 @@ class EmailLexer extends AbstractLexer * * @psalm-suppress InvalidScalarArgument */ - public function find($type) + public function find($type) : bool { $search = clone $this; $search->skipUntil($type); @@ -198,30 +199,24 @@ class EmailLexer extends AbstractLexer return true; } - /** - * getPrevious - * - * @return array - */ - public function getPrevious() - { - return $this->previous; - } - /** * moveNext * * @return boolean */ - public function moveNext() + public function moveNext() : bool { if ($this->hasToRecord && $this->previous === self::$nullToken) { $this->accumulator .= $this->token['value']; } $this->previous = $this->token; + + if($this->lookahead === null) { + $this->lookahead = self::$nullToken; + } + $hasNext = parent::moveNext(); - $this->token = $this->token ?: self::$nullToken; if ($this->hasToRecord) { $this->accumulator .= $this->token['value']; @@ -230,36 +225,6 @@ class EmailLexer extends AbstractLexer return $hasNext; } - /** - * Lexical catchable patterns. - * - * @return string[] - */ - protected function getCatchablePatterns() - { - return array( - '[a-zA-Z]+[46]?', //ASCII and domain literal - '[^\x00-\x7F]', //UTF-8 - '[0-9]+', - '\r\n', - '::', - '\s+?', - '.', - ); - } - - /** - * Lexical non-catchable patterns. - * - * @return string[] - */ - protected function getNonCatchablePatterns() - { - return [ - '[\xA0-\xff]+', - ]; - } - /** * Retrieve token type. Also processes the token value if necessary. * @@ -292,51 +257,64 @@ class EmailLexer extends AbstractLexer return self::GENERIC; } - protected function isInvalidChar(string $value) : bool - { - if(preg_match("/[^\p{S}\p{C}\p{Cc}]+/iu", $value) ) { - return false; - } - return true; - } - protected function isValid(string $value) : bool { - if (isset($this->charValue[$value])) { - return true; - } - - return false; + return isset($this->charValue[$value]); } - /** - * @param string $value - * @return bool - */ - protected function isNullType($value) + protected function isNullType(string $value) : bool { - if ($value === "\0") { - return true; - } + return $value === "\0"; + } - return false; + protected function isInvalidChar(string $value) : bool + { + return !preg_match(self::INVALID_CHARS_REGEX, $value); } protected function isUTF8Invalid(string $value) : bool { - if (preg_match('/\p{Cc}+/u', $value)) { - return true; - } + return preg_match(self::VALID_UTF8_REGEX, $value) !== false; + } - return false; + public function hasInvalidTokens() : bool + { + return $this->hasInvalidTokens; } /** - * @return string + * getPrevious + * + * @return array */ - protected function getModifiers() + public function getPrevious() : array { - return 'iu'; + return $this->previous; + } + + /** + * Lexical catchable patterns. + * + * @return string[] + */ + protected function getCatchablePatterns() : array + { + return self::CATCHABLE_PATTERNS; + } + + /** + * Lexical non-catchable patterns. + * + * @return string[] + */ + protected function getNonCatchablePatterns() : array + { + return self::NON_CATCHABLE_PATTERNS; + } + + protected function getModifiers() : string + { + return self::MODIFIERS; } public function getAccumulatedValues() : string diff --git a/src/MessageIDParser.php b/src/MessageIDParser.php index 9b029e1..fe58b9f 100644 --- a/src/MessageIDParser.php +++ b/src/MessageIDParser.php @@ -3,7 +3,6 @@ namespace Egulias\EmailValidator; use Egulias\EmailValidator\Parser; -use Egulias\EmailValidator\EmailLexer; use Egulias\EmailValidator\Result\Result; use Egulias\EmailValidator\Parser\IDLeftPart; use Egulias\EmailValidator\Parser\IDRightPart; diff --git a/src/Parser/DomainLiteral.php b/src/Parser/DomainLiteral.php index 54a6fab..769ac58 100644 --- a/src/Parser/DomainLiteral.php +++ b/src/Parser/DomainLiteral.php @@ -22,6 +22,15 @@ use Egulias\EmailValidator\Warning\DomainLiteral as WarningDomainLiteral; class DomainLiteral extends PartParser { + const IPV4_REGEX = '/\\b(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/'; + + const OBSOLETE_WARNINGS = [ + EmailLexer::INVALID, + EmailLexer::C_DEL, + EmailLexer::S_LF, + EmailLexer::S_BACKSLASH + ]; + public function parse() : Result { $this->addTagWarnings(); @@ -138,11 +147,8 @@ class DomainLiteral extends PartParser public function convertIPv4ToIPv6(string $addressLiteralIPv4) : string { - $matchesIP = array(); - $IPv4Match = preg_match( - '/\\b(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/', - $addressLiteralIPv4, - $matchesIP); + $matchesIP = []; + $IPv4Match = preg_match(self::IPV4_REGEX, $addressLiteralIPv4, $matchesIP); // Extract IPv4 part from the end of the address-literal (if there is one) if ($IPv4Match > 0) { @@ -164,11 +170,8 @@ class DomainLiteral extends PartParser */ protected function checkIPV4Tag($addressLiteral) : bool { - $matchesIP = array(); - $IPv4Match = preg_match( - '/\\b(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/', - $addressLiteral, - $matchesIP); + $matchesIP = []; + $IPv4Match = preg_match(self::IPV4_REGEX, $addressLiteral, $matchesIP); // Extract IPv4 part from the end of the address-literal (if there is one) @@ -186,11 +189,7 @@ class DomainLiteral extends PartParser private function addObsoleteWarnings() : void { - if ($this->lexer->token['type'] === EmailLexer::INVALID || - $this->lexer->token['type'] === EmailLexer::C_DEL || - $this->lexer->token['type'] === EmailLexer::S_LF || - $this->lexer->token['type'] === EmailLexer::S_BACKSLASH - ) { + if(in_array($this->lexer->token['type'], self::OBSOLETE_WARNINGS)) { $this->warnings[ObsoleteDTEXT::CODE] = new ObsoleteDTEXT(); } } diff --git a/src/Parser/DoubleQuote.php b/src/Parser/DoubleQuote.php index 19c098e..8b83273 100644 --- a/src/Parser/DoubleQuote.php +++ b/src/Parser/DoubleQuote.php @@ -19,18 +19,19 @@ class DoubleQuote extends PartParser $validQuotedString = $this->checkDQUOTE(); if($validQuotedString->isInvalid()) return $validQuotedString; - $special = array( + $special = [ EmailLexer::S_CR => true, EmailLexer::S_HTAB => true, EmailLexer::S_LF => true - ); + ]; - $invalid = array( + $invalid = [ EmailLexer::C_NUL => true, EmailLexer::S_HTAB => true, EmailLexer::S_CR => true, EmailLexer::S_LF => true - ); + ]; + $setSpecialsWarning = true; $this->lexer->moveNext(); diff --git a/src/Parser/FoldingWhiteSpace.php b/src/Parser/FoldingWhiteSpace.php index d32231e..da17d43 100644 --- a/src/Parser/FoldingWhiteSpace.php +++ b/src/Parser/FoldingWhiteSpace.php @@ -15,6 +15,14 @@ use Egulias\EmailValidator\Result\ValidEmail; class FoldingWhiteSpace extends PartParser { + const FWS_TYPES = [ + EmailLexer::S_SP, + EmailLexer::S_HTAB, + EmailLexer::S_CR, + EmailLexer::S_LF, + EmailLexer::CRLF + ]; + public function parse() : Result { if (!$this->isFWS()) { @@ -73,10 +81,6 @@ class FoldingWhiteSpace extends PartParser return false; } - return $this->lexer->token['type'] === EmailLexer::S_SP || - $this->lexer->token['type'] === EmailLexer::S_HTAB || - $this->lexer->token['type'] === EmailLexer::S_CR || - $this->lexer->token['type'] === EmailLexer::S_LF || - $this->lexer->token['type'] === EmailLexer::CRLF; + return in_array($this->lexer->token['type'], self::FWS_TYPES); } } \ No newline at end of file diff --git a/src/Parser/IDRightPart.php b/src/Parser/IDRightPart.php index bcf80dd..2432562 100644 --- a/src/Parser/IDRightPart.php +++ b/src/Parser/IDRightPart.php @@ -12,14 +12,14 @@ class IDRightPart extends DomainPart { protected function validateTokens(bool $hasComments) : Result { - $invalidDomainTokens = array( + $invalidDomainTokens = [ EmailLexer::S_DQUOTE => true, EmailLexer::S_SQUOTE => true, EmailLexer::S_BACKTICK => true, EmailLexer::S_SEMICOLON => true, EmailLexer::S_GREATERTHAN => true, EmailLexer::S_LOWERTHAN => true, - ); + ]; if (isset($invalidDomainTokens[$this->lexer->token['type']])) { return new InvalidEmail(new ExpectingATEXT('Invalid token in domain: ' . $this->lexer->token['value']), $this->lexer->token['value']); diff --git a/src/Parser/LocalPart.php b/src/Parser/LocalPart.php index d505c61..90f01a5 100644 --- a/src/Parser/LocalPart.php +++ b/src/Parser/LocalPart.php @@ -15,6 +15,17 @@ use Egulias\EmailValidator\Parser\CommentStrategy\LocalComment; class LocalPart extends PartParser { + const INVALID_TOKENS = [ + EmailLexer::S_COMMA => EmailLexer::S_COMMA, + EmailLexer::S_CLOSEBRACKET => EmailLexer::S_CLOSEBRACKET, + EmailLexer::S_OPENBRACKET => EmailLexer::S_OPENBRACKET, + EmailLexer::S_GREATERTHAN => EmailLexer::S_GREATERTHAN, + EmailLexer::S_LOWERTHAN => EmailLexer::S_LOWERTHAN, + EmailLexer::S_COLON => EmailLexer::S_COLON, + EmailLexer::S_SEMICOLON => EmailLexer::S_SEMICOLON, + EmailLexer::INVALID => EmailLexer::INVALID + ]; + /** * @var string */ @@ -88,17 +99,7 @@ class LocalPart extends PartParser protected function validateTokens(bool $hasComments) : Result { - $invalidTokens = array( - EmailLexer::S_COMMA => EmailLexer::S_COMMA, - EmailLexer::S_CLOSEBRACKET => EmailLexer::S_CLOSEBRACKET, - EmailLexer::S_OPENBRACKET => EmailLexer::S_OPENBRACKET, - EmailLexer::S_GREATERTHAN => EmailLexer::S_GREATERTHAN, - EmailLexer::S_LOWERTHAN => EmailLexer::S_LOWERTHAN, - EmailLexer::S_COLON => EmailLexer::S_COLON, - EmailLexer::S_SEMICOLON => EmailLexer::S_SEMICOLON, - EmailLexer::INVALID => EmailLexer::INVALID - ); - if (isset($invalidTokens[$this->lexer->token['type']])) { + if (isset(self::INVALID_TOKENS[$this->lexer->token['type']])) { return new InvalidEmail(new ExpectingATEXT('Invalid token found'), $this->lexer->token['value']); } return new ValidEmail(); diff --git a/src/Validation/DNSCheckValidation.php b/src/Validation/DNSCheckValidation.php index a935727..dd5c24a 100644 --- a/src/Validation/DNSCheckValidation.php +++ b/src/Validation/DNSCheckValidation.php @@ -17,6 +17,29 @@ class DNSCheckValidation implements EmailValidation */ protected const DNS_RECORD_TYPES_TO_CHECK = DNS_MX + DNS_A + DNS_AAAA; + /** + * Reserved Top Level DNS Names (https://tools.ietf.org/html/rfc2606#section-2), + * mDNS and private DNS Namespaces (https://tools.ietf.org/html/rfc6762#appendix-G) + */ + const RESERVED_DNS_TOP_LEVEL_NAMES = [ + // Reserved Top Level DNS Names + 'test', + 'example', + 'invalid', + 'localhost', + + // mDNS + 'local', + + // Private DNS Namespaces + 'intranet', + 'internal', + 'private', + 'corp', + 'home', + 'lan', + ]; + /** * @var array */ @@ -32,7 +55,6 @@ class DNSCheckValidation implements EmailValidation */ private $mxRecords = []; - public function __construct() { if (!function_exists('idn_to_ascii')) { @@ -53,29 +75,8 @@ class DNSCheckValidation implements EmailValidation // Get the domain parts $hostParts = explode('.', $host); - // Reserved Top Level DNS Names (https://tools.ietf.org/html/rfc2606#section-2), - // mDNS and private DNS Namespaces (https://tools.ietf.org/html/rfc6762#appendix-G) - $reservedTopLevelDnsNames = [ - // Reserved Top Level DNS Names - 'test', - 'example', - 'invalid', - 'localhost', - - // mDNS - 'local', - - // Private DNS Namespaces - 'intranet', - 'internal', - 'private', - 'corp', - 'home', - 'lan', - ]; - $isLocalDomain = count($hostParts) <= 1; - $isReservedTopLevel = in_array($hostParts[(count($hostParts) - 1)], $reservedTopLevelDnsNames, true); + $isReservedTopLevel = in_array($hostParts[(count($hostParts) - 1)], self::RESERVED_DNS_TOP_LEVEL_NAMES, true); // Exclude reserved top level DNS names if ($isLocalDomain || $isReservedTopLevel) {