From 59e161a750a723a9e977814d84ccc64747248d6e Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 11 May 2021 21:42:27 +0200 Subject: [PATCH] Generic/InlineControlStructure: bug fix for try/catch/finally Multi-catch and `finally` statements were not taken into account when determining the end of the statement within an inline control structure. Fixed now. Includes unit tests. Fixes 3345 --- .../InlineControlStructureSniff.php | 20 ++++++++++++------ .../InlineControlStructureUnitTest.1.inc | 19 +++++++++++++++++ ...InlineControlStructureUnitTest.1.inc.fixed | 21 +++++++++++++++++++ .../InlineControlStructureUnitTest.php | 2 ++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php b/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php index 19a58833be..a67a6a798c 100644 --- a/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php +++ b/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php @@ -210,7 +210,10 @@ public function process(File $phpcsFile, $stackPtr) if (isset($tokens[$end]['scope_opener']) === true) { $type = $tokens[$end]['code']; $end = $tokens[$end]['scope_closer']; - if ($type === T_DO || $type === T_IF || $type === T_ELSEIF || $type === T_TRY) { + if ($type === T_DO + || $type === T_IF || $type === T_ELSEIF + || $type === T_TRY || $type === T_CATCH || $type === T_FINALLY + ) { $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($end + 1), null, true); if ($next === false) { break; @@ -227,15 +230,20 @@ public function process(File $phpcsFile, $stackPtr) continue; } + // Account for TRY... CATCH/FINALLY statements. + if (($type === T_TRY + || $type === T_CATCH + || $type === T_FINALLY) + && ($nextType === T_CATCH + || $nextType === T_FINALLY) + ) { + continue; + } + // Account for DO... WHILE conditions. if ($type === T_DO && $nextType === T_WHILE) { $end = $phpcsFile->findNext(T_SEMICOLON, ($next + 1)); } - - // Account for TRY... CATCH statements. - if ($type === T_TRY && $nextType === T_CATCH) { - $end = $tokens[$next]['scope_closer']; - } } else if ($type === T_CLOSURE) { // There should be a semicolon after the closing brace. $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($end + 1), null, true); diff --git a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc index 1a6ab6900a..fb4a7380fb 100644 --- a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc +++ b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc @@ -253,3 +253,22 @@ for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++); if ($this->valid(fn(): bool => 2 > 1)) { } + +// Issue 3345. +function testMultiCatch() +{ + if (true) + try { + } catch (\LogicException $e) { + } catch (\Exception $e) { + } +} + +function testFinally() +{ + if (true) + try { + } catch (\LogicException $e) { + } finally { + } +} diff --git a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed index 4cd8ee4d6c..d80a32652a 100644 --- a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed +++ b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed @@ -286,3 +286,24 @@ for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++); if ($this->valid(fn(): bool => 2 > 1)) { } + +// Issue 3345. +function testMultiCatch() +{ + if (true) { + try { + } catch (\LogicException $e) { + } catch (\Exception $e) { + } + } +} + +function testFinally() +{ + if (true) { + try { + } catch (\LogicException $e) { + } finally { + } + } +} diff --git a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php index 92888b8581..a9c4f4f8ed 100644 --- a/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php +++ b/src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php @@ -71,6 +71,8 @@ public function getErrorList($testFile='InlineControlStructureUnitTest.1.inc') 236 => 1, 238 => 1, 242 => 1, + 260 => 1, + 269 => 1, ]; case 'InlineControlStructureUnitTest.js':