From 2569dd9c6b2c50cbdce0325d90978e0ed546db34 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 23 Jul 2026 13:58:35 +0300 Subject: [PATCH] Fix ControlStructureSpacing sniff requiring blank line around "finally" The sniff was forked from Squiz.WhiteSpace.ControlStructureSpacing in 2014, before upstream added finally handling in 2018 (fixing the same NoLineAfterClose bug for itself). This fork never picked up that fix, so try/finally and try/catch/finally blocks were treated as ordinary trailing code rather than a continuation of the try/catch chain, wrongly demanding (and auto-inserting) a blank line before finally. Mirrors the existing T_CATCH handling: registers T_FINALLY, adds an isFinally() helper, and exempts finally in both the leading- and trailing-content adjacency checks. Fixes #113 --- .../ControlStructureSpacingSniff.php | 18 +++++++++ .../ControlStructureSpacingUnitTest.1.inc | 40 +++++++++++++++++++ ...ontrolStructureSpacingUnitTest.1.inc.fixed | 38 ++++++++++++++++++ .../ControlStructureSpacingUnitTest.php | 4 ++ 4 files changed, 100 insertions(+) diff --git a/CodingStandard/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php b/CodingStandard/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php index 7dc94bf..36b114e 100644 --- a/CodingStandard/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php +++ b/CodingStandard/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php @@ -78,6 +78,7 @@ public function register() T_ELSEIF, T_TRY, T_CATCH, + T_FINALLY, ); }//end register() @@ -318,6 +319,7 @@ protected function checkLeadingContent(File $phpcsFile, $stackPtr) || $this->insideSwitchCase($phpcsFile, $leadingContent) === true || ($this->elseOrElseIf($phpcsFile, $stackPtr) === true && $this->ifOrElseIf($phpcsFile, $leadingContent) === true) || ($this->isCatch($phpcsFile, $stackPtr) === true && $this->isTryOrCatch($phpcsFile, $leadingContent) === true) + || ($this->isFinally($phpcsFile, $stackPtr) === true && $this->isTryOrCatch($phpcsFile, $leadingContent) === true) ) { if ($this->isFunction($phpcsFile, $leadingContent) === true) { // The previous content is the opening brace of a function @@ -503,6 +505,7 @@ protected function checkTrailingContent(File $phpcsFile, $stackPtr) // Code on the next line after control structure scope closer. if ($this->elseOrElseIf($phpcsFile, $trailingContent) === true || $this->isCatch($phpcsFile, $trailingContent) === true + || $this->isFinally($phpcsFile, $trailingContent) === true ) { return; } @@ -739,6 +742,21 @@ protected function isCatch(File $phpcsFile, $stackPtr) }//end isCatch() + /** + * Detects, that it is a closing brace of FINALLY. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return bool + */ + protected function isFinally(File $phpcsFile, $stackPtr) + { + return $this->isScopeCondition($phpcsFile, $stackPtr, T_FINALLY); + }//end isFinally() + + /** * Determines that a function is located at given position. * diff --git a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc index a6cb827..22e3683 100644 --- a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc +++ b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc @@ -1691,4 +1691,44 @@ catch ( Exception1 $e ) { $c = 'd'; } +// TRY/FINALLY (no blank line required before/after "finally"). +try { + $a = 'b'; +} +finally { + $c = 'd'; +} + +// TRY/CATCH/FINALLY (no blank line required before/after "finally"). +try { + $a = 'b'; +} +catch ( Exception $e ) { + $c = 'd'; +} +finally { + $c = 'd'; +} + +// TRY/FINALLY (blank line before "finally" is not allowed). +try { + $a = 'b'; +} + +finally { + $c = 'd'; +} + +// TRY/CATCH/FINALLY (blank line before "finally" is not allowed). +try { + $a = 'b'; +} +catch ( Exception $e ) { + $c = 'd'; +} + +finally { + $c = 'd'; +} + ?> diff --git a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc.fixed b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc.fixed index 0528d4c..a5ea193 100644 --- a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc.fixed +++ b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.1.inc.fixed @@ -1665,4 +1665,42 @@ catch ( Exception1 $e ) { $c = 'd'; } +// TRY/FINALLY (no blank line required before/after "finally"). +try { + $a = 'b'; +} +finally { + $c = 'd'; +} + +// TRY/CATCH/FINALLY (no blank line required before/after "finally"). +try { + $a = 'b'; +} +catch ( Exception $e ) { + $c = 'd'; +} +finally { + $c = 'd'; +} + +// TRY/FINALLY (blank line before "finally" is not allowed). +try { + $a = 'b'; +} +finally { + $c = 'd'; +} + +// TRY/CATCH/FINALLY (blank line before "finally" is not allowed). +try { + $a = 'b'; +} +catch ( Exception $e ) { + $c = 'd'; +} +finally { + $c = 'd'; +} + ?> diff --git a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php index 02f59df..4d5d5f6 100644 --- a/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php +++ b/CodingStandard/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php @@ -297,6 +297,10 @@ public function getErrorList($testFile) // Sequential catch statements not indented right. 1690 => 1, + + // Blank line before "finally" is not allowed. + 1718 => 1, + 1730 => 1, ); } elseif ($testFile === 'ControlStructureSpacingUnitTest.2.inc') { return array(