Php-cs-fixer: wrong indentation after comment in if statement

[ghispi]

All leading whitespaces are tabs.
Fixer is run with psr2 level.
As a result echo '1'; is not indented properly.
If I run fixer with config (listed below) then tab in echo line is not converted and line is not properly indented.

Also fixer doesn't cut to minimum indentation level although I'm not sure if this is a bug or intended behaviour.

.php_cs

<?php

return Symfony\CS\Config\Config::create()
    ->setUsingCache(true)
    ->level(Symfony\CS\FixerInterface::NONE_LEVEL)
    ->fixers(array(
        'encoding',
        'short_tag',
        'braces',
        'eof_ending',
        'linefeed',
        'parenthesis',
        'php_closing_tag',
        'whitespacy_lines',
        'trailing_spaces',
        'indentation'
    ))
;

test.php

<?php

    if (false) {
    // line after comment is not indented properly
    echo '1';

        $id = 10;
    }

[GrahamCampbell]

What version please?

[ghispi]

PHP CS Fixer version 1.8 by Fabien Potencier (93c723f),
downloaded from http://cs.sensiolabs.org/

[SpacePossum]

(@keradus please drop the to verify tag, there is a fail test for it now isn't there?)

[keradus]

done

[fabiofdsantos]

I'm experiencing the same issue.
Indentation breaks in every line after a comment //.

[keradus]

Please, could you check that error on 2.x line ?

[fabiofdsantos]

Before:

class TestClass
{
    // test
public function test()
{
    $test = '123';
}
}

After:

class TestClass
{
    // test
public function test()
{
    $test = '123';
}
}

Before:

class TestClass
{
    public function test()
    {
        //test
        $test = '123';
    if($test == '321') {
        print 'ok';
    }
    }
}

After:

class TestClass
{
    public function test()
    {
        //test
        $test = '123';
    if($test == '321') {
        print 'ok';
    }
    }
}

Before:

class TestClass
{
    public function test()
    {
        //test
        $test = '123';
    $test2 = '321';
    }
}

After:

class TestClass
{
    public function test()
    {
        //test
        $test = '123';
        $test2 = '321';
    }
}

[char101]

From #2311:
This seems to work, what do you think? The idea is to indent not only after ; and } but also after comments. Also do not skip indenting comment lines inside braces.

diff --git a/src/Fixer/Basic/BracesFixer.php b/src/Fixer/Basic/BracesFixer.php
index 1cc8ce1..5dbfd8b 100644
--- a/src/Fixer/Basic/BracesFixer.php
+++ b/src/Fixer/Basic/BracesFixer.php
@@ -229,13 +229,11 @@ function ($item) {
                     continue;
                 }

