From 56cec453f9cdcc9b3fb0c0d361a6e08569c8eb04 Mon Sep 17 00:00:00 2001 From: David Isberner Date: Wed, 22 Jun 2022 11:10:36 +0200 Subject: [PATCH 1/4] Add support for array generics as required by PHPStan. --- .../Sniffs/Docblock/ReturnTypeSniff.php | 65 +++++++++++++++---- .../doc/allowed/ReturnTypeArrayGenerics.php | 50 ++++++++++++++ ...ptyAlternativeInReturnTypeInDocComment.php | 14 ++++ .../EmptyGenericReturnTypeInDocComment.php | 14 ++++ ...validStructureInReturnTypeInDocComment.php | 14 ++++ ...ativeSeparatorInReturnTypeInDocComment.php | 14 ++++ .../UnknownGenericReturnTypeInDocComment.php | 14 ++++ ...nknownGenericSubReturnTypeInDocComment.php | 14 ++++ 8 files changed, 185 insertions(+), 14 deletions(-) create mode 100644 tests/rules/doc/allowed/ReturnTypeArrayGenerics.php create mode 100644 tests/rules/doc/not-allowed/EmptyAlternativeInReturnTypeInDocComment.php create mode 100644 tests/rules/doc/not-allowed/EmptyGenericReturnTypeInDocComment.php create mode 100644 tests/rules/doc/not-allowed/InvalidStructureInReturnTypeInDocComment.php create mode 100644 tests/rules/doc/not-allowed/MissingAlternativeSeparatorInReturnTypeInDocComment.php create mode 100644 tests/rules/doc/not-allowed/UnknownGenericReturnTypeInDocComment.php create mode 100644 tests/rules/doc/not-allowed/UnknownGenericSubReturnTypeInDocComment.php diff --git a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php index 6271637..d504893 100644 --- a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php +++ b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php @@ -58,22 +58,13 @@ public function process(File $phpcsFile, $stackPtr) $tokens = $phpcsFile->getTokens(); $returnTypePtr = $this->getDocReturnTypePtr($phpcsFile, $stackPtr); $returnTypeString = $tokens[$returnTypePtr]['content']; - $returnTypes = explode('|', $returnTypeString); - - foreach ($returnTypes as $returnType) { - $returnType = trim($returnType); - if (in_array($returnType, $this->returnTypeScalarWhitelist, true)) { - continue; - } - if (in_array($returnType, $this->returnTypeClassWhitelist, true)) { - continue; - } - if ($this->isStartingWithUppercaseLetter($returnType)) { - continue; - } + try { + $this->checkReturnTypeShape($returnTypeString); + } + catch (\InvalidArgumentException $exception) { $phpcsFile->addError( - sprintf('Return type "%s" is discouraged', $returnType), + $exception->getMessage(), $returnTypePtr, 'ProhibitedReturnType' ); @@ -145,4 +136,50 @@ private function isStartingWithUppercaseLetter($singleCharacter) return false; } + + /** + * @param string $subject The return type string to check. + * @return void + */ + private function checkReturnTypeShape(string $subject) + { + preg_match_all('#(?\s*\|\s*)?(?[^<>\|]+)(?<(?.*)>)?#', $subject, $matches); + + if (implode('', $matches[0]) !== $subject) { + throw new \InvalidArgumentException('Invalid structure in return type "' . $subject . '"'); + } + + if (strpos($matches['separator'][0], '|') !== false) { + throw new \InvalidArgumentException('Missing return type in first alternative of type "' . $subject . '"'); + } + + foreach ($matches['nested'] as $index => $match) { + if (!empty($matches['generic'][$index])) { + if (trim($matches['atom'][$index]) !== 'array') { + throw new \InvalidArgumentException('Unexpected generic specification in type "' . $matches[0][$index] . '"'); + } + + $match = trim($match); + if ($match === '') { + throw new \InvalidArgumentException('Generic specification may not be empty in type "' . $matches[0][$index] . '"'); + } + + $this->checkReturnTypeShape($match); + } + + // Check if atom is in whitelist. + $returnType = trim($matches['atom'][$index]); + if (in_array($returnType, $this->returnTypeScalarWhitelist, true)) { + continue; + } + if (in_array($returnType, $this->returnTypeClassWhitelist, true)) { + continue; + } + if ($this->isStartingWithUppercaseLetter($returnType)) { + continue; + } + + throw new \InvalidArgumentException('Return type "' . $returnType . '" is discouraged'); + } + } } diff --git a/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php b/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php new file mode 100644 index 0000000..c800c30 --- /dev/null +++ b/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php @@ -0,0 +1,50 @@ + + */ + public function testWithGeneric() + { + } + + /** + * @return array + */ + public function testWithAlternativeInside() + { + } + + /** + * @return int | array> + */ + public function testWithAlternativeOutside() + { + } + + /** + * @return array> | null + */ + public function testWithAlternativeAtTheEnd() + { + } + + /** + * @return int | array> + */ + public function testWithMultipleAlternatives() + { + } +} diff --git a/tests/rules/doc/not-allowed/EmptyAlternativeInReturnTypeInDocComment.php b/tests/rules/doc/not-allowed/EmptyAlternativeInReturnTypeInDocComment.php new file mode 100644 index 0000000..f1cfe5d --- /dev/null +++ b/tests/rules/doc/not-allowed/EmptyAlternativeInReturnTypeInDocComment.php @@ -0,0 +1,14 @@ +" + +class EmptyGenericReturnTypeInDocComment +{ + /** + * @return array<> + */ + public function foo() + { + return true; + } +} diff --git a/tests/rules/doc/not-allowed/InvalidStructureInReturnTypeInDocComment.php b/tests/rules/doc/not-allowed/InvalidStructureInReturnTypeInDocComment.php new file mode 100644 index 0000000..f446ab9 --- /dev/null +++ b/tests/rules/doc/not-allowed/InvalidStructureInReturnTypeInDocComment.php @@ -0,0 +1,14 @@ +" + +class UnknownGenericReturnTypeInDocComment +{ + /** + * @return int + */ + public function foo() + { + return true; + } +} diff --git a/tests/rules/doc/not-allowed/UnknownGenericSubReturnTypeInDocComment.php b/tests/rules/doc/not-allowed/UnknownGenericSubReturnTypeInDocComment.php new file mode 100644 index 0000000..769521f --- /dev/null +++ b/tests/rules/doc/not-allowed/UnknownGenericSubReturnTypeInDocComment.php @@ -0,0 +1,14 @@ + + */ + public function foo() + { + return true; + } +} From 67c1243f94987abae15dfdb4d7cbf2fc7c68b8e7 Mon Sep 17 00:00:00 2001 From: David Isberner Date: Wed, 22 Jun 2022 11:17:38 +0200 Subject: [PATCH 2/4] Add support for numerically indexed array generics. --- .../Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php | 5 +++++ tests/rules/doc/allowed/ReturnTypeArrayGenerics.php | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php index d504893..c33d480 100644 --- a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php +++ b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php @@ -160,6 +160,11 @@ private function checkReturnTypeShape(string $subject) } $match = trim($match); + if (strpos($match, 'int,') === 0) { + // Allow numeric indexing in generics, e.g. `array` + $match = substr($match, 4); + } + if ($match === '') { throw new \InvalidArgumentException('Generic specification may not be empty in type "' . $matches[0][$index] . '"'); } diff --git a/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php b/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php index c800c30..93082b6 100644 --- a/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php +++ b/tests/rules/doc/allowed/ReturnTypeArrayGenerics.php @@ -20,6 +20,13 @@ public function testWithGeneric() { } + /** + * @return array + */ + public function testWithNumericIndex() + { + } + /** * @return array */ From 68eb458f08c22da23241c97574f908c4fa8fb7bf Mon Sep 17 00:00:00 2001 From: Mirroar Date: Mon, 27 Jun 2022 12:41:55 +0200 Subject: [PATCH 3/4] Make sure the pattern was matched at least once. --- .../Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php index c33d480..391188b 100644 --- a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php +++ b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php @@ -143,9 +143,9 @@ private function isStartingWithUppercaseLetter($singleCharacter) */ private function checkReturnTypeShape(string $subject) { - preg_match_all('#(?\s*\|\s*)?(?[^<>\|]+)(?<(?.*)>)?#', $subject, $matches); + $matched = preg_match_all('#(?\s*\|\s*)?(?[^<>\|]+)(?<(?.*)>)?#', $subject, $matches); - if (implode('', $matches[0]) !== $subject) { + if (!$matched || implode('', $matches[0]) !== $subject) { throw new \InvalidArgumentException('Invalid structure in return type "' . $subject . '"'); } From a99a2d91229993c547d90ffd90bf48eb531dea65 Mon Sep 17 00:00:00 2001 From: David Isberner Date: Mon, 27 Jun 2022 12:45:11 +0200 Subject: [PATCH 4/4] Check for empty string instead of using php's `empty`. --- custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php index 391188b..f7fdb22 100644 --- a/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php +++ b/custom-standards/Flyeralarm/Sniffs/Docblock/ReturnTypeSniff.php @@ -154,7 +154,7 @@ private function checkReturnTypeShape(string $subject) } foreach ($matches['nested'] as $index => $match) { - if (!empty($matches['generic'][$index])) { + if ($matches['generic'][$index] === '') { if (trim($matches['atom'][$index]) !== 'array') { throw new \InvalidArgumentException('Unexpected generic specification in type "' . $matches[0][$index] . '"'); }