Php-cs-fixer: Indentation issue with comment

[raptor235]

Hey guys I'm seeing an issue with indentation when an inline comment is present... this has been formated by php-cs-fixer already

class fp_charts extends WP_Widget
{
    public function __construct()
    {
        $widget_details = array(
            'classname' => 'fp_charts',
            'description' => 'Create charts using supplied data'
        );
        parent::__construct('fp_charts', 'FlowPress Charts', $widget_details);
    }

    //Backend form
  public function form($instance)
  {

[raptor235]

Notice the second class declaration after the //backend form the public function form( only gets two spaces assigned in front of it.

[keradus]

php --version
php-cs-fixer --version
cat .php_cs
cat .php_cs.dist

and CLI command that you are using.

[kidager]

Same thing for me

php --version

PHP 5.6.25-1+deb.sury.org~xenial+1 (cli)
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

php-cs-fixer --version

PHP CS Fixer version 1.12.0 by Fabien Potencier and Dariusz Ruminski (ddac737)

cat .php_cs

cat: .php_cs: No such file or directory

cat .php_cs.dist

cat: .php_cs.dist: No such file or directory

# The command I ran
php-cs-fixer fix ./system/Actions/FacebookAction.class.php --level=psr2 -vvv

[SpacePossum]

this has been formated by php-cs-fixer already

Can you share the code before it was fixed? My guess is the braces-fixer did that. So this would be a FR for an indentation fixer.

[raptor235]

It was braces-fixer that fixed it... wish we had the option to toggle
between tabs and spaces.

On Wed, Sep 14, 2016 at 6:43 AM, SpacePossum [email protected]
wrote:

this has been formated by php-cs-fixer already

Can you share the code before it was fixed? My guess is the braces-fixer
did that. So this would be a FR for an indentation fixer.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/2176#issuecomment-246974213,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AATJ4SL332z3UKmC4sIaCy2Gw_LCj5Lfks5qp8_igaJpZM4J4QxG
.

Bart Dabek

---

Co-Founder & CTO of FlowPress Inc. | Sidekick.pro | WPUniversity.com

647.588.6285 | [email protected] | @bartdabek

[SpacePossum]

wish we had the option to toggle between tabs and spaces.

This is a recurring request and while I don't need it I would like to see that as well, together with Windows linebreak support. It is just that nobody seems to have find the time to implement/PR it. It is however on the list of some contributors here so it might happen in the (near) future. Let me know if you have time to help out :)

[SeBsZ]

I believe I'm seeing this same issue.

PHP Version: PHP 5.5.9-1ubuntu4.19 (cli) (built: Jul 28 2016 19:31:33)
php-cs-fixer version: v2.0.0-alpha, but first tried latest stable release with same effect.

Here are some examples of the resultant code:

    }
    //widget form creation
public function form($instance)
{
    $instance = wp_parse_args((array) $instance, array('title' => ''));

and

    // display widget
    public function widget($args, $instance)
    {
        extract($args);
    // widget options
    //$numposts = $instance['numposts'];
    global $apkm_global;
        global $widget_item_count;
        $widget_item_count = 0;
        $accent_color = getAccentColor($apkm_global);

Let me know if there is more information I can provide to help you solve this problem. I'm not sure what the comment above means by "it was braces-fixer that fixed it". Am I doing something wrong? Thanks.

[SpacePossum]

can you provide an example of the _input_ code which was fixed to the code you provided?

[SeBsZ]

Sure. I just made a very small example file, and I messed up the indentation a little on purpose. Luckily I managed to reproduce the problem fairly quickly. After running just php php-cs-fixer fix myphpfile.php I'm left with the following results:

Input file

<?php
class AppManager_previous_versions extends WP_Widget
{
    // update widget
    public function update($new_instance, $old_instance){
    $instance = $old_instance;
    // Fields
    $instance['numposts'] = strip_tags($new_instance['numposts']);
    $instance['title'] = strip_tags($new_instance['title']);

    return $instance;
    }
}
?>

Output

<?php
class AppManager_previous_versions extends WP_Widget
{
    // update widget
    public function update($new_instance, $old_instance)
    {
        $instance = $old_instance;
    // Fields
    $instance['numposts'] = strip_tags($new_instance['numposts']);
        $instance['title'] = strip_tags($new_instance['title']);

        return $instance;
    }
}

Note the wrong indentation following the // Fields comment. Also it removes the closing ?> php tag, but that might be desired behavior? Thanks for your quick reply!

[SpacePossum]

Also it removes the closing ?> php tag, but that might be desired behavior?

This is done by this fixer:

php_closing_tag [PSR-2]
The closing ?> tag MUST be omitted from files containing only PHP.

you can disable it if you want, but I strongly recommend to keep it. It prevents unintended echo-ing of trailing white space in .php files (it is also a recommendation done in PSR2) .

I'll have a look at your samples, thanks for providing those!

Just FYI, if you want to know which fixer does what on your code run the fixer and add -vvv to the command.

PS. it is the braces-fixer (to whom it may concern ;) )

[SeBsZ]

Hey @SpacePossum have you managed to take a look at my example to find out why it indents so funnily?

[SpacePossum]

I did and your report is indeed valid. It is an issue within the braces-fixer. I gave it a try to fix it, but I have no time ATM to do it. I hope someone else has time to pick this up.

[SeBsZ]

Would it help if I told you it seems the built-in PSR2 fixer in PHPStorm seems to do the same thing? Are you using the same codebase or something?

And thanks for the work you do of course.

[keradus]

we don't base on any logic from PHPStorm. maybe PHPStorm use our logic under the hood. who knows...

[kidager]

I saw a similar behavior on the Sublime Text built-in re-indent command, if that may help.

[archon810]

Any updates here?

[keradus]

feel free to contribute ;)

[SeBsZ]

With the latest 2.0.0 release, the issue and testcase I described above is still broken. Strangely I discovered if you fix the indentation manually and then run php-cs-fixer, it won't break anything. It's only when you start with a badly indented version that it will mess up - around comments especially.

[SpacePossum]

@SeBsZ thanks for testing and sharing your findings!
There has been no PR's to resolve this so please if anyone has time to work on it it would be very welcome!

[adri2k]

I can confirm that in the 2.0.0 the issue is still here.
Same bug of #1196

[owebia]

In Fixer/Basic/BracesFixer.php, line 319, add

