sirbrillig / phpcs-changed

🐘 Run phpcs on files and only report new warnings/errors compared to the previous version.
MIT License
31 stars 11 forks source link

Unable to run in git mode on directories #94

Closed lems3 closed 1 year ago

lems3 commented 1 year ago

Version : phpcs-changed version 2.11.2

Steps to reproduce :

This is the stack trace I get :

Fatal error: Uncaught PhpcsChanged\InvalidOptionException: You must supply at least one file or directory to run in git mode. in /app/vendor/sirbrillig/phpcs-changed/PhpcsChanged/CliOptions.php:372
Stack trace:
#0 /app/vendor/sirbrillig/phpcs-changed/PhpcsChanged/CliOptions.php(257): PhpcsChanged\CliOptions->validate()
#1 /app/vendor/sirbrillig/phpcs-changed/bin/phpcs-changed(78): PhpcsChanged\CliOptions::fromArray(Array)
#2 [internal function]: {closure}(Object(SplFileInfo), 'docroot/sites/e...', Object(RecursiveDirectoryIterator))
#3 [internal function]: CallbackFilterIterator->accept()
#4 /app/vendor/sirbrillig/phpcs-changed/bin/phpcs-changed(83): FilterIterator->rewind()
#5 /app/vendor/bin/phpcs-changed(120): include('/app/vendor/sir...')
#6 {main}
  thrown in /app/vendor/sirbrillig/phpcs-changed/PhpcsChanged/CliOptions.php on line 372

Meanwhile, running phpcs-changed --git --git-base main settings/settings.php works.

My goal would be to run this on our pull requests and see if the code changes follows our PHP CS rules.

Using a glob doesn't fix the issue either.

After a bit of diggint, I found that :

In the bin/phpcs-changed file, while iterating the list of files and folder, you use CliOptions::fromArray($options) to validate file extensions for files. But at that moment the files index of the $options array is empty, since we're still building up the file list.

I did some changes in the code to hardcode a first valid value in the $options["files"] index, and now passing folders to the command line works.

sirbrillig commented 1 year ago

Weird. This could have been a regression in the refactors from 2.11. I'll see what I can fix.

sirbrillig commented 1 year ago

Oh, this is actually worse than the one issue! Since creating CliOptions runs that validation, it also breaks other commands like phpcs-changed -i.

lems3 commented 1 year ago

For now I do a git diff --names-only and feed it to the command. Woth the --git and --git-base flags it works really nice.

It may be a bit more efficient for us as our codebase have a lot of files. 😂

sirbrillig commented 1 year ago

https://github.com/sirbrillig/phpcs-changed/pull/95 should fix this.