From 4f59b04396da59871b2328147cbed2bcb459c212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20K=C5=99=C3=AD=C5=BE?= Date: Tue, 25 Jul 2017 18:56:33 +0200 Subject: [PATCH] Check void return types on closures --- .../Helpers/FunctionHelper.php | 6 ++- .../TypeHints/TypeHintDeclarationSniff.php | 50 +++++++++++++++++++ .../TypeHintDeclarationSniffTest.php | 10 ++-- .../data/enabledVoidTypeHintErrors.php | 24 +++++++++ .../data/enabledVoidTypeHintNoErrors.php | 28 +++++++++++ ...eReturnTypeHintsWithDisabledVoid.fixed.php | 16 ++++++ ...fixableReturnTypeHintsWithDisabledVoid.php | 16 ++++++ ...leReturnTypeHintsWithEnabledVoid.fixed.php | 16 ++++++ .../fixableReturnTypeHintsWithEnabledVoid.php | 16 ++++++ 9 files changed, 177 insertions(+), 5 deletions(-) diff --git a/SlevomatCodingStandard/Helpers/FunctionHelper.php b/SlevomatCodingStandard/Helpers/FunctionHelper.php index 799b821ae..26dc15e05 100644 --- a/SlevomatCodingStandard/Helpers/FunctionHelper.php +++ b/SlevomatCodingStandard/Helpers/FunctionHelper.php @@ -127,10 +127,12 @@ public static function returnsValue(\PHP_CodeSniffer_File $codeSnifferFile, int $isInSameLevel = function (int $pointer) use ($functionPointer, $tokens): bool { foreach (array_reverse($tokens[$pointer]['conditions'], true) as $conditionPointer => $conditionTokenCode) { + if ($conditionPointer === $functionPointer) { + break; + } + if ($conditionTokenCode === T_CLOSURE || $conditionTokenCode === T_ANON_CLASS) { return false; - } elseif ($conditionPointer === $functionPointer) { - break; } } return true; diff --git a/SlevomatCodingStandard/Sniffs/TypeHints/TypeHintDeclarationSniff.php b/SlevomatCodingStandard/Sniffs/TypeHints/TypeHintDeclarationSniff.php index 68a849e52..d79187b5b 100644 --- a/SlevomatCodingStandard/Sniffs/TypeHints/TypeHintDeclarationSniff.php +++ b/SlevomatCodingStandard/Sniffs/TypeHints/TypeHintDeclarationSniff.php @@ -35,6 +35,8 @@ class TypeHintDeclarationSniff implements \PHP_CodeSniffer_Sniff const CODE_USELESS_RETURN_ANNOTATION = 'UselessReturnAnnotation'; + const CODE_INCORRECT_RETURN_TYPE_HINT = 'IncorrectReturnTypeHint'; + const CODE_USELESS_DOC_COMMENT = 'UselessDocComment'; /** @var bool */ @@ -71,6 +73,8 @@ public function process(\PHP_CodeSniffer_File $phpcsFile, $pointer) $this->checkParametersTypeHints($phpcsFile, $pointer); $this->checkReturnTypeHints($phpcsFile, $pointer); $this->checkUselessDocComment($phpcsFile, $pointer); + } elseif ($token['code'] === T_CLOSURE) { + $this->checkClosure($phpcsFile, $pointer); } elseif ($token['code'] === T_VARIABLE && PropertyHelper::isProperty($phpcsFile, $pointer)) { $this->checkPropertyTypeHint($phpcsFile, $pointer); } @@ -83,6 +87,7 @@ public function register(): array { return [ T_FUNCTION, + T_CLOSURE, T_VARIABLE, ]; } @@ -473,6 +478,51 @@ private function checkReturnTypeHints(\PHP_CodeSniffer_File $phpcsFile, int $fun } } + private function checkClosure(\PHP_CodeSniffer_File $phpcsFile, int $closurePointer) + { + $returnTypeHint = FunctionHelper::findReturnTypeHint($phpcsFile, $closurePointer); + $returnsValue = FunctionHelper::returnsValue($phpcsFile, $closurePointer); + + if (!$returnsValue && $returnTypeHint !== null && $returnTypeHint->getTypeHint() !== 'void') { + $fix = $phpcsFile->addFixableError( + 'Closure has incorrect return type hint.', + $closurePointer, + self::CODE_INCORRECT_RETURN_TYPE_HINT + ); + + if ($fix) { + $tokens = $phpcsFile->getTokens(); + $closeParenthesisPosition = TokenHelper::findPrevious($phpcsFile, [T_CLOSE_PARENTHESIS], $tokens[$closurePointer]['scope_opener'] - 1, $closurePointer); + + $phpcsFile->fixer->beginChangeset(); + for ($i = $closeParenthesisPosition + 1; $i < $tokens[$closurePointer]['scope_opener']; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + $phpcsFile->fixer->replaceToken($closeParenthesisPosition, $this->enableVoidTypeHint ? '): void ' : ') '); + $phpcsFile->fixer->endChangeset(); + } + + return; + } + + if ($this->enableVoidTypeHint && !$returnsValue && $returnTypeHint === null) { + $fix = $phpcsFile->addFixableError( + 'Closure does not have void return type hint.', + $closurePointer, + self::CODE_MISSING_RETURN_TYPE_HINT + ); + + if ($fix) { + $tokens = $phpcsFile->getTokens(); + $position = TokenHelper::findPreviousEffective($phpcsFile, $tokens[$closurePointer]['scope_opener'] - 1, $closurePointer); + + $phpcsFile->fixer->beginChangeset(); + $phpcsFile->fixer->addContent($position, ': void'); + $phpcsFile->fixer->endChangeset(); + } + } + } + private function checkUselessDocComment(\PHP_CodeSniffer_File $phpcsFile, int $functionPointer) { $docCommentSniffSuppressed = SuppressHelper::isSniffSuppressed($phpcsFile, $functionPointer, $this->getSniffName(self::CODE_USELESS_DOC_COMMENT)); diff --git a/tests/Sniffs/TypeHints/TypeHintDeclarationSniffTest.php b/tests/Sniffs/TypeHints/TypeHintDeclarationSniffTest.php index fc04a825f..7860b678f 100644 --- a/tests/Sniffs/TypeHints/TypeHintDeclarationSniffTest.php +++ b/tests/Sniffs/TypeHints/TypeHintDeclarationSniffTest.php @@ -140,12 +140,16 @@ public function testEnabledVoidTypeHintErrors() 'enableVoidTypeHint' => true, ]); - $this->assertSame(4, $report->getErrorCount()); + $this->assertSame(8, $report->getErrorCount()); $this->assertSniffError($report, 3, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); $this->assertSniffError($report, 14, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); $this->assertSniffError($report, 16, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); $this->assertSniffError($report, 24, TypeHintDeclarationSniff::CODE_USELESS_DOC_COMMENT); + $this->assertSniffError($report, 31, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); + $this->assertSniffError($report, 35, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); + $this->assertSniffError($report, 39, TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT); + $this->assertSniffError($report, 51, TypeHintDeclarationSniff::CODE_INCORRECT_RETURN_TYPE_HINT); } public function testEnabledNullableTypeHintsNoErrors() @@ -259,7 +263,7 @@ public function testFixableReturnTypeHintsWithEnabledVoid() $report = $this->checkFile(__DIR__ . '/data/fixableReturnTypeHintsWithEnabledVoid.php', [ 'enableNullableTypeHints' => false, 'enableVoidTypeHint' => true, - ], [TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT]); + ], [TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT, TypeHintDeclarationSniff::CODE_INCORRECT_RETURN_TYPE_HINT]); $this->assertAllFixedInFile($report); } @@ -269,7 +273,7 @@ public function testFixableReturnTypeHintsWithDisabledVoid() $report = $this->checkFile(__DIR__ . '/data/fixableReturnTypeHintsWithDisabledVoid.php', [ 'enableNullableTypeHints' => false, 'enableVoidTypeHint' => false, - ], [TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT]); + ], [TypeHintDeclarationSniff::CODE_MISSING_RETURN_TYPE_HINT, TypeHintDeclarationSniff::CODE_INCORRECT_RETURN_TYPE_HINT]); $this->assertAllFixedInFile($report); } diff --git a/tests/Sniffs/TypeHints/data/enabledVoidTypeHintErrors.php b/tests/Sniffs/TypeHints/data/enabledVoidTypeHintErrors.php index 69be5c402..442fd4b2d 100644 --- a/tests/Sniffs/TypeHints/data/enabledVoidTypeHintErrors.php +++ b/tests/Sniffs/TypeHints/data/enabledVoidTypeHintErrors.php @@ -27,3 +27,27 @@ public function withSuppress(): void } } + +function () { + +}; + +function () { + return; +}; + +function () { + function (): bool { + return true; + }; + new class { + public function foo(): bool + { + return true; + } + }; +}; + +function (): bool { + +}; diff --git a/tests/Sniffs/TypeHints/data/enabledVoidTypeHintNoErrors.php b/tests/Sniffs/TypeHints/data/enabledVoidTypeHintNoErrors.php index 5366893a1..f18dd81e8 100644 --- a/tests/Sniffs/TypeHints/data/enabledVoidTypeHintNoErrors.php +++ b/tests/Sniffs/TypeHints/data/enabledVoidTypeHintNoErrors.php @@ -43,3 +43,31 @@ public function withSuppress() } } + +function (): void { + +}; + +function (): void { + return; +}; + +function (): void { + function (): bool { + return true; + }; + new class { + public function foo(): bool + { + return true; + } + }; +}; + +function () { + return true; +}; + +function (): bool { + return true; +}; diff --git a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.fixed.php b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.fixed.php index acf3d03fe..9ab5cb19a 100644 --- a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.fixed.php +++ b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.fixed.php @@ -61,3 +61,19 @@ public function voidAnnotation() } } + +function () { + +}; + +function () { + return; +}; + +function () { + +}; + +function () use (& $foo) { + +}; diff --git a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.php b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.php index acf3d03fe..fe7e6c8f3 100644 --- a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.php +++ b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithDisabledVoid.php @@ -61,3 +61,19 @@ public function voidAnnotation() } } + +function () { + +}; + +function () { + return; +}; + +function (): bool { + +}; + +function () use (& $foo): \Foo\Bar { + +}; diff --git a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.fixed.php b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.fixed.php index b1fbd377a..cef358ae6 100644 --- a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.fixed.php +++ b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.fixed.php @@ -53,3 +53,19 @@ protected function returnsNothing(): void public abstract function voidAnnotation(): void; } + +function (): void { + +}; + +function (): void { + return; +}; + +function (): void { + +}; + +function () use (& $foo): void { + +}; diff --git a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.php b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.php index b6e4fe22c..518150859 100644 --- a/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.php +++ b/tests/Sniffs/TypeHints/data/fixableReturnTypeHintsWithEnabledVoid.php @@ -53,3 +53,19 @@ protected function returnsNothing() public abstract function voidAnnotation(); } + +function () { + +}; + +function () { + return; +}; + +function (): bool { + +}; + +function () use (& $foo): \Foo\Bar { + +};