-                if (1 === $nestLevel && $nestToken->equalsAny(array(';', '}'))) {
+                if (1 === $nestLevel && ($nestToken->equalsAny(array(';', '}')) || $nestToken->isComment())) {
                     $nextNonWhitespaceNestIndex = $tokens->getNextNonWhitespace($nestIndex);
                     $nextNonWhitespaceNestToken = $tokens[$nextNonWhitespaceNestIndex];

                     if (
-                        // next Token is not a comment
-                        !$nextNonWhitespaceNestToken->isComment() &&
                         // and it is not a `$foo = function () {};` situation
                         !($nestToken->equals('}') && $nextNonWhitespaceNestToken->equalsAny(array(';', ',', ']', array(CT::T_ARRAY_SQUARE_BRACE_CLOSE)))) &&
                         // and it is not a `Foo::{bar}()` situation

<?php

if (true) {
        echo 'a';
            /* test */
                echo 'd';
                    // test
                        echo 'k';
}

output:

<?php

if (true) {
    echo 'a';
    /* test */
    echo 'd';
    // test
    echo 'k';
}

[SpacePossum]

thanks for looking into this @char101 :)
did you try if the unit tests run OK with your patch applied?

[char101]

More complex test case

Statements seems OK, still need improvement in array and multiline comment formatter.

<?php

if (true) {
        echo 'a';
            /* test */
                echo 'd';
                    // test
                        echo 'k';
                  for ($i = 0; /* command */ $i < /* comment */ 10; ++$i)
                    {
                        echo 1;
                       /* comment */
                          // comment
                        echo 2;
                        $a = [0,
                            /* comment */
                                        1,
                                        // comment
                                    ]
                                    ;
                  }

            /* multi
                * line
             */
}

<?php

if (true) {
    echo 'a';
    /* test */
    echo 'd';
    // test
    echo 'k';
    for ($i = 0; /* command */ $i < /* comment */ 10; ++$i) {
        echo 1;
        /* comment */
        // comment
        echo 2;
        $a = [0,
                            /* comment */
        1,
                                        // comment
        ]
                                    ;
    }

    /* multi
                * line
             */
}

[char101]

@SpacePossum Tests: 3598, Assertions: 56790, Failures: 14, Warnings: 23, Skipped: 19.

But phpunit does not show the list of failures. Using phpunit 5.5.4 in ubuntu 16.04. Do you know why?

EDIT: failure is displayed with phpunit 5.6.0

[SpacePossum]

That is odd as typically PHPUnit will output details. If not you can always try to add the -vvv flag to up the details outputted by PHPUnit.

[char101]

Unit tests passed with these changes

OK, but incomplete, skipped, or risky tests!
Tests: 5412, Assertions: 111016, Skipped: 34.

diff --git a/src/Fixer/Basic/BracesFixer.php b/src/Fixer/Basic/BracesFixer.php
index 1cc8ce1..5afc9a5 100644
--- a/src/Fixer/Basic/BracesFixer.php
+++ b/src/Fixer/Basic/BracesFixer.php
@@ -229,13 +229,28 @@ function ($item) {
                     continue;
                 }

-                if (1 === $nestLevel && $nestToken->equalsAny(array(';', '}'))) {
+                $isNewLine = function($index, $direction) use ($tokens) {
+                    $index = $tokens->getTokenOfKindSibling($index, $direction, array(array(T_WHITESPACE)));
+                    if ($index !== null) {
+                        $token = $tokens[$index];
+                        return strpos($token->getContent(), "\n") !== false;
+                    }
+                    return false;
+                };
+
+                if (1 === $nestLevel && ($nestToken->equalsAny(array(';', '}')) || $nestToken->isComment())) { // indent after comment
                     $nextNonWhitespaceNestIndex = $tokens->getNextNonWhitespace($nestIndex);
                     $nextNonWhitespaceNestToken = $tokens[$nextNonWhitespaceNestIndex];
+                    $prevNonWhitespaceNestIndex = $tokens->getPrevNonWhitespace($nestIndex);
+                    $prevNonWhitespaceNestToken = $tokens[$prevNonWhitespaceNestIndex];
+                    $isNextWhitespaceNewline = $isNewLine($nestIndex, 1);

                     if (
-                        // next Token is not a comment
-                        !$nextNonWhitespaceNestToken->isComment() &&
+                        // next token is not a comment or it is not a comment in the same line as the opening brace or a comment before a control statement
+                        (!$nestToken->isComment() || !($prevNonWhitespaceNestToken->equals('{') || $nextNonWhitespaceNestToken->isGivenKind($indentTokens))) &&
+                        // next Token is not a comment or the comment is on another line
+                        (!$nextNonWhitespaceNestToken->isComment() || $isNextWhitespaceNewline) &&
+                        // !$nextNonWhitespaceNestToken->isComment() &&
                         // and it is not a `$foo = function () {};` situation
                         !($nestToken->equals('}') && $nextNonWhitespaceNestToken->equalsAny(array(';', ',', ']', array(CT::T_ARRAY_SQUARE_BRACE_CLOSE)))) &&
                         // and it is not a `Foo::{bar}()` situation

[SpacePossum]

Unit tests passed with these changes

good news :)
can you prepare a pull request with your changes including your test case?

[char101]

It works with small files but it does not work with longer files I tested (statements are still not indented after line comments). I will investigate this problem further.

[keradus]

Fixed, will be released soon. thanks to @julienfalque !