[PEAR-BUG] Bug #20432 [Fbk->Csd]: Switch statements with inline returns

[email protected] Tue, 28 Oct 2014 23:58:21 +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
+Status:           Closed
 Type:             Bug
 Package:          PHP_CodeSniffer
 Package Version:  1.5.5
 PHP Version:      5.6.0
 Assigned To:      squiz
 Roadmap Versions: 
 New Comment:

-Status: Feedback
+Status: Closed
> But I don't think you should have to remove the indentation as the PHP
Manual 
includes them.

The PEAR coding standard, which is the default for PHP_CodeSniffer, is
very clear 
about SWITCH statements:
http://pear.php.net/manual/en/standards.control.php

I didn't write this standard, so there isn't any point debating it with
me. I'm on your 
side anyway; I don't like writing SWITCH statements like this. That's
why I use a 
different coding standard for my other projects.

I'm going to close this because everything is working as intended. You
just don't 
agree with the coding standard, which is absolutely fine. You will just
need to 
customise your own standard a bit more to remove these extra parts of
the PEAR 
standard that you don't like.


Previous Comments:
------------------------------------------------------------------------

[2014-10-28 17:15:56] xmeltrut

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.

------------------------------------------------------------------------

[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.

------------------------------------------------------------------------

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