Skip to content

Commit aae6378

Browse files
gsherwoodgsherwood
committed
Fixed bug #621 : PSR2 SwitchDeclaration sniff doesn't detect, or correctly fix, case body on same line as statement
1 parent 0524c69 commit aae6378

File tree

5 files changed

+84
-20
lines changed

5 files changed

+84
-20
lines changed

‎CodeSniffer/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,29 +121,63 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
121121
$phpcsFile->fixer->replaceToken(($opener - 1), '');
122122
}
123123
}
124+
125+
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
126+
if ($tokens[$next]['line'] !== ($tokens[$opener]['line'] + 1)) {
127+
$error = 'The '.strtoupper($type).' body must start on the line following the statement';
128+
$fix = $phpcsFile->addFixableError($error, $nextCase, 'SpaceBeforeColon'.strtoupper($type));
129+
if ($fix === true) {
130+
if ($tokens[$next]['line'] === $tokens[$opener]['line']) {
131+
$padding = str_repeat(' ', ($caseAlignment + $this->indent - 1));
132+
$phpcsFile->fixer->addContentBefore($next, $phpcsFile->eolChar.$padding);
133+
} else {
134+
$phpcsFile->fixer->beginChangeset();
135+
for ($i = ($opener + 1); $i < $next; $i++) {
136+
if ($tokens[$i]['line'] === $tokens[$next]['line']) {
137+
break;
138+
}
139+
140+
$phpcsFile->fixer->replaceToken($i, '');
141+
}
142+
143+
$phpcsFile->fixer->addNewLineBefore($i);
144+
$phpcsFile->fixer->endChangeset();
145+
}
146+
}
147+
}//end if
124148
} else {
125149
$error = strtoupper($type).' statements must be defined using a colon';
126150
$phpcsFile->addError($error, $nextCase, 'WrongOpener'.$type);
127-
}
151+
}//end if
128152

129153
$nextCloser = $tokens[$nextCase]['scope_closer'];
130154
if ($tokens[$nextCloser]['scope_condition'] === $nextCase) {
131155
// Only need to check some things once, even if the
132156
// closer is shared between multiple case statements, or even
133157
// the default case.
134-
$diff = ($caseAlignment + $this->indent - $tokens[$nextCloser]['column']);
135-
if ($diff !== 0) {
136-
$error = 'Terminating statement must be indented to the same level as the CASE body';
137-
$fix = $phpcsFile->addFixableError($error, $nextCloser, 'BreakIndent');
158+
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($nextCloser - 1), $nextCase, true);
159+
if ($tokens[$prev]['line'] === $tokens[$nextCloser]['line']) {
160+
$error = 'Terminating statement must be on a line by itself';
161+
$fix = $phpcsFile->addFixableError($error, $nextCloser, 'BreakNotNewLine');
138162
if ($fix === true) {
139-
if ($diff > 0) {
140-
$phpcsFile->fixer->addContentBefore($nextCloser, str_repeat(' ', $diff));
141-
} else {
142-
$phpcsFile->fixer->substrToken(($nextCloser - 1), 0, $diff);
163+
$phpcsFile->fixer->addNewLine($prev);
164+
$phpcsFile->fixer->replaceToken($nextCloser, trim($tokens[$nextCloser]['content']));
165+
}
166+
} else {
167+
$diff = ($caseAlignment + $this->indent - $tokens[$nextCloser]['column']);
168+
if ($diff !== 0) {
169+
$error = 'Terminating statement must be indented to the same level as the CASE body';
170+
$fix = $phpcsFile->addFixableError($error, $nextCloser, 'BreakIndent');
171+
if ($fix === true) {
172+
if ($diff > 0) {
173+
$phpcsFile->fixer->addContentBefore($nextCloser, str_repeat(' ', $diff));
174+
} else {
175+
$phpcsFile->fixer->substrToken(($nextCloser - 1), 0, $diff);
176+
}
143177
}
144178
}
145-
}
146-
}
179+
}//end if
180+
}//end if
147181

148182
// We only want cases from here on in.
149183
if ($type !== 'case') {

‎CodeSniffer/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,13 @@ function test()
103103

104104
exit;
105105
}
106+
107+
switch ($foo) {
108+
case 1: $bar = 1; break;
109+
case 2:
110+
111+
$bar = 2; break;
112+
case 21:
113+
case 3: return 3;
114+
default: $bar = 0;
115+
}

‎CodeSniffer/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,17 @@ function test()
103103

104104
exit;
105105
}
106+
107+
switch ($foo) {
108+
case 1:
109+
$bar = 1;
110+
break;
111+
case 2:
112+
$bar = 2;
113+
break;
114+
case 21:
115+
case 3:
116+
return 3;
117+
default:
118+
$bar = 0;
119+
}

‎CodeSniffer/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,20 @@ class PSR2_Tests_ControlStructures_SwitchDeclarationUnitTest extends AbstractSni
4141
public function getErrorList()
4242
{
4343
return array(
44-
10 => 1,
45-
11 => 1,
46-
14 => 1,
47-
16 => 1,
48-
20 => 1,
49-
23 => 1,
50-
29 => 1,
51-
33 => 1,
52-
37 => 2,
44+
10 => 1,
45+
11 => 1,
46+
14 => 1,
47+
16 => 1,
48+
20 => 1,
49+
23 => 1,
50+
29 => 1,
51+
33 => 1,
52+
37 => 2,
53+
108 => 2,
54+
109 => 1,
55+
111 => 1,
56+
113 => 2,
57+
114 => 1,
5358
);
5459

5560
}//end getErrorList()

‎package.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
5050
- Fixed bug #615 : Squiz OperatorBracketSniff incorrectly reports and fixes operations using self::
5151
- Fixed bug #616 : Squiz DisallowComparisonAssignmentSniff inconsistent errors with inline IF statements
5252
- Fixed bug #617 : Space after switch keyword in PSR-2 is not being enforced
53+
- Fixed bug #621 : PSR2 SwitchDeclaration sniff doesn't detect, or correctly fix, case body on same line as statement
5354
</notes>
5455
<contents>
5556
<dir name="/">

0 commit comments

Comments
 (0)