Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing multiple filenames without a configuration #4279

Closed
jdufresne opened this issue Jan 24, 2019 · 19 comments
Closed

Allow passing multiple filenames without a configuration #4279

jdufresne opened this issue Jan 24, 2019 · 19 comments

Comments

@jdufresne
Copy link

When reporting an issue (bug) please provide the following information:

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

$ php --version
PHP 7.2.14 (cli) (built: Jan  8 2019 09:59:17) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Xdebug v2.6.1, Copyright (c) 2002-2018, by Derick Rethans

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

$ ./vendor/bin/php-cs-fixer -V
PHP CS Fixer 2.13.1 Yogi's BBQ by Fabien Potencier and Dariusz Ruminski

The command you use to run PHP CS Fixer:

$ ./vendor/bin/php-cs-fixer fix path/to/file1.php path/to/file2.php
In ConfigurationResolver.php line 578:
                                                    
  For multiple paths config parameter is required.  

The configuration file you are using, if any:

None!


I've read through the rationale as to why this occurs here:

I'm asking if the project can reconsider this decision and allow multiple paths without a configuration file. Typically, POSIX CLI tools are built to allow an arbitrary number of paths. If you take a look at other static analysis and code formatting tools, you'll see this is very common. Standard POSIX tools like xargs are built with this in mind.

My project chooses not to use a PHP-CS-Fixer configuration file. The goal is to have minimal fuss w/r/t code formatting and instead accept community defaults. As a result, I get the above error when using php-cs-fixer with multiple files.

This became a particular pain point when integrating with pre-commit. pre-commit is built to pass all changed files to a tool in a single command. Requests to allow a "one at a time feature" have been rejected with the response "the best way to improve the community is to improve the specific linter to take filenames in a more standard way." So here I am.

As a user, I'm stuck between these two tools. My team's workaround has been to configure pre-commit to run php-cs-fixer against the entire source directory on every change. But it would be nice to apply it to just the changed files instead (like other tools).

@jasonmccreary
Copy link

jasonmccreary commented May 7, 2019

Sorry to pile on, but I'm a recent adopter and very surprised to see this limitation. While I can understand there is some confusion which config file to use (based on previous threads), there should be a way to still pass multiple paths, especially when passed --rules.

Example usage:

vendor/bin/php-cs-fixer fix --rules='{"array_syntax": {"syntax": "short"}}' -- path1 path2 file.php

@SpacePossum
Copy link
Contributor

Since we do not try to find a config file when --rules is supplied we won't have the problem of finding the correct configuration file when passing multiple files to fix without the configuration file passed as well.

Therefore I think we can add support for that use-case, for example:
php-cs-fixer fix --rules={...} file1 file2...

So the restriction changes from;

when passing multiple files to fix you must pass a configuration file

To:

when passing multiple files to fix you must pass a configuration file or rules

like to hear other opinions on this case as well :)

@keradus
Copy link
Member

keradus commented May 28, 2019

@jdufresne, use this:
php-cs-fixer fix --config=my_config.php_cs.dist fileA.php directoryB pathC
does this solve your request?

@jasonmccreary , passing --rules or not has nothing to do, config file provide not only this.

Since we do not try to find a config file when --rules is supplied

@SpacePossum , oh we do

@keradus
Copy link
Member

keradus commented May 28, 2019

@jdufresne , you can use slightly modified version of what we suggest to use on CI:
https://github.com/FriendsOfPHP/PHP-CS-Fixer#using-php-cs-fixer-on-ci
look at the last command.

@jdufresne
Copy link
Author

IIUC, both of the suggestions are passing a --config= so I don't think that is quite what I'm suggesting. I'm saying, I think I should be able to use php-cs-fixer without passing a config option at all to just rely on the defaults. I'm also saying this should work when multiple files are passed.

@keradus
Copy link
Member

keradus commented May 28, 2019

the thing is to distinguish do we have no config and we are fine with it or we do have a config but we want to ignore it.
what do you think about #4408 (comment) ?

@Taluu
Copy link
Contributor

Taluu commented Jan 9, 2020