                $nestToken = $tokens[$nestIndex];

                /* Fix start */
                if ($nestToken->isComment() && substr($nestToken->getContent(), 0, 2) == '//') {
                    $tokens->ensureWhitespaceAtIndex($nestIndex - 1, 0, rtrim($tokens[$nestIndex - 1]->getContent(), ' ') . $indent . $this->whitespacesConfig->getIndent());
                }
                /* Fix end */

In Fixer/Basic/BracesFixer.php, line 330, replace

                if (1 === $nestLevel && $nestToken->equalsAny(array(';', '}'))) {

by

                if (1 === $nestLevel && $nestToken->equalsAny(array(';', '}', array(T_COMMENT)))) {

[SpacePossum]

@owebia Thanks for the feedback and taking time to look into this. Could you maybe make a PR out of the proposed changes?

[messju]

The proposed fix by @owebia makes soms tests break for me.

like:

./vendor/phpunit/phpunit/phpunit --filter testPreserveLineAfterControlBrace tests/Fixer/Basic/BracesFixerTest.php

[...]

2) PhpCsFixer\Tests\Fixer\Basic\BracesFixerTest::testPreserveLineAfterControlBrace with data set #1 (' Code build on input code must match expected code.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
\<\?php
-if ($test) { // foo
+if ($test) { // foo
echo 1;
}
-if (1 === 1) {//a
+if (1 === 1) {
+ {//a
$a = "b"; /*d*/
}//c
echo $a;
if ($a === 3) /**/
{echo 1;}

as you see "{//a" becomes "{\n {//a". The opening brace get's duplicated. This produces invalid code.

[keradus]

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