stevegrunwell / wp-enforcer

Git hooks to encourage well-written WordPress.
https://stevegrunwell.com/blog/wp-enforcer/
MIT License
112 stars 15 forks source link

Process only PHP files / add additional excludes #12

Closed leewillis77 closed 8 years ago

leewillis77 commented 8 years ago

Out-of-the-box the plugin won't let me commit the changes to add it to my repo. It looks like it's running PHPCS over everything, not just PHP files. I'm not sure whether the desired solution here would just be to add those files (Which should be common to anyone using this?) to the same phpcs.xml as excluded paths? or to make PHPCS only called on PHP files? Or is there something funky going on for me?

$ git commit -m 'Trialling WP enforcer for pre-commit awesomeness'
Running PHP CodeSniffer...

FILE: ...t/.gitignore
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/composer.json
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/composer.lock
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/phpcs.xml
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

Time: 17ms; Memory: 4Mb
stevegrunwell commented 8 years ago

The root of this appears to be the changes that were introduced in #1 – by explicitly passing files into phpcs, PHP_CodeSniffer will ignore the file extension:

If you have asked PHP_CodeSniffer to check a specific file rather than an entire directory, the extension of the specified file will be ignored. The file will be checked even if it has an invalid extension or no extension at all.

The trick here will be to filter out the changed files that aren't of a natively-checked extension before passing them to phpcs (e.g. Get staged files > filter the list > run PHP_CodeSniffer). We'll also need to make this list of extensions editable in some way, as it can be modified from the phpcs.xml file.

stevegrunwell commented 8 years ago

I believe I have a patch that will work in dd5470cc5a992a5e150ee05302e48fbbe355199d, but it doesn't take the repo's phpcs.xml file into account. Truly fixing this may mean parsing XML (yuck) and dynamically building the filter.

bswatson commented 8 years ago

@stevegrunwell - I know it'll be a pain, but I think we need to defer to the phpcs.xml and figure out a solution that takes those include / excludes into affect.

stevegrunwell commented 8 years ago

@bswatson agreed. Ideally there would be a flag in PHP_CodeSniffer to say "here are files to check but let the ruleset determine what gets used"...maybe a PR to CodeSniffer is in order.

stevegrunwell commented 8 years ago

Ha, got it sorted out. By adding the following to our phpcs.xml file we can skip the "No PHP code was found in this file and short open tags are not allowed by this install of PHP. This file may be using short open tags but PHP does not allow them." error:

<!--
    There is a special internal error message produced by PHP_CodeSniffer
    when it is unable to detect code in a file, possible due to
    the use of short open tags even though php.ini disables them.
    You can disable this message in the same way as sniff messages.

    Again, the code here will be displayed in the PHP_CodeSniffer
    output when using the -s command line argument while checking a file.
 -->
 <rule ref="Internal.NoCodeFound">
  <severity>0</severity>
 </rule>

This means the check will never get run, but it's only run when PHP_CodeSniffer thinks it's reading a PHP file, short_open_tag is disabled, and no code is detected (relevant code).

Reference: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml

leewillis77 commented 8 years ago

Yes - works for me - thanks Steve (For this & for the repo in general!)

stevegrunwell commented 8 years ago

@leewillis77 Absolutely. Thanks for being patient as we've been getting over a few post-launch hurdles!