multiple path vs distributed configuration file

[havvg]

Hi there,

when running a command to apply the cs fixer on all files within the git staging area
git diff-index --cached --name-only --diff-filter=ACMR HEAD | xargs php-cs-fixer fix

the following exception is raised:

[PhpCsFixer\ConfigurationException\InvalidConfigurationException (16)]
  For multiple paths config parameter is required.

Running an alternative on a per-file basis works just fine (from version 1.x).
git diff-index --cached --name-only --diff-filter=ACMR HEAD | xargs -n1 php-cs-fixer fix

If I would add the config option, I'm loosing the default configuration fallback mechanism (.php_cs, .php_cs.dist). What is the expected way to combine both features in a one-shot command?

There are two main reasons I want to use the one-shot:

1. Performance: 2.81s user 0.09s system 99% cpu 2.912 total vs 28.36s user 11.14s system 91% cpu 42.974 total on a 430 files sample.
2. Output verbosity: the repeating output for each file is annoying to read, in terminal and log files.

[keradus]

this is expected.
one-shot is preferred way ;)

Please check out this: https://github.com/FriendsOfPHP/PHP-CS-Fixer#using-php-cs-fixer-on-ci

[havvg]

I see, so there is currently no way to get both features working at the same time?

[SpacePossum]

Multiple files from commandline was requested before, can't recall why this is not possible

[keradus]

multiple file is working, from what i understand he want to have it without config file

[keradus]

@havvg its very problematic to determine correct config file / default fallback when you have multiple paths, especially if one would have file A, second file B, and 3rd default fallback. That's the reason the path to config is required while providing multiple paths.

I strongly advise to make a project configuration file even if one doesn't use multiple path argument.

[GrahamCampbell]

I agree with @keradus. Our current system is already sophisticated enough for most, and I don't think it would be a good idea to make it more complex.

[SpacePossum]

I mean when you pass multiple files (not directories) by command line and no config is found, we could use the default (or commandline provided rule (sets)), same as we do when one file is passed. I didn't mean adding more layers of configuration resolving.

[keradus]

I mean when you pass multiple files (not directories) by command line and no config is found

where would you look for the config ? this is the problematic part.

[havvg]

@keradus I think there is some kind of misunderstanding :)

What I mean is the following:

I run a command like php-cs-fixer fix path1 path2 path3
This currently forces me to add --config=....
However the command is yielded by a git alias; based on the project, there might be a .php_cs or not, but there will be a .php_cs.dist (other considerations out of scope here).

The two features I want to combine is the config mechanism using either .php_cs or .php_cs.dist from the current working directory when running the command with multiple paths.

[keradus]

so, why can't you specify --config=.php_cs.dist on your CI or --config=.php_cs on your local env ?

[havvg]

Because it's an alias, which is used across many projects, some of them don't have a .php_cs, some do. There is no correlation to any CI with my use-case :)

[keradus]

Then, for the sake of clean and safe solution, when we are not juggling which config will be used and how, we decided to require configuration parameter for multi-paths call. embrace that ;)

[gharlan]

@havvg You could use this workaround:
Create a file like ~/.php_cs.global and use --config=~/.php_cs.global.
In this global config file resolve the actual config file path by your own rules and include that file.

[havvg]

Thank you @gharlan, I will take a look into this solution.

@keradus I don't get the distinction. What's the issue with applying the same rule for a single path? It's not like I'm randomly using paths where each path may yield a different configuration file, but all paths use the very same configuration when issued on the same CLI call.

I see your point, if one would randomly use paths where each path is separate in its configuration. However, I would guess the 80% use-case would reflect all given paths being part of the same project/base, sharing the same configuration as if you call all the paths (without config option) one by one.

[keradus]

the issue is when one will run cli command from root path and point multiple paths to different projects.
sadly, you cannot make the assumption that user won't make that and you have to handle that case

[ghost]

Please can this issue be reopened? It still has not been fixed.

@SpacePossum wrote:

I mean when you pass multiple files (not directories) by command line and no config is found, we could use the default (or commandline provided rule (sets)), same as we do when one file is passed. I didn't mean adding more layers of configuration resolving.

Exactly. I am experiencing the same issue.

Among other things, this bug - the failure to behave as described in the passage I quoted above - seems to needlessly reduce or eliminate compatibility between PHP-CS-Fixer and the standard POSIX tool xargs (including the GNU Project implementation of xargs), as you can see in the examples posted a little further below.

None of the proposed workarounds seem satisfactory. The ability for a tool to be able to accept more than one input file, listed by the user as arguments on the command-line, is a long-standing UNIX convention. cat can do it; ls can do it; grep can do it; and so can dozens or hundreds of other ancient and modern tools. There is no good reason why PHP-CS-Fixer cannot or should not do it, too.

Here are some examples of PHP-CS-Fixer invocations that would, if PHP-CS-Fixer's CLI followed standard POSIX/UNIX-like conventions, succeed in fixing the files whose paths are being passed. Currently, due to this bug, these invocations fail to perform those fixes and instead simply produce error messages.

