RequireNonCapturingCatchSniff: Fixed false positive

slevomat · Jul 9, 2021 · 751e737 · 751e737
1 parent 59bdaf8
commit 751e737
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 17 deletions.
diff --git a/SlevomatCodingStandard/Helpers/CatchHelper.php b/SlevomatCodingStandard/Helpers/CatchHelper.php
@@ -4,14 +4,37 @@
 
 use PHP_CodeSniffer\Files\File;
 use function array_merge;
+use function in_array;
 use const T_BITWISE_OR;
+use const T_CATCH;
+use const T_FINALLY;
 
 /**
  * @internal
  */
 class CatchHelper
 {
 
+	public static function getTryEndPointer(File $phpcsFile, int $catchPointer): int
+	{
+		$tokens = $phpcsFile->getTokens();
+
+		$endPointer = $tokens[$catchPointer]['scope_closer'];
+
+		do {
+			$nextPointer = TokenHelper::findNextEffective($phpcsFile, $endPointer + 1);
+
+			if ($nextPointer === null || !in_array($tokens[$nextPointer]['code'], [T_CATCH, T_FINALLY], true)) {
+				break;
+			}
+
+			$endPointer = $tokens[$nextPointer]['scope_closer'];
+
+		} while (true);
+
+		return $endPointer;
+	}
+
 	/**
 	 * @param File $phpcsFile
 	 * @param array<string, array<int, int|string>|int|string> $catchToken

diff --git a/SlevomatCodingStandard/Sniffs/Exceptions/RequireNonCapturingCatchSniff.php b/SlevomatCodingStandard/Sniffs/Exceptions/RequireNonCapturingCatchSniff.php
@@ -4,11 +4,15 @@
 
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use SlevomatCodingStandard\Helpers\CatchHelper;
 use SlevomatCodingStandard\Helpers\ParameterHelper;
 use SlevomatCodingStandard\Helpers\ScopeHelper;
 use SlevomatCodingStandard\Helpers\SniffSettingsHelper;
 use SlevomatCodingStandard\Helpers\TokenHelper;
 use SlevomatCodingStandard\Helpers\VariableHelper;
+use function array_keys;
+use function array_reverse;
+use function count;
 use function in_array;
 use const T_CATCH;
 use const T_DOUBLE_QUOTED_STRING;
@@ -61,25 +65,26 @@ public function process(File $phpcsFile, $catchPointer): void
 
 		$variableName = $tokens[$variablePointer]['content'];
 
-		for ($i = $tokens[$catchPointer]['scope_opener'] + 1; $i < $tokens[$catchPointer]['scope_closer']; $i++) {
-			if ($tokens[$i]['code'] === T_VARIABLE) {
-				if (ParameterHelper::isParameter($phpcsFile, $i)) {
-					continue;
-				}
+		if ($this->isVariableUsedInCodePart(
+			$phpcsFile,
+			$tokens[$catchPointer]['scope_opener'],
+			$tokens[$catchPointer]['scope_closer'],
+			$variableName
+		)) {
+			return;
+		}
 
-				if (!ScopeHelper::isInSameScope($phpcsFile, $tokens[$catchPointer]['scope_opener'] + 1, $i)) {
-					continue;
-				}
+		$tryEndPointer = CatchHelper::getTryEndPointer($phpcsFile, $catchPointer);
 
-				if ($tokens[$i]['content'] === $variableName) {
-					return;
-				}
-			} elseif (
-				in_array($tokens[$i]['code'], [T_DOUBLE_QUOTED_STRING, T_HEREDOC], true)
-				&& VariableHelper::isUsedInScopeInString($phpcsFile, $variableName, $i)
-			) {
-				return;
-			}
+		if ($tokens[$tryEndPointer]['conditions'] !== []) {
+			$lastConditionPointer = array_reverse(array_keys($tokens[$tryEndPointer]['conditions']))[0];
+			$nextScopeEnd = $tokens[$lastConditionPointer]['scope_closer'];
+		} else {
+			$nextScopeEnd = count($tokens) - 1;
+		}
+
+		if ($this->isVariableUsedInCodePart($phpcsFile, $tryEndPointer, $nextScopeEnd, $variableName)) {
+			return;
 		}
 
 		$fix = $phpcsFile->addFixableError('Non capturing catch is required.', $catchPointer, self::CODE_NON_CAPTURING_CATCH_REQUIRED);
@@ -109,4 +114,44 @@ public function process(File $phpcsFile, $catchPointer): void
 		$phpcsFile->fixer->endChangeset();
 	}
 
+	private function isVariableUsedInCodePart(File $phpcsFile, int $codeStartPointer, int $codeEndPointer, string $variableName): bool
+	{
+		$tokens = $phpcsFile->getTokens();
+
+		$firstPointerInCode = $codeStartPointer + 1;
+
+		for ($i = $firstPointerInCode; $i <= $codeEndPointer; $i++) {
+			if ($tokens[$i]['code'] === T_VARIABLE) {
+				if (ParameterHelper::isParameter($phpcsFile, $i)) {
+					continue;
+				}
+
+				if (!ScopeHelper::isInSameScope($phpcsFile, $firstPointerInCode, $i)) {
+					continue;
+				}
+
+				if ($tokens[$i]['content'] !== $variableName) {
+					continue;
+				}
+
+				$catchPointer = TokenHelper::findPrevious($phpcsFile, T_CATCH, $i - 1, $firstPointerInCode);
+				if ($catchPointer === null) {
+					return true;
+				}
+
+				if ($tokens[$catchPointer]['parenthesis_closer'] < $i) {
+					return true;
+				}
+
+			} elseif (
+				in_array($tokens[$i]['code'], [T_DOUBLE_QUOTED_STRING, T_HEREDOC], true)
+				&& VariableHelper::isUsedInScopeInString($phpcsFile, $variableName, $i)
+			) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 }
diff --git a/tests/Sniffs/Exceptions/data/requireNonCapturingCatchNoErrors.php b/tests/Sniffs/Exceptions/data/requireNonCapturingCatchNoErrors.php
@@ -9,10 +9,23 @@
 try {
 
 } catch (Throwable $e) {
+	$a = $e;
 	rollback($e);
 }
 
 try {
 } catch (\ErrorException $exception) {
 	echo "Exception message: {$exception->getMessage()}";
 }
+
+function () {
+	try {
+	} catch (\ErrorException $exception) {
+	} finally {
+		// Nothing
+	}
+
+	if (isset($exception)) {
+
+	}
+};