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

notName/notPath ignored if filename provided as parameter #4882

Open
Yivan opened this issue Mar 10, 2020 · 11 comments
Open

notName/notPath ignored if filename provided as parameter #4882

Yivan opened this issue Mar 10, 2020 · 11 comments

Comments

@Yivan
Copy link

Yivan commented Mar 10, 2020

Hello,

Using ->notName('*.html.php') works fine if PHP CS Fixer is run on a directory.
But if a file is passed php php-cs-fixer.phar fix myfile.html.php it get formated.
It should not as the config forbid it.

Use case:
It breaks tools like lint-staged or other commit hook tools which send list of files. So even excluded name with notName are formated.
It is pretty dangerous and should clearly indicated in documentation that passing a file will ignore notName rules.
I didn't check if exclude/notPath are ignored or not...

But as rules of thumbs, what is said in config file should be respected whatever parameter (field or directory) is passed to php-cs-fixer.phar.

Hope it could be fixed.

For now the solution is to make shell wrappers to try to pre-check before running php-cs-fixer.phar or configure more grainy the commit hook tools to exclude files at this level.

Thanks.

@Yivan
Copy link
Author

Yivan commented Mar 11, 2020

Just to add, i tried using notPath, same it is not respected.
->notPath('#views#') the file inside are formated if running directly by filename php php-cs-fixer.phar fix src/MyBundle/Resources/views/partial/myfile.html.php

Whereas the find linux tools respect well the exclusion, something like:
find src/MyBundle/Resources/views/partial/myfile.html.php ! -path '**/views/**'
dont return nothing.

In fact, even ->name('*.ext') is not respected. So I think as soon a file or list of files is given as parameter they formated whatever is configured.

@Yivan Yivan changed the title [Bug] notName ignored if filename provided as parameter [Bug] notName/notPath ignored if filename provided as parameter Mar 11, 2020
@julienfalque
Copy link
Member

julienfalque commented Mar 11, 2020

From the documentation:

By default --path-mode is set to override, which means, that if you specify the path to a file or a directory via command arguments, then the paths provided to a Finder in config file will be ignored. You can use --path-mode=intersection to merge paths from the config file and from the argument:

$ php php-cs-fixer.phar fix --path-mode=intersection /path/to/dir

This means files passed as arguments on command line are always fixed, you should avoid passing them.

@Yivan
Copy link
Author

Yivan commented Mar 11, 2020

@julienfalque Thanks for your return. I was aware of this, but specifing a path in cli, the config file is well taked into account despite what is said in documentation, using default path-mode.

For instance:
php php-cs-fixer.phar fix src/MyBundle/Resources
And in finder config:
->notName('*.html.php')
works fine and *.html.php file are not formated.

But with php php-cs-fixer.phar fix src/MyBundle/Resources/views/partial/myfile.html.php it is formated.

So there seems to really be a different behaviour between passing a path and a file. Path use config, file not.

I will try --path-mode=intersection, thanks!

@Yivan
Copy link
Author

Yivan commented Mar 11, 2020

Just tried with --path-mode=intersection, it changes nothing... : (

Running:
php php-cs-fixer.phar fix --path-mode=intersection src/MyBundle/Resources/views/partial/myfile.html.php
With config: ->name('*.ttt')
Format the myfile.html.php file, but should not.

Just to say, on our pipeline this is one of the very few tools which doesn't respect it's config when passing direct file fullpath on command lines.
Other tools works fine: find (linux), prettier, PHP_CodeSniffer, etc., they respect totally the rules and ignored pattern configured so no risk to format wrong files when using commit hook tools.

I think this issue could be reopend, and if it is not a bug but the intended way to work so this issue should be labeled as improvement, so it works same than other well-know linting tools
.
Docs should maybe be arranged to better explain this as --path-mode change nothing in fact and the rule is just: If you specify a file directly via command arguments, finder exclusion/ignore pattern in config file are not considered. If GIT hook tools are used (lint-staged, pre-commit, etc.), you should take care yourself to send the right files.. It will help developers to prevent any surprise.

Thanks.

@julienfalque
Copy link
Member

Mmmh indeed, maybe this could be improved, or the doc should be at least.

@julienfalque julienfalque reopened this Mar 11, 2020
@Wirone Wirone added the status/to verify issue needs to be confirmed or analysed to continue label May 16, 2023
@github-actions

This comment was marked as outdated.

@Yivan
Copy link
Author

Yivan commented Aug 19, 2023

Hi, should not be closed as it need a fix or documentation update. Thanks.

@Wirone Wirone removed the status/to verify issue needs to be confirmed or analysed to continue label Aug 26, 2023
@Wirone Wirone changed the title [Bug] notName/notPath ignored if filename provided as parameter notName/notPath ignored if filename provided as parameter Oct 10, 2023

This comment has been minimized.

@s0me0ther
Copy link

This bug currently prevents us from efficiently running through only the files that have been changed in a precommit hook instead of running through all files.

IFS='
'
CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB)
if [ -n "$CHANGED_FILES" ]; then
#	EXTRA_ARGS=$(printf -- '--path-mode=override\n--\n%s' "${CHANGED_FILES}");
    lib/vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php --path-mode=intersection -vv --stop-on-violation #  --using-cache=no ${EXTRA_ARGS}
fi

Running trough the complete project takes much more time and blocks somehow..

@Wirone
Copy link
Member

Wirone commented Feb 5, 2024

@s0me0ther I think #7777 can help you with this too, because parallel analysis would be faster for the whole project. Anyway, this issue should be solved so it's possible to specify list of paths and exclude some of them on config level.

Copy link

github-actions bot commented May 8, 2024

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

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

5 participants