$ php-cs-fixer fix --dry-run --rules=@Symfony 1.php 2.php 
In ConfigurationResolver.php line 578:
                                                    
  For multiple paths config parameter is required.  
                                                    

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--diff-format DIFF-FORMAT] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>]...

find -O3 . \( -path ./resources/views/vendor -o -path ./vendor -o -path ./.git \) -prune -o -iname '*.php' -print0 | sort -z | xargs -0 php-cs-fixer fix --rules=@Symfony -- "{}"

In ConfigurationResolver.php line 384:
                                  
  The path "{}" is not readable.  
                                  

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--diff-format DIFF-FORMAT] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>]...

find -O3 . \
\( -path ./resources/views/vendor -o -path ./vendor -o -path ./.git \) \
-prune -o -iname '*.php' -print0 \
| xargs -0 php-cs-fixer fix --rules=@Symfony --

In ConfigurationResolver.php line 578:
                                                    
  For multiple paths config parameter is required.  
                                                    

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--diff-format DIFF-FORMAT] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>]...

find -O3 . \
\( -path ./resources/views/vendor -o -path ./vendor -o -path ./.git \) \
-prune -o -iname '*.php' -print0 \
| xargs -0 php-cs-fixer fix --rules=@Symfony

In ConfigurationResolver.php line 578:
                                                    
  For multiple paths config parameter is required.  
                                                    

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--diff-format DIFF-FORMAT] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>]...

The PHP version you are using ($ php -v):

7.0.27-0+deb9u1

PHP CS Fixer version you are using ($ php-cs-fixer -V):

PHP CS Fixer 2.12.0 Long Journey by Fabien Potencier and Dariusz Ruminski (a53f39a)

[keradus]

Writing again in 2 years old issue doesn't help @sampablokuper .

Due to complexity of determining single configuration for all passed paths, like sometimes given dir path can contain it's own config inside, sometimes not, sometimes there is config in cwa, sometimes not, we decided to require a config file argument, as error you got ask you for it, when multiple paths are passed. Simply provide config file and you can benefit from passing multiple paths.

[ghost]

@keradus wrote

Writing again in 2 years old issue doesn't help @sampablokuper .

• Noting than a bug is still present, is helpful feedback that should be appreciated by project maintainers. (This is why many bug trackers have "This affects me too" buttons.)
• Reporting additional use-cases in which an unfixed bug appears, especially if shown via reproducible demonstrations (i.e. MWEs), is helpful feedback that should be appreciated by project maintainers.
• Keeping reports/comments about an issue in the same thread as the original report avoids creating unnecessary duplicate issues, and is helpful behaviour that should be appreciated by project maintainers. It means that all the relevant information about the issue can be found on one page, and only requires scrolling. Otherwise, people also have to follow links (best case) or search the issue tracker or the Web (worst case) in order to read all relevant information that has been filed about the issue.

As such, my comment was helpful and constructive feedback, and I would be glad if it could be appreciated in that vein.

we decided to require a config file argument ... when multiple paths are passed

Exactly. This is the bug. It was a decision that damaged PHP-CS-Fixer's CLI's usability by breaking with standard UNIX-like CLI behaviour.

Additional evidence that it is indeed a bug is that the documentation produced by php-cs-fixer fix --help implies that fix should work with multiple paths:

Usage:
  fix [options] [--] [<path>]...

Note the ellipsis.

Due to complexity of determining single configuration for all passed paths, like sometimes given dir path can contain it's own config inside, sometimes not

How is this even a problem? Have a cascade of default behaviours, like other CLI tools have. Something like:

• if an option is specified in a system-wide configuration file, use that setting;
• unless it is specified in a user-wide configuration file, in which case use this setting instead;
• unless it is specified in a directory-wide configuration file, in which case use this setting instead (for files in this directory);
• (unless it is specified in a subdirectory-wide configuration file, in which case use this setting instead (for files in this directory);)
• unless it is specified on the CLI, in which case use this setting instead.

sometimes there is config in cwa, sometimes not

I don't know what you mean by "cwa", sorry.

[keradus]

Noting than a bug is still present (...)

not a bug present, but feature missing.

Reporting additional use-cases (...)

nothing new were presented

Keeping reports/comments about an issue in the same thread as the original report avoids creating unnecessary (...)

not, if thread was few years (!!!) old and was referring to completely different state of software.
Note that due to that behaviour I already got confusion from users on 3 different support channels.

As such, my comment was helpful and constructive feedback (...)

As said, whole input of yours, in those 10 comments or so, can be compressed into "I want the feature, but I will ignore your welcome to open a PR myself"

Exactly. This is the bug.

no, this was a conscious decision; we decided than it's better than disallowing for passing multiple paths.
the change you wanted is a feature request, and as said multiple times, simply raise a PR.

Additional evidence that it is indeed a bug (...)

not an evidence at all, it said you can provide multiple paths - and you can. But for that, you need to fulfill requirement of passing config file explicitly.

How is this even a problem? (...)

apparently, it was a problem for us.
If you think it's trivial - i'm happy to hear that, and, as said already 10 times - PR is more than welcome.

I don't know what you mean by "cwa", sorry.

cwa -> typo for cwd, sorry