From 6558c831ae742547dab08c2b44def19b242f7478 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 28 Jan 2021 11:47:28 +0000 Subject: [PATCH 1/7] Code quality improvements --- composer.json | 2 +- src/DefaultValidationRules.php | 2 +- src/ErrorList.php | 8 +++++--- src/Rule/Pattern.php | 2 +- src/Rule/Required.php | 2 +- src/Rule/Rule.php | 3 ++- src/Rule/TypeDate.php | 2 +- src/Rule/TypeEmail.php | 2 +- src/Rule/TypeNumber.php | 6 +++++- src/Rule/TypeUrl.php | 2 +- src/ValidationRules.php | 8 ++++---- src/Validator.php | 13 ++++--------- 12 files changed, 27 insertions(+), 25 deletions(-) diff --git a/composer.json b/composer.json index 91ecdf8..de9b19c 100644 --- a/composer.json +++ b/composer.json @@ -9,7 +9,7 @@ "type": "library", "require": { - "php": ">=7.2", + "php": ">=7.4", "ext-dom": "*", "phpgt/cssxpath": "1.*" }, diff --git a/src/DefaultValidationRules.php b/src/DefaultValidationRules.php index d2f7531..3ef5312 100644 --- a/src/DefaultValidationRules.php +++ b/src/DefaultValidationRules.php @@ -10,7 +10,7 @@ use Gt\DomValidation\Rule\TypeUrl; class DefaultValidationRules extends ValidationRules { - protected function setRuleList() { + protected function setRuleList():void { $this->ruleList = [ new Required(), new Pattern(), diff --git a/src/ErrorList.php b/src/ErrorList.php index bcfdfae..140644f 100644 --- a/src/ErrorList.php +++ b/src/ErrorList.php @@ -5,9 +5,11 @@ use Countable; use Iterator; +/** @implements Iterator */ class ErrorList implements Countable, Iterator { - protected $errorArray; - protected $iteratorKey; + /** @var array */ + protected array $errorArray; + protected int $iteratorKey; public function __construct() { $this->errorArray = []; @@ -20,7 +22,7 @@ public function add(DOMElement $element, string $errorMessage):void { $this->errorArray[$name] = []; } - $this->errorArray[$name] []= $errorMessage; + array_push($this->errorArray[$name], $errorMessage); } public function count():int { diff --git a/src/Rule/Pattern.php b/src/Rule/Pattern.php index bd96e06..9d2a179 100644 --- a/src/Rule/Pattern.php +++ b/src/Rule/Pattern.php @@ -4,7 +4,7 @@ use DOMElement; class Pattern extends Rule { - protected $attributes = [ + protected array $attributes = [ "pattern", ]; diff --git a/src/Rule/Required.php b/src/Rule/Required.php index e43a796..3fea74b 100644 --- a/src/Rule/Required.php +++ b/src/Rule/Required.php @@ -4,7 +4,7 @@ use DOMElement; class Required extends Rule { - protected $attributes = [ + protected array $attributes = [ "required", ]; diff --git a/src/Rule/Rule.php b/src/Rule/Rule.php index 5516bcc..d665e51 100644 --- a/src/Rule/Rule.php +++ b/src/Rule/Rule.php @@ -10,10 +10,11 @@ abstract class Rule { * equals sign (e.g. "type=email"). For attributes without a value, pass the * attribute name on its own (e.g. "required"). */ - protected $attributes = [ + protected array $attributes = [ "name" ]; + /** @return string[] */ public function getAttributes():array { return $this->attributes; } diff --git a/src/Rule/TypeDate.php b/src/Rule/TypeDate.php index b5219d1..61b9d1f 100644 --- a/src/Rule/TypeDate.php +++ b/src/Rule/TypeDate.php @@ -12,7 +12,7 @@ class TypeDate extends Rule { const FORMAT_DATETIME_LOCAL = "Y-m-d\TH:i"; const FORMAT_TIME = "H:i"; - protected $attributes = [ + protected array $attributes = [ "type=date", "type=month", "type=week", diff --git a/src/Rule/TypeEmail.php b/src/Rule/TypeEmail.php index 7a53dca..b57630d 100644 --- a/src/Rule/TypeEmail.php +++ b/src/Rule/TypeEmail.php @@ -4,7 +4,7 @@ use DOMElement; class TypeEmail extends Rule { - protected $attributes = [ + protected array $attributes = [ "type=email", ]; diff --git a/src/Rule/TypeNumber.php b/src/Rule/TypeNumber.php index 19448bf..f8be53f 100644 --- a/src/Rule/TypeNumber.php +++ b/src/Rule/TypeNumber.php @@ -4,7 +4,7 @@ use DOMElement; class TypeNumber extends Rule { - protected $attributes = [ + protected array $attributes = [ "type=number", "type=range", ]; @@ -22,6 +22,8 @@ public function isValid(DOMElement $element, string $value):bool { return false; } + $value = (float)$value; + if($min !== "" && $value < $min) { return false; @@ -52,6 +54,8 @@ public function getHint(DOMElement $element, string $value):string { return "Field must be a number"; } + $value = (float)$value; + if($min !== "" && $value < $min) { return "Field value must not be less than $min"; diff --git a/src/Rule/TypeUrl.php b/src/Rule/TypeUrl.php index 074e20e..4351e23 100644 --- a/src/Rule/TypeUrl.php +++ b/src/Rule/TypeUrl.php @@ -4,7 +4,7 @@ use DOMElement; class TypeUrl extends Rule { - protected $attributes = [ + protected array $attributes = [ "type=url", ]; diff --git a/src/ValidationRules.php b/src/ValidationRules.php index bef33bd..4affd2a 100644 --- a/src/ValidationRules.php +++ b/src/ValidationRules.php @@ -5,21 +5,21 @@ abstract class ValidationRules { /** @var Rule[] */ - protected $ruleList; + protected array $ruleList; public function __construct() { $this->setRuleList(); } /** Must instantiate $this->ruleList as an array of Rule objects */ - abstract protected function setRuleList(); + abstract protected function setRuleList():void; /** * Returns an associative array of rules affected by each attribute. * The key of the array is the DOMElement attribute name that affects * the rule(s). The value of the array is an array of Rule objects. * - * @return Rule[] + * @return array */ public function getAttributeRuleList():array { $attributeRuleList = []; @@ -30,7 +30,7 @@ public function getAttributeRuleList():array { $attributeRuleList[$attrString] = []; } - $attributeRuleList[$attrString] []= $rule; + array_push($attributeRuleList[$attrString], $rule); } } diff --git a/src/Validator.php b/src/Validator.php index d53590d..a67582b 100644 --- a/src/Validator.php +++ b/src/Validator.php @@ -2,20 +2,15 @@ namespace Gt\DomValidation; use DOMElement; -use DOMNode; use DOMXPath; use Gt\CssXPath\Translator; use Gt\DomValidation\Rule\Rule; class Validator { - /** @var ValidationRules */ - protected $rules; - /** @var array */ - protected $errors; - /** @var ErrorList */ - protected $errorList; + protected ?ValidationRules $rules; + protected ErrorList $errorList; - public function __construct(ValidationRules $rules = null) { + public function __construct(?ValidationRules $rules = null) { if(is_null($rules)) { $rules = new DefaultValidationRules(); } @@ -25,7 +20,7 @@ public function __construct(ValidationRules $rules = null) { /** * @param DOMElement $form Form to validate - * @param array $input Associative array of user input + * @param array $input Associative array of user input */ public function validate(DOMElement $form, array $input):void { $this->errorList = new ErrorList(); From 35dead1d6b7b40f546480080606d955053194d7f Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Fri, 21 May 2021 19:49:35 +0100 Subject: [PATCH 2/7] build: update deps --- composer.lock | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/composer.lock b/composer.lock index fb9e475..fadcaa2 100644 --- a/composer.lock +++ b/composer.lock @@ -66,12 +66,12 @@ "source": { "type": "git", "url": "https://github.com/PhpGt/Dom.git", - "reference": "514ee3b4ef4b3ffdd748dafd1035ea9884e2ba86" + "reference": "5128ea82191fcc28fd44ea843f25288dc220c0e7" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/PhpGt/Dom/zipball/514ee3b4ef4b3ffdd748dafd1035ea9884e2ba86", - "reference": "514ee3b4ef4b3ffdd748dafd1035ea9884e2ba86", + "url": "https://api.github.com/repos/PhpGt/Dom/zipball/5128ea82191fcc28fd44ea843f25288dc220c0e7", + "reference": "5128ea82191fcc28fd44ea843f25288dc220c0e7", "shasum": "" }, "require": { @@ -147,11 +147,11 @@ }, "funding": [ { - "url": "https://github.com/phpgt", + "url": "https://github.com/sponsors/PhpGt", "type": "github" } ], - "time": "2021-03-23T14:45:19+00:00" + "time": "2021-05-19T10:14:42+00:00" }, { "name": "phpgt/propfunc", @@ -389,16 +389,16 @@ }, { "name": "nikic/php-parser", - "version": "v4.10.4", + "version": "v4.10.5", "source": { "type": "git", "url": "https://github.com/nikic/PHP-Parser.git", - "reference": "c6d052fc58cb876152f89f532b95a8d7907e7f0e" + "reference": "4432ba399e47c66624bc73c8c0f811e5c109576f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/c6d052fc58cb876152f89f532b95a8d7907e7f0e", - "reference": "c6d052fc58cb876152f89f532b95a8d7907e7f0e", + "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/4432ba399e47c66624bc73c8c0f811e5c109576f", + "reference": "4432ba399e47c66624bc73c8c0f811e5c109576f", "shasum": "" }, "require": { @@ -439,9 +439,9 @@ ], "support": { "issues": "https://github.com/nikic/PHP-Parser/issues", - "source": "https://github.com/nikic/PHP-Parser/tree/v4.10.4" + "source": "https://github.com/nikic/PHP-Parser/tree/v4.10.5" }, - "time": "2020-12-20T10:01:03+00:00" + "time": "2021-05-03T19:11:20+00:00" }, { "name": "phar-io/manifest", @@ -781,16 +781,16 @@ }, { "name": "phpstan/phpstan", - "version": "0.12.82", + "version": "0.12.88", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "3920f0fb0aff39263d3a4cb0bca120a67a1a6a11" + "reference": "464d1a81af49409c41074aa6640ed0c4cbd9bb68" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/3920f0fb0aff39263d3a4cb0bca120a67a1a6a11", - "reference": "3920f0fb0aff39263d3a4cb0bca120a67a1a6a11", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/464d1a81af49409c41074aa6640ed0c4cbd9bb68", + "reference": "464d1a81af49409c41074aa6640ed0c4cbd9bb68", "shasum": "" }, "require": { @@ -821,7 +821,7 @@ "description": "PHPStan - PHP Static Analysis Tool", "support": { "issues": "https://github.com/phpstan/phpstan/issues", - "source": "https://github.com/phpstan/phpstan/tree/0.12.82" + "source": "https://github.com/phpstan/phpstan/tree/0.12.88" }, "funding": [ { @@ -837,20 +837,20 @@ "type": "tidelift" } ], - "time": "2021-03-19T06:08:17+00:00" + "time": "2021-05-17T12:24:49+00:00" }, { "name": "phpunit/php-code-coverage", - "version": "9.2.5", + "version": "9.2.6", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-code-coverage.git", - "reference": "f3e026641cc91909d421802dd3ac7827ebfd97e1" + "reference": "f6293e1b30a2354e8428e004689671b83871edde" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/f3e026641cc91909d421802dd3ac7827ebfd97e1", - "reference": "f3e026641cc91909d421802dd3ac7827ebfd97e1", + "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/f6293e1b30a2354e8428e004689671b83871edde", + "reference": "f6293e1b30a2354e8428e004689671b83871edde", "shasum": "" }, "require": { @@ -906,7 +906,7 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/php-code-coverage/issues", - "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.5" + "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.6" }, "funding": [ { @@ -914,7 +914,7 @@ "type": "github" } ], - "time": "2020-11-28T06:44:49+00:00" + "time": "2021-03-28T07:26:59+00:00" }, { "name": "phpunit/php-file-iterator", From 4c4c230cef5f1e53d449391598fe7f71bc638e0e Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Fri, 21 May 2021 19:56:47 +0100 Subject: [PATCH 3/7] build: update to PHP.Gt/Dom's facade --- composer.json | 10 ++++++++-- composer.lock | 5 ++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 8de7065..e94e635 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,6 @@ "require": { "php": ">=8.0", - "ext-dom": "*", "phpgt/cssxpath": "1.*", "phpgt/dom": "dev-facade" }, @@ -29,5 +28,12 @@ "psr-4": { "Gt\\DomValidation\\Test\\": "./test/phpunit" } - } + }, + + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/PhpGt" + } + ] } diff --git a/composer.lock b/composer.lock index fadcaa2..f3753e4 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "db7cc9ab3192793fb9175a6edd58f9b6", + "content-hash": "5fe914123f757b59da86716e9cd2c182", "packages": [ { "name": "phpgt/cssxpath", @@ -2420,8 +2420,7 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": ">=8.0", - "ext-dom": "*" + "php": ">=8.0" }, "platform-dev": [], "plugin-api-version": "2.0.0" From b1bbd78139aff4d59d018e2cced9a272bf753ec6 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Sun, 23 May 2021 19:44:52 +0100 Subject: [PATCH 4/7] feature: update to use php.gt/dom facade --- README.md | 6 +++-- src/ErrorList.php | 6 ++--- src/Rule/MaxLength.php | 6 ++--- src/Rule/MinLength.php | 6 ++--- src/Rule/Pattern.php | 6 ++--- src/Rule/Required.php | 8 +++---- src/Rule/Rule.php | 6 ++--- src/Rule/SelectElement.php | 29 ++++++++++--------------- src/Rule/TypeDate.php | 8 +++---- src/Rule/TypeEmail.php | 8 +++---- src/Rule/TypeNumber.php | 6 ++--- src/Rule/TypeUrl.php | 8 +++---- src/Validator.php | 24 +++++--------------- test/phpunit/DomValidationTestCase.php | 23 ++++++++------------ test/phpunit/Rule/SelectElementTest.php | 21 ++++++++++++++++-- 15 files changed, 82 insertions(+), 89 deletions(-) diff --git a/README.md b/README.md index cb75c12..050ff8a 100644 --- a/README.md +++ b/README.md @@ -61,11 +61,13 @@ Validation rules present in the above HTML form: + `nation` input must be one of the three enumerations present in the `` and `` elements, their contained options are used as validation enumerations, meaning that values that are not part of the contained options will throw validation errors. [dom]: https://www.php.gt/dom -[webengine]: https://www.php.gt/webengine \ No newline at end of file +[webengine]: https://www.php.gt/webengine diff --git a/src/ErrorList.php b/src/ErrorList.php index 140644f..9157c6f 100644 --- a/src/ErrorList.php +++ b/src/ErrorList.php @@ -1,8 +1,8 @@ */ @@ -15,7 +15,7 @@ public function __construct() { $this->errorArray = []; } - public function add(DOMElement $element, string $errorMessage):void { + public function add(Element $element, string $errorMessage):void { $name = $element->getAttribute("name"); if(!isset($this->errorArray[$name])) { @@ -51,4 +51,4 @@ public function key():string { $keys = array_keys($this->errorArray); return $keys[$this->iteratorKey]; } -} \ No newline at end of file +} diff --git a/src/Rule/MaxLength.php b/src/Rule/MaxLength.php index f970724..70dcda6 100644 --- a/src/Rule/MaxLength.php +++ b/src/Rule/MaxLength.php @@ -1,19 +1,19 @@ getAttribute("maxlength"); return strlen($value) <= $maxLength; } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { $maxLength = $element->getAttribute("maxlength"); return "This field's value must contain $maxLength characters or less"; } diff --git a/src/Rule/MinLength.php b/src/Rule/MinLength.php index e87c895..8322659 100644 --- a/src/Rule/MinLength.php +++ b/src/Rule/MinLength.php @@ -1,19 +1,19 @@ getAttribute("minlength"); return strlen($value) >= $minLength; } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { $minLength = $element->getAttribute("minlength"); return "This field's value must contain at least $minLength characters"; } diff --git a/src/Rule/Pattern.php b/src/Rule/Pattern.php index 6e090c3..3f75b3e 100644 --- a/src/Rule/Pattern.php +++ b/src/Rule/Pattern.php @@ -1,19 +1,19 @@ getAttribute("pattern") . "/"; return preg_match($pattern, $value); } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { $hint = "This field does not match the required pattern"; if($title = $element->getAttribute("title")) { diff --git a/src/Rule/Required.php b/src/Rule/Required.php index 3fea74b..28222fe 100644 --- a/src/Rule/Required.php +++ b/src/Rule/Required.php @@ -1,18 +1,18 @@ attributes; } - abstract public function isValid(DOMElement $element, string $value):bool; + abstract public function isValid(Element $element, string $value):bool; - abstract public function getHint(DOMElement $element, string $value):string; + abstract public function getHint(Element $element, string $value):string; } diff --git a/src/Rule/SelectElement.php b/src/Rule/SelectElement.php index 305a82e..3174248 100644 --- a/src/Rule/SelectElement.php +++ b/src/Rule/SelectElement.php @@ -1,33 +1,26 @@ tagName !== "select") { + if($element->tagName !== "SELECT") { return true; } + /** @var HTMLSelectElement $element */ if($value === "") { return true; } - $optionElementList = $element->getElementsByTagName("option"); - for($i = 0, $len = $optionElementList->length; $i < $len; $i++) { - $option = $optionElementList->item($i); - - if($option->hasAttribute("value")) { - $optionValue = $option->getAttribute("value"); - } - else { - $optionValue = $option->nodeValue; - } - - if($optionValue !== "") { - $availableValues []= $optionValue; + foreach($element->options as $option) { + if($optionValue = $option->value) { + array_push($availableValues, $optionValue); } } @@ -38,7 +31,7 @@ public function isValid(DOMElement $element, string $value):bool { return true; } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { return "This field's value must match one of the available options"; } -} \ No newline at end of file +} diff --git a/src/Rule/TypeDate.php b/src/Rule/TypeDate.php index 61b9d1f..4b5db18 100644 --- a/src/Rule/TypeDate.php +++ b/src/Rule/TypeDate.php @@ -2,7 +2,7 @@ namespace Gt\DomValidation\Rule; use DateTime; -use DOMElement; +use Gt\Dom\Element; class TypeDate extends Rule { // ISO-8601 derived date formats: @@ -20,7 +20,7 @@ class TypeDate extends Rule { "type=time", ]; - public function isValid(DOMElement $element, string $value):bool { + public function isValid(Element $element, string $value):bool { if($value === "") { return true; } @@ -79,7 +79,7 @@ public function isValid(DOMElement $element, string $value):bool { return $dateTime !== false; } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { $format = null; $type = $element->getAttribute("type"); @@ -107,4 +107,4 @@ public function getHint(DOMElement $element, string $value):string { return "Field must be a $type in the format $format"; } -} \ No newline at end of file +} diff --git a/src/Rule/TypeEmail.php b/src/Rule/TypeEmail.php index b57630d..f73caab 100644 --- a/src/Rule/TypeEmail.php +++ b/src/Rule/TypeEmail.php @@ -1,19 +1,19 @@ getAttribute("min") ?: null; $max = $element->getAttribute("max") ?: null; $step = $element->getAttribute("step") ?: null; @@ -49,7 +49,7 @@ public function isValid(DOMElement $element, string $value):bool { return $validity; } - public function getHint(DOMElement $element, string $value):string { + public function getHint(Element $element, string $value):string { $min = $element->getAttribute("min") ?: null; $max = $element->getAttribute("max") ?: null; $step = $element->getAttribute("step") ?: null; diff --git a/src/Rule/TypeUrl.php b/src/Rule/TypeUrl.php index 4351e23..605e6ed 100644 --- a/src/Rule/TypeUrl.php +++ b/src/Rule/TypeUrl.php @@ -1,19 +1,19 @@ rules = $rules; } - /** - * @param DOMElement $form Form to validate - * @param array $input Associative array of user input - */ - public function validate(DOMElement $form, array $input):void { + /** @param array $input Associative array of user input */ + public function validate(Element $form, array $input):void { $this->errorList = new ErrorList(); foreach($this->rules->getAttributeRuleList() as $attrString => $ruleArray) { /** @var Rule[] $ruleArray */ - $cssSelector = "[$attrString]"; - - $xpath = new DOMXPath($form->ownerDocument); - $inputElementList = $xpath->query( - new Translator($cssSelector) - ); - - for($i = 0, $len = $inputElementList->length; $i < $len; $i++) { - /** @var DOMElement $element */ - $element = $inputElementList->item($i); + foreach($form->querySelectorAll("[$attrString]") as $element) { $name = $element->getAttribute("name"); foreach($ruleArray as $rule) { @@ -62,4 +48,4 @@ public function validate(DOMElement $form, array $input):void { public function getLastErrorList():ErrorList { return $this->errorList; } -} \ No newline at end of file +} diff --git a/test/phpunit/DomValidationTestCase.php b/test/phpunit/DomValidationTestCase.php index 24b01a6..cb319d1 100644 --- a/test/phpunit/DomValidationTestCase.php +++ b/test/phpunit/DomValidationTestCase.php @@ -1,21 +1,16 @@ loadHTML($html); - - /** @var DOMElement $domElement */ - $domElement = $document->getElementsByTagName( - "form" - )->item( - 0 - ); - return $domElement; + public static function getFormFromHtml(string $html):HTMLFormElement { + $document = new HTMLDocument($html); + /** @var HTMLFormElement $form */ + /** @noinspection PhpUnnecessaryLocalVariableInspection */ + $form = $document->forms[0]; + return $form; } -} \ No newline at end of file +} diff --git a/test/phpunit/Rule/SelectElementTest.php b/test/phpunit/Rule/SelectElementTest.php index 20e5543..5ebed4e 100644 --- a/test/phpunit/Rule/SelectElementTest.php +++ b/test/phpunit/Rule/SelectElementTest.php @@ -81,10 +81,12 @@ public function testSelectTextContentInvalid() { } } - public function testSelectValueInvalid() { + public function testSelectValue_textContent() { $form = self::getFormFromHtml(Helper::HTML_SELECT); $validator = new Validator(); + $exception = null; + try { $validator->validate($form, [ "currency" => "USD", @@ -92,6 +94,21 @@ public function testSelectValueInvalid() { // There is an