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

gitstaged filter not working when called through npm husky precommit hook #3412

Open
dingo-d opened this issue Aug 17, 2021 · 9 comments
Open

Comments

@dingo-d
Copy link

dingo-d commented Aug 17, 2021

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)
@jrfnl
Copy link
Contributor

jrfnl commented Aug 17, 2021

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

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Aug 17, 2021
@gsherwood gsherwood moved this from Idea Bank to Track in PHPCS v3 Development Aug 17, 2021
@dingo-d
Copy link
Author

dingo-d commented Aug 18, 2021

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

@dingo-d
Copy link
Author

dingo-d commented Aug 19, 2021

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

foreach ($output as $path) {

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).

@jrfnl
Copy link
Contributor

jrfnl commented Aug 19, 2021

@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.

@dingo-d
Copy link
Author

dingo-d commented Aug 19, 2021

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 👍🏼

@jrfnl
Copy link
Contributor

jrfnl commented Sep 6, 2021

@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 ?

@dingo-d
Copy link
Author

dingo-d commented Sep 6, 2021

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

test

@jrfnl
Copy link
Contributor

jrfnl commented Sep 6, 2021

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 ?

@dingo-d
Copy link
Author

dingo-d commented Sep 6, 2021

Sure, just say when 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants