Closed aadmathijssen closed 3 years ago
@aadmathijssen Thanks for reporting this. This behaviour is by design. This was discussed extensively when the feature was added: https://github.com/squizlabs/PHP_CodeSniffer/pull/2137
Hi @jrfnl, thanks for the reply. I think it is best to document this somewhere; adding a section on the --filter
option to the Advanced Usage wiki page looks like an appropriate place to me.
In the original PR I also found a reference to the diff-sniffer tool by @morozov, which seems to provide a solution to the issue I reported.
I think I found a way to create a git pre-commit hook that employs the GitStaged
filter while disregarding the changes in the working tree. I included this solution at the end of the issue description.
I also created a GitHub gist for easier copy-pasting: https://gist.github.com/aadmathijssen/e539b808a5da9a7e8f74244610554111
@aadmathijssen That looks about right to me. This was one of the things discussed in the original ticket. It is, however, not currently possible to do this from within PHPCS as there is no "undo changes made by a filter after a run" functionality/hook.
All the same, the script in the gist is already much simpler than your original script, isn't it ? 😉
I'm going to close this issue as it looks like the conversation has resolved. Thanks @jrfnl.
Describe the bug When using
GitStaged
filter option, phpcs checks the staged files instead of all files (which is good). However, instead of checking the staged version of each file, the working tree version is checked, which might be different. When using theGitStaged
filter as a pre-commit hook, you want the staged version to be checked and not the working tree version to avoid accidentally committing files with phpcs violations.Steps to reproduce Take the following PHP file:
Add it to the git staging area:
Running phpcs with the
GitStaged
filter correctly returns 1 error:Now change the file as follows:
This fixes the issue in the working tree, but for some reason you might not want to commit this change yet (for instance because you want to test the performance impact of using
intval
instead of casting).Now when I run phpcs with the
GitStaged
filter it returns 0 errors:This is unexpected, because I am not committing the version in my working tree but in staging.
The same happens when I delete the file in my working tree:
Now the file that I am about to commit is not taken into account in the analysis by the phpcs tool.
Expected behavior
A would have expected the phpcs checks to be performed on the version in staging instead of the working tree version.
Versions:
Additional context
This Github Gist provides a pre-commit hook that resolves this issue by creating a temporary working tree containing only the staged versions of the staged files and runs phpcs on that folder instead of the current working tree.
Possible solution using git stash
It seems to be possible to create a git
pre-commit
hook that employs thegitstaged
filter while disregarding the changes in the working tree:gitstaged
filter and store the exit code in a variable:This particular use of
git stash push
,git reset
andgit stash pop
avoids potential merge conflicts. See the following StackOverflow post for details: https://stackoverflow.com/questions/26379637/whats-the-index-option-for-git-stash-pop-for