Auto-fixing issue encountered with inconsistent use of braces

[jedi58]

The below example is taken from ADOdb/ADOdb's replicate include:

    foreach ($sql as $s)
        if (!$this->execute) echo "<pre>",$s.";\n</pre>";
        else {
            $ok = $this->connDest->Execute($s);
            if (!$ok)
                if ($this->neverAbort) $ret = false;
                else return false;
        }

In this example, what causes the issue is the lack of braces. When phpcbf encounters the for, and then the else, it is putting braces around the block it believes to be the for-statement and then around the if but then moves the else to become part of the for which will cause a fatal error in PHP. The result of running phpcbf --standard=PSR2 is:

        foreach ($sql as $s) {
            if (!$this->execute) {
                echo "<pre>",$s.";\n</pre>";
            }
        } else {
            $ok = $this->connDest->Execute($s);
            if (!$ok) {
                if ($this->neverAbort) {
                    $ret = false;
                }
            } else {
                return false;
            }
        }

[aik099]

1. which version of PHP_CodeSniffer you're using?
2. does it work if you use master branch?

[jedi58]

PHP_CodeSniffer version 2.5.1 (stable) by Squiz (http://www.squiz.net)

installed using curl -OL https://squizlabs.github.io/PHP_CodeSniffer/phpcbf.phar

will retest from master 😄

[jedi58]

yes, this happens when testing from master as well

Admittedly it's pretty horrible that the code was missing the braces in the first place!

[gsherwood]

I have this horrible feeling that this is going to be a tough one :(

[gsherwood]

The error here is actually the very first correction (well, second, technically) in the very first loop, where it incorrectly places the closing brace for the foreach.

$ phpcs temp.php --standard=Generic --report=diff -vvv --sniffs=Generic.ControlStructures.InlineControlStructure
<...snip...>
---START FILE CONTENT---
 1|<?php
 2|foreach ($sql as $s)
 3|        if (!$this->execute) echo "<pre>",$s.";\n</pre>";
 4|        else {
 5|            $ok = $this->connDest->Execute($s);
 6|            if (!$ok)
 7|                if ($this->neverAbort) $ret = false;
 8|                else return false;
 9|        }
10|
--- END FILE CONTENT ---
    E: [Line 2] Inline control structures are not allowed (Generic.ControlStructures.InlineControlStructure.NotAllowed)
    => Changeset started by Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 142)
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 152) replaced token 9 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 227) replaced token 28 (T_SEMICOLON) ";" => ";·}"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 9 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 28 (T_SEMICOLON) ";" => ";·}"
    => Changeset ended: 2 changes applied
    E: [Line 3] Inline control structures are not allowed (Generic.ControlStructures.InlineControlStructure.NotAllowed)
    => Changeset started by Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 142)
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 152) replaced token 19 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 227) replaced token 28 (T_SEMICOLON) ";·}" => ";·}·}"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 19 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        * token 28 has already been modified, skipping *
        R: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) reverted token 19 (T_CLOSE_PARENTHESIS) ")·{" => ")"
    => Changeset failed to apply
    E: [Line 6] Inline control structures are not allowed (Generic.ControlStructures.InlineControlStructure.NotAllowed)
    => Changeset started by Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 142)
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 152) replaced token 56 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 227) replaced token 72 (T_SEMICOLON) ";" => ";·}"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 56 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 72 (T_SEMICOLON) ";" => ";·}"
    => Changeset ended: 2 changes applied
    E: [Line 7] Inline control structures are not allowed (Generic.ControlStructures.InlineControlStructure.NotAllowed)
    => Changeset started by Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 142)
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 152) replaced token 65 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 227) replaced token 72 (T_SEMICOLON) ";·}" => ";·}·}"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 65 (T_CLOSE_PARENTHESIS) ")" => ")·{"
        * token 72 has already been modified, skipping *
        R: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) reverted token 65 (T_CLOSE_PARENTHESIS) ")·{" => ")"
    => Changeset failed to apply
    E: [Line 8] Inline control structures are not allowed (Generic.ControlStructures.InlineControlStructure.NotAllowed)
    => Changeset started by Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 142)
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 152) replaced token 75 (T_ELSE) "else" => "else·{"
        Q: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 227) replaced token 80 (T_SEMICOLON) ";" => ";·}"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 75 (T_ELSE) "else" => "else·{"
        A: Generic_Sniffs_ControlStructures_InlineControlStructureSniff (line 239) replaced token 80 (T_SEMICOLON) ";" => ";·}"
    => Changeset ended: 2 changes applied
    * fixed 6 violations, starting loop 2 *
---START FILE CONTENT---
 1|<?php
 2|foreach ($sql as $s) {
 3|        if (!$this->execute) echo "<pre>",$s.";\n</pre>"; }
 4|        else {
 5|            $ok = $this->connDest->Execute($s);
 6|            if (!$ok) {
 7|                if ($this->neverAbort) $ret = false; }
 8|                else { return false; }
 9|        }
10|
--- END FILE CONTENT ---
<...snip...>

[gsherwood]

Well, that wasn't as hard as I thought it would be.

Silicon Valley to the rescue. The answer was "middle-out".

The important change here is to fix these inline control structures from the middle-out. i.e., from the inner-most structure to the outer-most. Previously, they were fixed as they were encountered. Now, the fixer bails out if it sees a nested inline control structure before is finds the end point.

After this change, the posted code is fixed like this:

foreach ($sql as $s) {
    if (!$this->execute) {
        echo "<pre>",$s.";\n</pre>";
    } else {
        $ok = $this->connDest->Execute($s);
        if (!$ok) {
            if ($this->neverAbort) {
                $ret = false;
            } else {
                return false;
            }
        }
    }
}