PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

gitstaged filter not working when called through npm husky precommit hook

Open dingo-d opened this issue 3 years ago • 9 comments

Describe the bug In my project, I'm using husky to run my pre-commit hooks. In package.json I have

...
"scripts": {
		"__eslintTheme": "eslint src/**/*.js",
		"__stylelintTheme": "stylelint src/**/*.scss",
		"lintStyle": "npm run __stylelintTheme",
		"lintJs": "npm run __eslintTheme",
		"lint": "npm run lintJs && npm run lintStyle && composer standards:check -- --filter=gitstaged",
...
},
"husky": {
	"hooks": {
		"pre-commit": "npm run lint"
	}
}

So it triggers any time I do a commit. In my composer.json I have these scripts

	"scripts": {
		"analyze": "@php ./vendor/bin/phpstan analyze",
		"standards:check": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs",
		"standards:fix": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf"
	},

When I run my commit, all the npm hooks run ok, but the phpcs script fails with

 @php ./vendor/squizlabs/php_codesniffer/bin/phpcs '--filter=gitstaged'
PHP Fatal error:  Uncaught Error: Class '\PHP_CodeSniffer\Filters\gitstaged' not found in /home/dzoljom/Sites/Project/project-name/wp-content/themes/project-name/vendor/squizlabs/php_codesniffer/src/Files/FileList.php:83
Stack trace:
#0 /home/dzoljom/Sites/Project/project-name/wp-content/themes/project-name/vendor/squizlabs/php_codesniffer/src/Runner.php(382): PHP_CodeSniffer\Files\FileList->__construct()
#1 /home/dzoljom/Sites/Project/project-name/wp-content/themes/project-name/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
#2 /home/dzoljom/Sites/Project/project-name/wp-content/themes/project-name/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
#3 {main}
  thrown in /home/dzoljom/Sites/Project/project-name/wp-content/themes/project-name/vendor/squizlabs/php_codesniffer/src/Files/FileList.php on line 83

When I remove the filter, the script runs fine, but over the entire codebase, not just the committed files.

Custom ruleset

<?xml version="1.0"?>
<ruleset name="Eightshift Boilerplate ruleset">
	<description>Ruleset for the Eightshift Boilerplate.</description>

	<rule ref="Eightshift" />

	<exclude-pattern>*/tests/*</exclude-pattern>
	<exclude-pattern>*/vendor/*</exclude-pattern>
	<exclude-pattern>*/public/*</exclude-pattern>
	<exclude-pattern>*/node_modules/*</exclude-pattern>
	<exclude-pattern>*/storybook/*</exclude-pattern>

	<!-- Additional arguments. -->
	<arg value="sp"/>
	<arg name="basepath" value="."/>
	<arg name="parallel" value="8"/>
	<arg name="extensions" value="php"/>

	<file>.</file>

	<!-- Check for PHP cross-version compatibility. -->
	<config name="testVersion" value="7.2-"/>
	<rule ref="PHPCompatibilityWP"/>

	<config name="minimum_supported_wp_version" value="5.3"/>

	<exclude-pattern>/src/CompiledContainer\.php</exclude-pattern>

</ruleset>

Expected behavior PHP_CS should run with the gitstaged filter.

Versions (please complete the following information):

  • OS: Windows 10, using WSL2 with Ubuntu 20.04
  • PHP: 7.4.20
  • PHPCS: 3.5.8
  • Standard: Eightshift (custom)

dingo-d avatar Aug 17 '21 15:08 dingo-d

@dingo-d While Windows is case-insensitive, Linux is not. I think your issue will be fixed if you use --filter=GitStaged instead 😀

jrfnl avatar Aug 17 '21 15:08 jrfnl

Thanks for the tip, I'll give it a go 👍🏼

dingo-d avatar Aug 18 '21 07:08 dingo-d

Ok, so setting it to GitStaged fixed throwing of the PHP errors, but I don't get any error reported, and when I run composer standards:check I get the error in the staged file (I introduced it intentionally).

I'll try to investigate what happens when I run one vs the other command with some additional flags.

EDIT:

Ok, so something is odd with the realpath utility. Because when I check the $path variable here https://github.com/squizlabs/PHP_CodeSniffer/blob/b245bb315dae6dc7347681e4f5a55dd7bdfcd045/src/Filters/GitStaged.php#L50

I get the path to be wp-content/themes/my-theme/src/CustomPostType/GuidesAndResourcesPostType.php after it's passed to the Util\Common::realpath helper, it gets converted to boolean false.

Could this be because of WSL? I think my colleagues on mac are getting the same thing, so it could be due to Linux as well (since I am running Ubuntu 20.04 in WSL, and they are using MacOS unix based terminal).

dingo-d avatar Aug 19 '21 09:08 dingo-d

@dingo-d I would suspect this is to do with WSL, so would be good if you could check whether your colleagues are getting this issue.

IIRC from previous reports, the PHP native file system functions don't all support WSL well enough.

Also see #2965 and #3388.

jrfnl avatar Aug 19 '21 13:08 jrfnl

I'll poke this around since I have a Mac available and a Win PC that I can use, so I'll try to see what is happening, and report back 👍🏼

dingo-d avatar Aug 19 '21 13:08 dingo-d

@dingo-d I've created PR #3428 to address a similar issue. That PR may solve your issue as well. Would you be able to test this and leave feedback on the PR ?

jrfnl avatar Sep 06 '21 06:09 jrfnl

I've added the changes from the patch, but I'm still not getting an error when using the GitStaged filter

test

dingo-d avatar Sep 06 '21 06:09 dingo-d

Hmm... darn... Looking at the code in the GitStaged class, I suspect the use of Util\Common::realpath() may be problematic.

Might be something we could debug together ?

jrfnl avatar Sep 06 '21 06:09 jrfnl

Sure, just say when 😄

dingo-d avatar Sep 06 '21 07:09 dingo-d