From 6b106cc18458f14c95ff6e6e435f61926c285c5c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 27 Nov 2024 10:36:56 +0100 Subject: [PATCH] Generic/OpeningFunctionBraceKernighanRitchie: improve error message Follow up on 707 When there are multiple tabs between the end of the function signature and the open brace, the "found: $length" part of the error message would refer to the number of tabs found. This is inconsistent with other sniffs and confusing. This commit fixes this, while still maintaining the previous behaviour - as introduced via https://github.com/squizlabs/PHP_CodeSniffer/commit/c4b9807cdb661635b71d5c1950e05c5189904a70 - of special casing the messaging for a single tab - independently of whether or not tab replacement is in effect. Example for line 10 of test case file 2: ``` // Before: Expected 1 space before opening brace; found 3 // After: Expected 1 space before opening brace; found 11 ``` Covered via pre-existing tests in both test case files + some additional new tests. --- ...ningFunctionBraceKernighanRitchieSniff.php | 20 +++++++++++-------- ...unctionBraceKernighanRitchieUnitTest.2.inc | 8 ++++++++ ...nBraceKernighanRitchieUnitTest.2.inc.fixed | 8 ++++++++ ...gFunctionBraceKernighanRitchieUnitTest.php | 2 ++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php index 1e2df37d86..d5a84982cd 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php @@ -138,22 +138,26 @@ public function process(File $phpcsFile, $stackPtr) return; } - // We are looking for tabs, even if they have been replaced, because - // we enforce a space here. - if (isset($tokens[($openingBrace - 1)]['orig_content']) === true) { - $spacing = $tokens[($openingBrace - 1)]['orig_content']; - } else { - $spacing = $tokens[($openingBrace - 1)]['content']; - } - + // Enforce a single space. Tabs not allowed. + $spacing = $tokens[($openingBrace - 1)]['content']; if ($tokens[($openingBrace - 1)]['code'] !== T_WHITESPACE) { $length = 0; } else if ($spacing === "\t") { + // Tab without tab-width set, so no tab replacement has taken place. $length = '\t'; } else { $length = strlen($spacing); } + // If tab replacement is on, avoid confusing the user with a "expected 1 space, found 1" + // message when the "1" found is actually a tab, not a space. + if ($length === 1 + && isset($tokens[($openingBrace - 1)]['orig_content']) === true + && $tokens[($openingBrace - 1)]['orig_content'] === "\t" + ) { + $length = '\t'; + } + if ($length !== 1) { $error = 'Expected 1 space before opening brace; found %s'; $data = [$length]; diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc index a145bb9efe..36b5e8187d 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc @@ -9,3 +9,11 @@ function myFunction() { // Uses three tabs. function myFunction() { } + +// Uses one tab in a way that it translates to exactly one space with tab replacement. +function oneT() { +} + +// Mixed tabs and spaces. +function mixed() { +} diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed index 515a93e50a..a3c47435de 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed @@ -9,3 +9,11 @@ function myFunction() { // Uses three tabs. function myFunction() { } + +// Uses one tab in a way that it translates to exactly one space with tab replacement. +function oneT() { +} + +// Mixed tabs and spaces. +function mixed() { +} diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php index 0c1a99300a..747d54d84c 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php @@ -88,6 +88,8 @@ public function getErrorList($testFile='') return [ 6 => 1, 10 => 1, + 14 => 1, + 18 => 1, ]; default: return [];