Skip to content

Commit 6b106cc

Browse files
committed
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 squizlabs/PHP_CodeSniffer@c4b9807 - 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.
1 parent 941a00e commit 6b106cc

File tree

4 files changed

+30
-8
lines changed

4 files changed

+30
-8
lines changed

src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,22 +138,26 @@ public function process(File $phpcsFile, $stackPtr)
138138
return;
139139
}
140140

141-
// We are looking for tabs, even if they have been replaced, because
142-
// we enforce a space here.
143-
if (isset($tokens[($openingBrace - 1)]['orig_content']) === true) {
144-
$spacing = $tokens[($openingBrace - 1)]['orig_content'];
145-
} else {
146-
$spacing = $tokens[($openingBrace - 1)]['content'];
147-
}
148-
141+
// Enforce a single space. Tabs not allowed.
142+
$spacing = $tokens[($openingBrace - 1)]['content'];
149143
if ($tokens[($openingBrace - 1)]['code'] !== T_WHITESPACE) {
150144
$length = 0;
151145
} else if ($spacing === "\t") {
146+
// Tab without tab-width set, so no tab replacement has taken place.
152147
$length = '\t';
153148
} else {
154149
$length = strlen($spacing);
155150
}
156151

152+
// If tab replacement is on, avoid confusing the user with a "expected 1 space, found 1"
153+
// message when the "1" found is actually a tab, not a space.
154+
if ($length === 1
155+
&& isset($tokens[($openingBrace - 1)]['orig_content']) === true
156+
&& $tokens[($openingBrace - 1)]['orig_content'] === "\t"
157+
) {
158+
$length = '\t';
159+
}
160+
157161
if ($length !== 1) {
158162
$error = 'Expected 1 space before opening brace; found %s';
159163
$data = [$length];

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ function myFunction() {
99
// Uses three tabs.
1010
function myFunction() {
1111
}
12+
13+
// Uses one tab in a way that it translates to exactly one space with tab replacement.
14+
function oneT() {
15+
}
16+
17+
// Mixed tabs and spaces.
18+
function mixed() {
19+
}

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ function myFunction() {
99
// Uses three tabs.
1010
function myFunction() {
1111
}
12+
13+
// Uses one tab in a way that it translates to exactly one space with tab replacement.
14+
function oneT() {
15+
}
16+
17+
// Mixed tabs and spaces.
18+
function mixed() {
19+
}

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public function getErrorList($testFile='')
8888
return [
8989
6 => 1,
9090
10 => 1,
91+
14 => 1,
92+
18 => 1,
9193
];
9294
default:
9395
return [];

0 commit comments

Comments
 (0)