EarlyExitSniff: fixed false positive

Author: kukulich

SlevomatCodingStandard/Helpers/TokenHelper.php

@@ -41,6 +41,16 @@ class TokenHelper
 		T_CALLABLE,
 	];
 
+	/** @var mixed[] */
+	public static $earlyExitTokenCodes = [
+		T_RETURN,
+		T_CONTINUE,
+		T_BREAK,
+		T_THROW,
+		T_YIELD,
+		T_EXIT,
+	];
+
 	/**
 	 * @param \PHP_CodeSniffer\Files\File $phpcsFile
 	 * @param mixed|mixed[] $types

SlevomatCodingStandard/Sniffs/ControlStructures/EarlyExitSniff.php

@@ -44,15 +44,7 @@ private function processElse(\PHP_CodeSniffer\Files\File $phpcsFile, int $elsePo
 	{
 		$tokens = $phpcsFile->getTokens();
 
-		$earlyExitTokenCodes = [T_RETURN, T_CONTINUE, T_BREAK, T_THROW, T_YIELD, T_EXIT];
-
-		$lastSemicolonInElseScopePointer = TokenHelper::findPreviousEffective($phpcsFile, $tokens[$elsePointer]['scope_closer'] - 1);
-		if ($tokens[$lastSemicolonInElseScopePointer]['code'] !== T_SEMICOLON) {
-			return;
-		}
-
-		$earlyExitInElseScopePointer = TokenHelper::findPreviousLocal($phpcsFile, $earlyExitTokenCodes, $lastSemicolonInElseScopePointer - 1, $tokens[$elsePointer]['scope_opener']);
-		if ($earlyExitInElseScopePointer === null) {
+		if (!$this->isEarlyExitInScope($phpcsFile, $tokens[$elsePointer]['scope_opener'], $tokens[$elsePointer]['scope_closer'])) {
 			return;
 		}
 
@@ -65,12 +57,7 @@ private function processElse(\PHP_CodeSniffer\Files\File $phpcsFile, int $elsePo
 
 		$ifPointer = $previousConditionPointer;
 
-		$lastSemicolonInIfScopePointer = TokenHelper::findPreviousEffective($phpcsFile, $tokens[$ifPointer]['scope_closer'] - 1);
-		$isEarlyExitInIfScope = $tokens[$lastSemicolonInIfScopePointer]['code'] === T_SEMICOLON
-			? TokenHelper::findPreviousLocal($phpcsFile, $earlyExitTokenCodes, $lastSemicolonInIfScopePointer - 1, $tokens[$ifPointer]['scope_opener']) !== null
-			: false;
-
-		if ($isEarlyExitInIfScope) {
+		if ($this->isEarlyExitInScope($phpcsFile, $tokens[$ifPointer]['scope_opener'], $tokens[$ifPointer]['scope_closer'])) {
 			$fix = $phpcsFile->addFixableError(
 				'Remove useless else to reduce code nesting.',
 				$elsePointer,
@@ -153,6 +140,10 @@ private function processIf(\PHP_CodeSniffer\Files\File $phpcsFile, int $ifPointe
 			return;
 		}
 
+		if ($this->isEarlyExitInScope($phpcsFile, $tokens[$ifPointer]['scope_opener'], $tokens[$ifPointer]['scope_closer'])) {
+			return;
+		}
+
 		$fix = $phpcsFile->addFixableError(
 			'Use early exit to reduce code nesting.',
 			$ifPointer,
@@ -341,4 +332,12 @@ private function fixIndentation(string $code, string $eolChar, string $defaultIn
 		}, explode($eolChar, rtrim($code))));
 	}
 
+	private function isEarlyExitInScope(\PHP_CodeSniffer\Files\File $phpcsFile, int $startPointer, int $endPointer): bool
+	{
+		$lastSemicolonInScopePointer = TokenHelper::findPreviousEffective($phpcsFile, $endPointer - 1);
+		return $phpcsFile->getTokens()[$lastSemicolonInScopePointer]['code'] === T_SEMICOLON
+			? TokenHelper::findPreviousLocal($phpcsFile, TokenHelper::$earlyExitTokenCodes, $lastSemicolonInScopePointer - 1, $startPointer) !== null
+			: false;
+	}
+
 }

tests/Sniffs/ControlStructures/data/earlyExitNoErrors.php

@@ -133,3 +133,9 @@ function () {
 
 	doSomethingAgain();
 }
+
+foreach (['foo', 'bar'] as $identity) {
+	if ($identity === 'foo') {
+		break;
+	}
+}