Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed bug #19616 : Nested switches cause false error in PSR2

  • Loading branch information...
commit 1499c3077914e678b30659ea8c5ddfc69954aceb 1 parent b899439
Greg Sherwood gsherwood authored
46 CodeSniffer/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php
View
@@ -67,13 +67,7 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
$caseCount = 0;
$foundDefault = false;
- while (($nextCase = $phpcsFile->findNext(array(T_CASE, T_DEFAULT, T_SWITCH), ($nextCase + 1), $switch['scope_closer'])) !== false) {
- // Skip nested SWITCH statements; they are handled on their own.
- if ($tokens[$nextCase]['code'] === T_SWITCH) {
- $nextCase = $tokens[$nextCase]['scope_closer'];
- continue;
- }
-
+ while (($nextCase = $this->_findNextCase($phpcsFile, ($nextCase + 1), $switch['scope_closer'])) !== false) {
if ($tokens[$nextCase]['code'] === T_DEFAULT) {
$type = 'default';
$foundDefault = true;
@@ -151,12 +145,7 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
// This case statement has content. If the next case or default comes
// before the closer, it means we dont have a terminating statement
// and instead need a comment.
- $nextCode = $phpcsFile->findNext(
- array(T_CASE, T_DEFAULT),
- ($tokens[$nextCase]['scope_opener'] + 1),
- $nextCloser
- );
-
+ $nextCode = $this->_findNextCase($phpcsFile, ($tokens[$nextCase]['scope_opener'] + 1), $nextCloser);
if ($nextCode !== false) {
$prevCode = $phpcsFile->findPrevious(T_WHITESPACE, ($nextCode - 1), $nextCase, true);
if ($tokens[$prevCode]['code'] !== T_COMMENT) {
@@ -164,12 +153,41 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
$phpcsFile->addError($error, $nextCase, 'TerminatingComment');
}
}
- }//end if
+ }
}//end while
}//end process()
+ /**
+ * Find the next CASE or DEFAULT statement from a point in the file.
+ *
+ * Note that nested switches are ignored.
+ *
+ * @param PHP_CodeSniffer_File $phpcsFile The file being scanned.
+ * @param int $stackPtr The position to start looking at.
+ * @param int $end The position to stop looking at.
+ *
+ * @return int | bool
+ */
+ private function _findNextCase(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $end)
Ton Sharp
66Ton99 added a note

Private method makes not possible to extend current Sniff.

Greg Sherwood Owner

This is not the sort of thing you need to override. It is just looking for the next CASE statement. So if you need to override this bit, you probably need to override the process() method instead, or just write a new sniff. If you have a reason for changing it, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $tokens = $phpcsFile->getTokens();
+ while (($stackPtr = $phpcsFile->findNext(array(T_CASE, T_DEFAULT, T_SWITCH), $stackPtr, $end)) !== false) {
+ // Skip nested SWITCH statements; they are handled on their own.
+ if ($tokens[$stackPtr]['code'] === T_SWITCH) {
+ $stackPtr = $tokens[$stackPtr]['scope_closer'];
+ continue;
+ }
+
+ break;
+ }
+
+ return $stackPtr;
+
+ }//end _findNextCase()
+
+
}//end class
?>
10 CodeSniffer/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc
View
@@ -57,4 +57,14 @@ switch (true) {
case is_object($value):
return 'object';
}
+
+switch (0) {
+ case 0:
+ switch (1) {
+ case 1:
+ echo 'a';
+ break;
+ }
+ break;
+}
?>
30 package.xml
View
@@ -17,8 +17,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<date>2012-09-26</date>
<time>14:13:00</time>
<version>
- <release>1.4.0</release>
- <api>1.4.0</api>
+ <release>1.4.1</release>
+ <api>1.4.1</api>
</version>
<stability>
<release>stable</release>
@@ -26,31 +26,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
</stability>
<license uri="http://matrix.squiz.net/developer/tools/php_cs/licence">BSD License</license>
<notes>
- - Added PSR1 and PSR2 coding standards that can be used to check your code against these guidelines
- - PHP 5.4 short array syntax is now detected and tokens are assigned to the open and close characters
- -- New tokens are T_OPEN_SHORT_ARRAY and T_CLOSE_SHORT_ARRAY as PHP does not define its own
- - Added the ability to explain a coding standard by listing the sniffs that it includes
- -- The sniff list includes all imported and native sniffs
- -- Explain a standard by using the -e and --standard=[standard] command line arguments
- -- E.g., phpcs -e --standard=Squiz
- -- Thanks to Ben Selby for the idea
- - Added report to show results using notify-send
- -- Use --report=notifysend to generate the report
- -- Thanks to Christian Weiske for the contribution
- - The JS tokenizer now recognises RETURN as a valid closer for CASE and DEFAULT inside switch statements
- - AbstractPatternSniff now sets the ignoreComments option using a public var rather than through the constructor
- -- This allows the setting to be overwritten in ruleset.xml files
- -- Old method remains for backwards compatibility
- - Generic LowerCaseConstantSniff and UpperCaseConstantSniff no longer report errors on classes named True, False or Null
- - 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
- - Squiz BlockCommentSniff now correctly reports an error for blank lines before blocks at the start of a control structure
- - Fixed a PHP notice generated when loading custom array settings from a rulset.xml file
- - Fixed bug #17908 : CodeSniffer does not recognise optional @params
- -- Thanks to Pete Walker for the patch
- - Fixed bug #19538 : Function indentation code sniffer checks inside short arrays
- - Fixed bug #19565 : Non-Executable Code Sniff Broken for Case Statements with both return and break
- - Fixed bug #19612 : Invalid @package suggestion
+ - Fixed bug #19616 : Nested switches cause false error in PSR2
</notes>
<contents>
<dir name="/">
Ton Sharp

Private method makes not possible to extend current Sniff.

Greg Sherwood

This is not the sort of thing you need to override. It is just looking for the next CASE statement. So if you need to override this bit, you probably need to override the process() method instead, or just write a new sniff. If you have a reason for changing it, let me know.

Please sign in to comment.
Something went wrong with that request. Please try again.