[PEAR-BUG] Bug #20432 [Fbk]: Switch statements with inline returns
[email protected] Tue, 28 Oct 2014 17:15:56 +0000 (GMT)
| Newsgroups | php.pear.bugs |
|---|---|
| Message-ID | <[email protected]> |
Edit report at https://pear.php.net/bugs/bug.php?id=20432&edit=1 ID: 20432 Updated by: [email protected] Reported By: xmeltrut at gmail dot com Summary: Switch statements with inline returns Status: Feedback Type: Bug Package: PHP_CodeSniffer Package Version: 1.5.5 PHP Version: 5.6.0 Assigned To: squiz Roadmap Versions: New Comment: Yes, to clarify, I think it is a problem with the sniffs, but these are built in aren't they? You're right that if you remove the intend of the case lines as you have done in your example, then it doesn't show a warning. But I don't think you should have to remove the indentation as the PHP Manual includes them. I guess you could also argue that it is a coding standard, and so there is no right or wrong, however, if you do with with standard breaks, you don't get the same issue you get when using a return statement. Previous Comments: ------------------------------------------------------------------------ [2014-10-27 21:54:54] squiz My testing indicates that all the sniffs are working as expected and that your problem does not actually appear to be a problem with PHP_CodeSniffer itself but rather an issue caused by 2 sniffs working together to enforce the PEAR coding standard. The PEAR coding standard wants you to do this: <?php function test ($var) { switch ($var) { case 'x': return '1'; case 'y': return '2'; } } If you don't like to write code like that (I personally don't) then you have to exclude both the ScopeIndent sniff and the ScopeClosingBrace sniff because they are enforcing that same CASE/DEFAULT indent rule. You may prefer to include the Squiz.ControlStructures.SwitchDeclaration and Squiz.WhiteSpace.ScopeClosingBrace sniffs instead, which might format switch statements in the way you want. Does this make sense or is there a software bug I'm missing in your report? ------------------------------------------------------------------------ [2014-10-27 17:40:25] xmeltrut Test.php: <?php function test ($var) { switch ($var) { case 'x': return '1'; case 'y': return '2'; } } With the command: phpcs --standard=PEAR -s test.php Generates: -------------------------------------------------------------------------------- FOUND 6 ERROR(S) AFFECTING 5 LINE(S) -------------------------------------------------------------------------------- 2 | ERROR | Missing file doc comment (PEAR.Commenting.FileComment.Missing) 2 | ERROR | Missing function doc comment | | (PEAR.Commenting.FunctionComment.Missing) 5 | ERROR | Line indented incorrectly; expected 4 spaces, found 8 | | (PEAR.WhiteSpace.ScopeIndent.Incorrect) 6 | ERROR | Closing brace indented incorrectly; expected 8 spaces, found 12 | | (PEAR.WhiteSpace.ScopeClosingBrace.Indent) 7 | ERROR | Line indented incorrectly; expected 4 spaces, found 8 | | (PEAR.WhiteSpace.ScopeIndent.Incorrect) 8 | ERROR | Closing brace indented incorrectly; expected 8 spaces, found 12 | | (PEAR.WhiteSpace.ScopeClosingBrace.Indent) -------------------------------------------------------------------------------- We normally run it with this ruleset instead: <?xml version="1.0"?> <ruleset> <rule ref="PEAR"> <exclude name="PEAR.WhiteSpace.ScopeIndent" /> </rule> <rule ref="Generic.WhiteSpace.ScopeIndent"> <properties> <property name="indent" value="4" /> </properties> </rule> </ruleset> So with the command: phpcs --standard=ruleset.xml -s test.php I get: -------------------------------------------------------------------------------- FOUND 4 ERROR(S) AFFECTING 3 LINE(S) -------------------------------------------------------------------------------- 2 | ERROR | Missing file doc comment (PEAR.Commenting.FileComment.Missing) 2 | ERROR | Missing function doc comment | | (PEAR.Commenting.FunctionComment.Missing) 6 | ERROR | Closing brace indented incorrectly; expected 8 spaces, found 12 | | (PEAR.WhiteSpace.ScopeClosingBrace.Indent) 8 | ERROR | Closing brace indented incorrectly; expected 8 spaces, found 12 | | (PEAR.WhiteSpace.ScopeClosingBrace.Indent) -------------------------------------------------------------------------------- ------------------------------------------------------------------------ [2014-10-25 01:19:25] squiz Thanks, but as I said before, I can't replicate any error with your code. Are you able to provide a sample file that shows the problem, and also show me the output of PHP_CodeSniffer (please use the -s command line argument so I can see where the error messages come from). ------------------------------------------------------------------------ [2014-10-24 20:08:47] xmeltrut I just added the break to see if that fixed it, originally I was just using return and I had the same issue. ------------------------------------------------------------------------ [2014-10-24 13:21:39] squiz -Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz I can't replicate the closing brace error, but I can tell you that the RETURN statement is actually the breaking statement of the case. Your BREAK line is unreachable code and should be removed. PHP_CodeSniffer recognises the RETURN as the closing statement for reach CASE, so you may be getting indentation errors because of that. So you'd write the code like this: switch { case 'a': return 'a'; case 'b': return 'b'; } Does that remove your error? If not, I'd need to see code around the switch statement because I can't replicate any indent errors with the switch statement you've provided. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://pear.php.net/bugs/bug.php?id=20432 -- Edit this bug report at https://pear.php.net/bugs/bug.php?id=20432&edit=1