I don't think it's related to "having no config". Maybe what should be done as does other tools is to have an option --ignore-config so that no config are loaded, otherwise try .php_cs and .php_cs.dist in that order...

Especially as when giving only one path, the error messages doesn't popup, so it's kinda inconsistent...

(sorry for upping this subject, just hit my head on that problem)

@ardabeyazoglu
Copy link

The ci code suggested by @keradus does not solve the issue. The following command always detects one file.

php-cs-fixer fix --config=".php_cs" -vvv --dry-run --stop-on-violation --path-mode=intersection --using-cache=no -- file1 file2

@crazytonyi
Copy link

Why can't we just have nice things? This concept is very straightforward, and yet it's almost 2 years later and it's still not built in.

@keradus
Copy link
Member

keradus commented Apr 15, 2021

The following command always detects one file.

I find this claim not accurate.

example execution - take a look at two dots (.) below the Runtime - each one is for another file:

ker@dus:~/github/FriendsOfPHP/PHP-CS-Fixer λ php-cs-fixer fix --config=".php_cs.dist" -vvv --dry-run --stop-on-violation --path-mode=intersection --using-cache=no -- src/FileReader.php src/FileRemoval.php 
Loaded config default from ".php_cs.dist".
Runtime: PHP 7.4.3
..
Legend: ?-unknown, I-invalid file syntax (file ignored), S-skipped (cached or empty file), .-no changes, F-fixed, E-error

Checked all files in 0.055 seconds, 12.000 MB memory used

@keradus
Copy link
Member

keradus commented Apr 15, 2021

Why can't we just have nice things? This concept is very straightforward, and yet it's almost 2 years later and it's still not built-in.

@crazytonyi , as already described in this and some other threads, it's tricky to autodetect the configuration if we don't know which is the main dir of the project, and it could be that one run the tool to fix multiple projects in a single line, which, in the current state, PHP CS Fixer does not handle.

imagine:

ker@dus:/tmp/github λ ls
project_a  project_b  project_c
ker@dus:/tmp/github λ php-cs-fixer fix project_a project_b/file.php project_c/subfolder

Which configuration should be taken to apply fixing? how can we be sure that all files are part of the same project, and not different ones (with different configuration) ? The current way, as the easiest approach, is to ask the user explicitly for the configuration file.

But hey, your question is very right ! 2 years and such a feature was not yet implemented? How can that be!

The answer is simple, actually - welcome into the open-source. EVERYONE can contribute! And maintainers, well - while investing their free time, which is limited, they are focusing on topics with high demand (eg PHP8) rather than request raised by few individuals without the bigger amount of community asking for it, especially if an alternative solution is available.

With that, as I don't see this feature request be picked up anytime soon, I am closing this request.
It does not mean that the idea is rejected. If anyone has willing to have such a feature, please raise a PR and we will be happy to review it!

@crazytonyi
Copy link

Why can't we just have nice things? This concept is very straightforward, and yet it's almost 2 years later and it's still not built-in.

@crazytonyi , as already described in this and some other threads, it's tricky to autodetect the configuration if we don't know which is the main dir of the project, and it could be that one run the tool to fix multiple projects in a single line, which, in the current state, PHP CS Fixer does not handle.

imagine:

ker@dus:/tmp/github λ ls
project_a  project_b  project_c
ker@dus:/tmp/github λ php-cs-fixer fix project_a project_b/file.php project_c/subfolder

Which configuration should be taken to apply fixing? how can we be sure that all files are part of the same project, and not different ones (with different configuration) ? The current way, as the easiest approach, is to ask the user explicitly for the configuration file.

But hey, your question is very right ! 2 years and such a feature was not yet implemented? How can that be!

The answer is simple, actually - welcome into the open-source. EVERYONE can contribute! And maintainers, well - while investing their free time, which is limited, they are focusing on topics with high demand (eg PHP8) rather than request raised by few individuals without the bigger amount of community asking for it, especially if an alternative solution is available.

With that, as I don't see this feature request be picked up anytime soon, I am closing this request.
It does not mean that the idea is rejected. If anyone has willing to have such a feature, please raise a PR and we will be happy to review it!

