Check void return types on closures

slevomat · Sep 15, 2017 · 4f59b04 · 4f59b04
1 parent 1c3df50
commit 4f59b04
Show file tree

Hide file tree

Showing 9 changed files with 177 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -53,3 +53,19 @@ protected function returnsNothing()
 	public abstract function voidAnnotation();
 
 }
+
+function () {
+
+};
+
+function () {
+	return;
+};
+
+function (): bool {
+
+};
+
+function () use (& $foo): \Foo\Bar {
+
+};