Conditional with missing braces confused by indirect variables

[manderson]

I've boiled down my issue to the following code snippet:

if ($this) {
    if ($that)
        foo(${$a[$b]});
}

When I run phpcs --standard=PSR2 filename.php it identifies 3 errors and suggests phpcbf can fix all 3. When I run phpcbf on it the following is the result:

if ($this) {
    if ($that) {
        foo(
            $$a[$b]
    }
        );
}

This is not equivalent to the original code and is not even syntactically correct.

I've tried several variants of the starting code and it seems to require an outer block (could be a conditional as above or a function declaration), the inner conditional must have an inline statement, the inline statement must be on a separate line, and the inline statement must indirectly access a variable using braces to make the intent explicit.

One more quirk is if you change the foo(...) call to a return(...) the result is formatted differently but the syntax is flawed in the same way.

I'm not familiar with all of the code sniffs that are being used and how they interact but it seems clear the inner conditional incorrectly finds its missing braces in the ${$a[$b]} statement.

If you need any other details please let me know.

[manderson]

It's been a week with no response, what should be my expectations? This is a definite repeatable issue where some valid code is beautified into invalid syntax.

I'm not trying to be a jerk, I'm really not. I'm just curious if there's something else I should be doing to help move this along other than digging in and attempting to solve the issue myself. I'm far too unfamiliar with all of this to attempt doing that.

Thanks for any help anyone can offer.

[gsherwood]

I've seen your report and it's on my list of issues to review. I've just come back from 3 weeks being ill and my backlog is huge. I'm also catching up on my actual paying job after being away so long, so I've had very little time for PHPCS. What time I have is going into releasing the already late 3.2.3 version, which a fix for this issue will not make it into.

Typically, anything that involves tokenizer issues because people have left out braces for control structures tends to be pretty tricky to resolve, so I wont get a chance to even look into this until after the next release or it will use up all my time.

[jrfnl]

Just in case you need more example code/test cases, at WPCS we received a report about the same issue: WordPress/WordPress-Coding-Standards#1321

[gsherwood]

Debug output for the sample code showing how the curly brace of the variable is being incorrectly assigned as the opening brace of the second IF:

*** START SCOPE MAP ***
Start scope map at 1:T_IF => if
=> Begin scope map recursion at token 1 with depth 1
Process token 2 on line 2 []: T_WHITESPACE => ·
Process token 3 on line 2 []: T_OPEN_PARENTHESIS => (
Process token 4 on line 2 []: T_VARIABLE => $this
Process token 5 on line 2 []: T_CLOSE_PARENTHESIS => )
Process token 6 on line 2 []: T_WHITESPACE => ·
Process token 7 on line 2 []: T_OPEN_CURLY_BRACKET => {
=> Found scope opener for 1:T_IF
Process token 8 on line 2 [opener:7;]: T_WHITESPACE => \n
Process token 9 on line 3 [opener:7;]: T_WHITESPACE => ····
Process token 10 on line 3 [opener:7;]: T_IF => if
* token is an opening condition *
* searching for opener *
    => Begin scope map recursion at token 10 with depth 2
    Process token 11 on line 3 []: T_WHITESPACE => ·
    Process token 12 on line 3 []: T_OPEN_PARENTHESIS => (
    Process token 13 on line 3 []: T_VARIABLE => $that
    Process token 14 on line 3 []: T_CLOSE_PARENTHESIS => )
    Process token 15 on line 3 []: T_WHITESPACE => \n
    Process token 16 on line 4 []: T_WHITESPACE => ········
    Process token 17 on line 4 []: T_STRING => foo
    Process token 18 on line 4 []: T_OPEN_PARENTHESIS => (
    Process token 19 on line 4 []: T_DOLLAR => $
    Process token 20 on line 4 []: T_OPEN_CURLY_BRACKET => {
    => Found scope opener for 10:T_IF
    Process token 21 on line 4 [opener:20;]: T_VARIABLE => $a
    Process token 22 on line 4 [opener:20;]: T_OPEN_SQUARE_BRACKET => [
    Process token 23 on line 4 [opener:20;]: T_VARIABLE => $b
    Process token 24 on line 4 [opener:20;]: T_CLOSE_SQUARE_BRACKET => ]
    Process token 25 on line 4 [opener:20;]: T_CLOSE_CURLY_BRACKET => }
    => Found scope closer (25:T_CLOSE_CURLY_BRACKET) for 10:T_IF
Process token 26 on line 4 [opener:7;]: T_CLOSE_PARENTHESIS => )
Process token 27 on line 4 [opener:7;]: T_SEMICOLON => ;
Process token 28 on line 4 [opener:7;]: T_WHITESPACE => \n
Process token 29 on line 5 [opener:7;]: T_CLOSE_CURLY_BRACKET => }
=> Found scope closer (29:T_CLOSE_CURLY_BRACKET) for 1:T_IF
*** END SCOPE MAP ***

[gsherwood]

This turned out to be easier than I thought as there was already code to ignore braces for code like this, but it didn't support ${varname} syntax; it was expecting $varname{2} like a string index.

Thanks for reporting this.