Permalink
Browse files

Squiz SwitchDeclarationSniff now allows RETURN statements to close a …

…CASE or DEFAULT statement
  • Loading branch information...
gsherwood committed Aug 16, 2012
1 parent d565a82 commit ff332a4c22b8125e3e79fbd4241a86fb5d29d486
@@ -125,13 +125,13 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
}
$nextBreak = $tokens[$nextCase]['scope_closer'];
- if ($tokens[$nextBreak]['code'] === T_BREAK) {
+ if ($tokens[$nextBreak]['code'] === T_BREAK || $tokens[$nextBreak]['code'] === T_RETURN) {
if ($tokens[$nextBreak]['scope_condition'] === $nextCase) {
// Only need to check a couple of things once, even if the
// break is shared between multiple case statements, or even
// the default case.
if ($tokens[$nextBreak]['column'] !== $caseAlignment) {
- $error = 'BREAK statement must be indented 4 spaces from SWITCH keyword';
+ $error = 'Case breaking statement must be indented 4 spaces from SWITCH keyword';
$phpcsFile->addError($error, $nextBreak, 'BreakIndent');
}
@@ -145,7 +145,7 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
}
if ($prevLine !== ($breakLine - 1)) {
- $error = 'Blank lines are not allowed before BREAK statements';
+ $error = 'Blank lines are not allowed before case breaking statements';
$phpcsFile->addError($error, $nextBreak, 'SpacingBeforeBreak');
}
@@ -163,13 +163,13 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
// Ensure the BREAK statement is followed by
// a single blank line, or the end switch brace.
if ($nextLine !== ($breakLine + 2) && $i !== $tokens[$stackPtr]['scope_closer']) {
- $error = 'BREAK statements must be followed by a single blank line';
+ $error = 'Case breaking statements must be followed by a single blank line';
$phpcsFile->addError($error, $nextBreak, 'SpacingAfterBreak');
}
} else {
// Ensure the BREAK statement is not followed by a blank line.
if ($nextLine !== ($breakLine + 1)) {
- $error = 'Blank lines are not allowed after the DEFAULT case\'s BREAK statement';
+ $error = 'Blank lines are not allowed after the DEFAULT case\'s breaking statement';
$phpcsFile->addError($error, $nextBreak, 'SpacingAfterDefaultBreak');
}
}
@@ -189,45 +189,49 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
}
}//end if
- if ($type === 'Case') {
- // Ensure empty CASE statements are not allowed.
- // They must have some code content in them. A comment is not enough.
- $foundContent = false;
- for ($i = ($tokens[$nextCase]['scope_opener'] + 1); $i < $nextBreak; $i++) {
- if ($tokens[$i]['code'] === T_CASE) {
- $i = $tokens[$i]['scope_opener'];
- continue;
+ if ($tokens[$nextBreak]['code'] === T_BREAK) {
+ if ($type === 'Case') {
+ // Ensure empty CASE statements are not allowed.
+ // They must have some code content in them. A comment is not enough.
+ // But count RETURN statements as valid content if they also
+ // happen to close the CASE statement.
+ $foundContent = false;
+ for ($i = ($tokens[$nextCase]['scope_opener'] + 1); $i < $nextBreak; $i++) {
+ if ($tokens[$i]['code'] === T_CASE) {
+ $i = $tokens[$i]['scope_opener'];
+ continue;
+ }
+
+ if (in_array($tokens[$i]['code'], PHP_CodeSniffer_Tokens::$emptyTokens) === false) {
+ $foundContent = true;
+ break;
+ }
}
- if (in_array($tokens[$i]['code'], PHP_CodeSniffer_Tokens::$emptyTokens) === false) {
- $foundContent = true;
- break;
+ if ($foundContent === false) {
+ $error = 'Empty CASE statements are not allowed';
+ $phpcsFile->addError($error, $nextCase, 'EmptyCase');
}
- }
-
- if ($foundContent === false) {
- $error = 'Empty CASE statements are not allowed';
- $phpcsFile->addError($error, $nextCase, 'EmptyCase');
- }
- } else {
- // Ensure empty DEFAULT statements are not allowed.
- // They must (at least) have a comment describing why
- // the default case is being ignored.
- $foundContent = false;
- for ($i = ($tokens[$nextCase]['scope_opener'] + 1); $i < $nextBreak; $i++) {
- if ($tokens[$i]['type'] !== 'T_WHITESPACE') {
- $foundContent = true;
- break;
+ } else {
+ // Ensure empty DEFAULT statements are not allowed.
+ // They must (at least) have a comment describing why
+ // the default case is being ignored.
+ $foundContent = false;
+ for ($i = ($tokens[$nextCase]['scope_opener'] + 1); $i < $nextBreak; $i++) {
+ if ($tokens[$i]['type'] !== 'T_WHITESPACE') {
+ $foundContent = true;
+ break;
+ }
}
- }
- if ($foundContent === false) {
- $error = 'Comment required for empty DEFAULT case';
- $phpcsFile->addError($error, $nextCase, 'EmptyDefault');
- }
+ if ($foundContent === false) {
+ $error = 'Comment required for empty DEFAULT case';
+ $phpcsFile->addError($error, $nextCase, 'EmptyDefault');
+ }
+ }//end if
}//end if
} else if ($type === 'Default') {
- $error = 'DEFAULT case must have a BREAK statement';
+ $error = 'DEFAULT case must have a breaking statement';
$phpcsFile->addError($error, $nextCase, 'DefaultNoBreak');
}//end if
}//end while
@@ -24,7 +24,7 @@ switch ($something) {
switch ($something) {
case '1':
$case = '1';
- break;
+ return '1';
case '2':
case '3':
@@ -177,7 +177,7 @@ switch ($name) {
break;
case "2":
- return true;
+ return true;
break;
default:
@@ -193,7 +193,7 @@ switch ($name2) {
break;
case "2":
- return true;
+ return true;
break;
default:
@@ -207,8 +207,7 @@ switch ($name) {
case "1":
switch ($name2) {
case "1":
- return true;
- break;
+ return true;
default:
// No default.
@@ -229,13 +228,32 @@ switch ($name2) {
switch ($foo) {
case "1":
- return true;
- break;
+ return true;
default:
if ($foo === FALSE) {
break(2);
}
break;
}
-?>
+
+// Valid SWITCH statement.
+switch ($something) {
+ case '1':
+ $case = '1';
+ return '1';
+
+ case '2':
+ case '3':
+ $case = '5';
+ return '2';
+
+ case '4':
+ $case = '4';
+ return '3';
+
+ default:
+ $case = null;
+ return '4';
+}
+?>
@@ -177,7 +177,7 @@ switch (name) {
break;
case "2":
- return true;
+ return true;
break;
default:
@@ -193,7 +193,7 @@ switch (name2) {
break;
case "2":
- return true;
+ return true;
break;
default:
@@ -207,8 +207,7 @@ switch (name) {
case "1":
switch (name2) {
case "1":
- return true;
- break;
+ return true;
default:
// No default.
@@ -229,12 +228,31 @@ switch (name2) {
switch (foo) {
case "1":
- return true;
- break;
+ return true;
default:
if (foo === false) {
break;
}
break;
-}
+}
+
+// Valid SWITCH statement.
+switch (something) {
+ case '1':
+ myvar = '1';
+ return '1';
+
+ case '2':
+ case '3':
+ myvar = '5';
+ return '2';
+
+ case '4':
+ myvar = '4';
+ return '3';
+
+ default:
+ myvar = null;
+ return '4';
+}
@@ -77,7 +77,11 @@ public function getErrorList($testFile='SwitchDeclarationUnitTest.inc')
147 => 1,
165 => 1,
172 => 1,
- 224 => 1,
+ 176 => 2,
+ 180 => 1,
+ 192 => 2,
+ 196 => 1,
+ 223 => 1,
);
}//end getErrorList()
View
@@ -35,8 +35,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
- Added report to show results using notify-send
-- Use --report=notifysend to generate the report
-- Thanks to Christian Weiske for the contribution
- - Fixed a PHP notice generated when loading custom array settings from a rulset.xml file
+ - The JS tokenizer now recognises RETURN as a valid closer for CASE and DEFAULT inside switch statements
- PEAR ValidFunctionNameSniff no longer enforces exact case matching for PHP magic methods
+ - Squiz SwitchDeclarationSniff now allows RETURN statements to close a CASE or DEFAULT statement
+ - Fixed a PHP notice generated when loading custom array settings from a rulset.xml file
- Fixed bug #19565 : Non-Executable Code Sniff Broken for Case Statements with both return and break
</notes>
<contents>

0 comments on commit ff332a4

Please sign in to comment.