I guess I don't understand the role of the config file or how php_cs_fixer is project oriented. If I pass in one file and a list of rules, it formats it and says it used the default config. Why would I expect it not to have the same functionality for multiple files? If I pass it 5 files, I'm effectively saying "hey, imagine I just did this 5 times, and do that".

I don't usually work out of an ide (one of the reasons I want php_cs_fixer, since I'm not in an environment that auto formats) so there's no potential for multiple projects with different configs. Perhaps this tool could detect that? Or have a switch to tell it to use the default config (or a "non-project-mode" switch)? Or it could prompt with something about how since no config was set, it will use the one it would use for the first file in the list.
This tool has so many options and features, it can just be frustrating that it can't do some of these fairly intuitive basic things and that it spends a year in deliberation while a hundred other new features are added. I was similarly frustrated awhile back that you can't pipe stdin to php_cs_fixer, and found at least two PRs, one which seemed like it was closed with an attitude of "don't anybody dare try to comment again on this" when all comments were people begging for the feature.

@keradus
Copy link
Member

keradus commented Apr 16, 2021

so there's no potential for multiple projects with different configs

You know that by heart, but the tool has no such knowledge. and somehow, that knowledge has to be provided to the tool, otherwise there is risk that files will be converted using different rules than intended (case with multi-projects).
I imagine passing list of paths + config file can have an alternative of passing `list of paths + list of rules + set path-mode (maybe even implicitly?) to "override").

Strong recommendation: avoid the frustration that nobody provided the feature you are looking for and think how you can encourage ppl to provide it, or maybe even provide it yourself? We need a decent PR followed by a decent review.

If the feature has nobody willing to implement it, or maybe there is one alone soul willing to raise a PR, yet nobody willing to review it within the community, then, well, unfortunately - it's not realistic to expect from limited maintainers group to fulfil all requests.

@jasonmccreary
Copy link

@keradus, in fairness, I did submit a PR years ago (#4411). After some back and forth it was closed as you did not provide any actionable feedback.

I still have a use case for being able to use php-cs-fixer by simply passing --rules and not being forced to create a config file. I feel such a feature would make this tool so much more powerful as it would allow users to run it directly. For example:

php-cs-fixer fix --rules=no_unused_imports -- app database

I'd be willing to offer a bounty if it helps.

@eusonlito
Copy link

Same here, I always run 3 times the php-cs-fixer binary to fix 3 folders:

php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv app
php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv database
php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv config

it's very annoying.

@TomaIvanovTomovYotpo
Copy link

2022 Still doesn't work with multiple changed files ..

@SpacePossum
Copy link
Contributor

php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv app
php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv database
php-cs-fixer fix --rules=@PHP80Migration,@PSR2,no_unused_imports --allow-risky=yes --verbose -vvv config

why not create a config file that specifies those 3 directories?

2022 Still doesn't work with multiple changed files ..

It does, but it requires some work, you would have to make a config file that specifies those changed files (dynamically typically), this can be done through returning a Finder object.
On the symfony project this is done by fabbot which only reports CS issue on the files changed in a PR, not all files of the whole project, so it can be done.

@eusonlito
Copy link

@SpacePossum, yes, I can create a config, also we can remove all available parameters and use only a config file. The idea is if we allow the path as parameter and we can set the multiple paths in the config file, why not add the multipath feature also as parameter?

@keradus
Copy link
Member

keradus commented Sep 13, 2022

There are 2 challanges for doing so, about which config file to use.

Please raise the suggestion how to overcome those, and if it's agreed to go that path by community, please raise the PR.

divspace added a commit to bisnow/pre-commit that referenced this issue Apr 15, 2024
When multiple paths are passed, PHP CS Fixer requires the `--config`
parameter.

PHP-CS-Fixer/PHP-CS-Fixer#4279
divspace added a commit to bisnow/pre-commit that referenced this issue Apr 15, 2024
When multiple paths are passed, PHP CS Fixer requires the `--config`
parameter.

PHP-CS-Fixer/PHP-CS-Fixer#4279
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants