[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