pint: `method_chaining_indentation` rule doesn't apply correctly the first time when combined with `no_space_around_double_colon`

Pint Version

1.13.6

PHP Version

8.2.12

Description

Running into an interesting corner case where Pint incorrectly formats a file and subsequently passes future Pint --test checks in the same environment.

The two rules at play here, per Pint, are method_chaining_indentation and no_space_around_double_colon.

Without looking under the hood of Pint, seems like it might be caching results, as renaming the affected file and running Pint again corrects the formatting issue.

Steps To Reproduce

1. Create Test.php:

<?php

$formattedIncorrectly = Foo::bar
    ::baz()
    ->where('id', 1)
    ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

2. Run ./vendor/bin/pint:

  ................................................................................................................✓.........................................................
  .................

  ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel  
    FIXED   ................................................................................................................................. 187 files, 1 style issue fixed  
  ✓ app/Http/Controllers/TestFile.php                                                                              method_chaining_indentation, no_space_around_double_colon

(Note the 186 other files are part of an existing project where this issue surfaced.)

3. Inspect the contents of Test.php:

Note the $formattedIncorrectly block has extra spacing behind ->where and ->firstOrFail.

<?php

$formattedIncorrectly = Foo::bar::baz()
        ->where('id', 1)
        ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

4. ./vendor/bin/pint --test -vvv doesn’t pick up on any issues:


Box Requirements Checker
========================

> Using PHP 8.2.12
> PHP is using the following php.ini file:
  /opt/homebrew/etc/php/8.2/php.ini

> Checking Box requirements:
  ✔ The application requires a version matching "^8.1.0".
  ✔ The application requires the extension "zlib".
  ✔ The application requires the extension "json".
  ✔ The application requires the extension "mbstring".
  ✔ The application requires the extension "tokenizer".
  ✔ The application requires the extension "xml".
  
                                                                                                                                                                              
 [OK] Your system is ready to run the application.                                                                                                                            
                                                                                                                                                                              


  ..........................................................................................................................................................................
  .................

  ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel  
    PASS   ....................................................................................................................................................... 187 files

5. Rename Test.php to AnotherTest.php Issues are now detected by ./vendor/bin/pint --test -vvv, and running ./vendor/bin/pint formats the file as expected.

Box Requirements Checker
========================

> Using PHP 8.2.12
> PHP is using the following php.ini file:
  /opt/homebrew/etc/php/8.2/php.ini

> Checking Box requirements:
  ✔ The application requires a version matching "^8.1.0".
  ✔ The application requires the extension "zlib".
  ✔ The application requires the extension "json".
  ✔ The application requires the extension "mbstring".
  ✔ The application requires the extension "tokenizer".
  ✔ The application requires the extension "xml".
  
                                                                                                                                                                              
 [OK] Your system is ready to run the application.                                                                                                                            
                                                                                                                                                                              


  ...................................................................................................................⨯......................................................
  .................

  ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel  
    FAIL   ........................................................................................................................................ 187 files, 1 style issue  
  ⨯ app/Http/Controllers/AnotherTest.php                                                                                                         method_chaining_indentation  
  @@ -1,8 +1,8 @@
   <?php
   
   $formattedIncorrectly = Foo::bar::baz()
  -        ->where('id', 1)
  -        ->firstOrFail();
  +    ->where('id', 1)
  +    ->firstOrFail();
   
   $formattedCorrectly = Foo::bar::baz()
       ->where('id', 1)

The final corrected file looks like:

<?php

$formattedIncorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

About this issue

Original URL
State: closed
Created 7 months ago
Comments: 23 (3 by maintainers)

Most upvoted comments

@shengslogar @nunomaduro The pull request #7723 addressing this issue has been merged into the PHP CS Fixer master branch. Now awaiting the next release. 👍

mho22 on Jan 14, 2024

Yes the next release will have the new versions defined in composer.lock. (Should probably be released sometime today)

Jubeki on Jan 16, 2024

@mho22 THANK YOU for all your hard work on this! 07494e299d2704c3ea61b51196d1639b0897e4ec bumped composer.json to 3.47.0, so I assume the next tagged Pint release (not sure if the pint binary embeds this package?) will contain this fix. 🙌

shengslogar on Jan 16, 2024

@shengslogar It is now released ! [v3.47.0](https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/releases/tag/v3.47.0)

mho22 on Jan 15, 2024

@mho22 Appreciate you taking the time to make a repro! You seem to have a much better handle on this than I do. Would you mind opening a ticket in the PHP CS Fixer repo or would you rather I do? This is the only issue I spotted that looks similar, but was closed due to being reported for an older version. Thanks for all your help on this!

shengslogar on Jan 8, 2024

@shengslogar I tried to reproduce the issue using PHPCSFixer alone and it succeeded reproducing it. The files here to reproduce it quickly :

composer.json

{
    "require": {
        "php": "^8.2"
    },
    "require-dev": {
        "friendsofphp/php-cs-fixer": "^3.46"
    }
}

.php-cs-fixer.php

<?php

use PhpCsFixer\Finder;
use PhpCsFixer\Config;


$finder = ( new Finder() )->in( __DIR__ );


return ( new Config() )
    ->setUsingCache( false )
    ->setRules( [
        'method_chaining_indentation' => true,
        'no_space_around_double_colon' => true
    ] )
    ->setFinder( $finder );

test.php

<?php

$formattedIncorrectly = Foo::bar
    ::baz()
                    ->where('id', 1)
    ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

run vendor/bin/php-cs-fixer fix

mho22 on Jan 8, 2024

@shengslogar The issue you described appears to be related to PHPCSFixer. To confirm, try reproducing the issue using PHPCSFixer alone. If you succeed in reproducing the issue, please report it to the PHPCSFixer repository; if not, feel free to re-open this issue.

nunomaduro on Jan 8, 2024

@mho22 Agreed. Will wait to hear Dries’ opinion on whether he’d like this resolved here or there. Thanks for spearheading a resolution! 🙏

shengslogar on Jan 5, 2024

@shengslogar I believe that if PHP-CS-FIXER included a preset with both rules, they might face a similar issue to what we’ve encountered. However, it appears that method_chaining_indentation is exclusive to the PhpCsFixer set, while no_spaces_around_double_colon is in the PSR2 set. It might be worthwhile to propose to PHP-CS-FIXER to incorporate priority settings for these rules as well.

mho22 on Jan 5, 2024

@shengslogar

I had a doubt regarding the sequence of the two specific rules, the order in which no_space_around_double_colon and method_chaining_indentation should be executed. Initially, it seemed logical that no_space_around_double_colon should be applied before method_chaining_indentation, but the observed behavior was the opposite.

The issue should come from the rule order in the Laravel preset, where they are arranged alphabetically. However, when I tried with a modified resources/presets/laravel.php file in a local forked repository, I achieved the desired outcome, and the rules were executed in the correct order.

@driesvints What’s next ? A Pull Request with one rule not in the correct alphabetical order ? Or a Pull Request to add priority in the rules in PHP_CS_FIXER ?

mho22 on Jan 5, 2024

@driesvints I’ll look into it.

mho22 on Jan 5, 2024

It seems like it is something linked with the laravel preset.

I tried the same test with this pint.json configuration :

 {
    "preset" : "psr12",
    "rules" : {
        "method_chaining_indentation" : true
    }
}

And the behavior is as expected.

<?php

$formattedIncorrectly = Foo::bar
    ::baz()
    ->where('id', 1)
    ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

vendor/bin/pint test.php --config pint.json

<?php

$formattedIncorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

$formattedCorrectly = Foo::bar::baz()
    ->where('id', 1)
    ->firstOrFail();

mho22 on Dec 26, 2023