[PEAR-BUG] Bug #20196 [Ana->Csd]: 1.5.2 breaks scope_closer position

Edit report at https://pear.php.net/bugs/bug.php?id=20196&edit=1

 ID:               20196
 Updated by:       [email protected]
 Reported By:      james dot titcumb at protected dot co dot uk
 Summary:          1.5.2 breaks scope_closer position
-Status:           Analyzed
+Status:           Closed
 Type:             Bug
 Package:          PHP_CodeSniffer
 Package Version:  1.5.2
 PHP Version:      5.4.22
 Assigned To:      squiz
 Roadmap Versions: 
 New Comment:

-Status: Analyzed
+Status: Closed
I've finally fixed the original issue here: 
https://github.com/squizlabs/PHP_CodeSniffer/commit/6568a825c7731410f73cc54ec57
9eb923e9b6ed2

I had to add a special case for this syntax for if/elseif/else/endif,
but the openers and 
closers are now correct.


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

[2014-02-19 23:35:01] squiz

None of the changes I've made has re-broken anything for me. I haven't
been able 
to fix the original issue, which is why your original code is still
broken.

The only way to write your statement so that PHPCS can detect it
properly is like 
this:

if ($foo):
    if ($bar):
        $foo = 1;
    elseif ($baz):
        $foo = 2;
    endif;
endif;

Because there is no ambiguity with the endif line, PHPCS can properly
match then. 
This was not working originally when you report the bug either, but I
have fixed 
this specific case.

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

[2014-02-14 09:48:28] asgrim

Hmm - it seems to have gone back to being broken now, I think perhaps
the ELSE problem was masking it.

The scope opener (the first "if") is still picking up the "elseif" as
it's scope closer.

The sniff is basically the PEAR.WhiteSpace.ScopeClosingBrace.Indent
sniff, but modified slightly to use tabs instead of spaces. So I thought
I'd change the code snippet I provided to spaces:

<?php
if ($foo):
    if ($bar) $foo = 1;
    elseif ($baz) $foo = 2;
endif;

and using the PEAR standard, sure enough I get the same error:

 5 | ERROR   | Closing brace indented incorrectly; expected 0 spaces,
found 4
   |         | (PEAR.WhiteSpace.ScopeClosingBrace.Indent)

Hopefully that will allow you to test a bit easier so we're on the same
page? :) (there's a few other errors from the PEAR standard, but that's
expected I think).

Just to note, that it's only really on L119 of
PEAR_Sniffs_WhiteSpace_ScopeClosingBraceSniff starts to differ
significantly in our modified sniff - so the stuff that is determining
where the scope_closer appears to be affecting that standard too I
think.

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

[2014-02-13 22:56:37] squiz

Fixed that case I was talking about here: 
https://github.com/squizlabs/PHP_CodeSniffer/commit/2679192dfe5a3550ddf04a8e1b
86bcb5934158c1

Let me know if you have any more code that has this problem.

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

[2014-02-13 22:47:08] squiz

You must be testing code like this:

if ($foo):
else if ($baz): $foo = 2;
endif;

Where the ELSE and IF are separated, instead of ELSEIF.

In this case, PHP_CodeSniffer sees that ELSE has to act as the end token
for the IF 
due to the alternate syntax being used, but it never finds a scope
opener for the 
ELSE because the next IF takes over and claims them for itself.

While it makes sense what is happening to me, I suspect it makes no
sense to 
anyone else :) So I'll see if I can restore the scope_condition index
and make the 
ELSE appear to be a simple closer for the previous IF.

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

[2014-02-13 12:26:24] asgrim

Thanks Greg - I tried out the latest from master (71526ae7) and this
fixes the problem, but I did notice an E_NOTICE (well, many of them).. I
don't know if it's related:

Notice: Undefined index: scope_condition

On this "if":

if ($tokens[$scopeEnd]['scope_condition'] !== $stackPtr) {
    return;
}

This only seems to happen when the scope ending token
($tokens[$scopeEnd]) is an "ELSE"... Not sure if this is intentional
behaviour? The check is pretty much exactly the same check as here
(https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/PEAR/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php#L78)
if that's any help?

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

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=20196

-- 
Edit this bug report at https://pear.php.net/bugs/bug.php?id=20196&edit=1