squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.67k stars 1.48k forks source link

Print a message if no files were checked during a run #1595

Closed ht-bs closed 4 years ago

ht-bs commented 7 years ago

We are using a custom file extension for cli scripts. While upgrading to phpcs 3 the issue arose that i can not check these files using the following invocation anymore:

phpcs --standard=path/ruleset.xml file.custom

The file is plainly ignored.

On further investigation via debugging i found the following:

After clarification of the desired behavior i am willing to work on a PR.

gsherwood commented 7 years ago

Contrary to the comment in the help output of phpcs, the extension filtering is active even for single files.

I obviously forgot to remove this when I rebuilt the filtering system for 3.0. All filters should now be applied to all files and directories, as well as STDIN. This was not previously the case.

Setting the extensions parameter using the command line parameters is not possible in my current environment (but works) and when setting this option in the config file or custom ruleset it is not honoured (/ not passed into the config-objects values).

I set the extensions CLI arg in one of my rulesets and it works fine. I'm checking .html files (these are templates) but using the PHP tokenizer for them. So I have this in my ruleset, which is working for me: <arg name="extensions" value="html/php" />

Try running PHPCS with the -vv CLI arg and it will show you how your ruleset has been processed. You should see something like this: => set command line value --extensions=html/php

ht-bs commented 7 years ago

Ok i did not set the value the right way in the ruleset, thanks for the hint, that works.

Filtering files that are explicitly passed is a bit confusing because there is no hint on whats happening, just empty output like no errors. Maybe there is the possibility to pass a notification about skipped files to the caller without breaking something.

jrfnl commented 7 years ago

Loosely related to #1592 where a file passed on the command line was also not scanned.

Maybe there is the possibility to pass a notification about skipped files to the caller without breaking something.

That sounds like a great idea!

Or maybe just a "x files scanned, y files were skipped due to filtering" closing statement at the end of each run or between gathering the files and the start of the scan ?

I believe you can already see which files are skipped with -v option ? If not, that would be a useful addition to. The "x files scanned, y files were skipped due to filtering" message would at least notify people things are working even when they don't use -p as I've seen questions about that before as well (Someone wondering if an external standard was running as they didn't get any results. Turned out their code was clean).

gsherwood commented 7 years ago

I believe you can already see which files are skipped with -v option ?

Not in this case because they are filtered out. If I showed this sort of output, PHPCS would be showing a line item for every test files, vendor file, etc that is skipped due to exclusion rules.

Or maybe just a "x files scanned, y files were skipped due to filtering" closing statement at the end of each run or between gathering the files and the start of the scan ?

For the same reason as the above, this is not easily achievable, but would also be quite verbose in other common cases.

Filtering files that are explicitly passed is a bit confusing because there is no hint on whats happening, just empty output like no errors.

I understand that you find this confusing, and I had people say the same thing when the opposite was happening (no filters applied to passed file paths). This is one of those no-win cases for me.

The empty output when everything is fine is also intentional. PHPCS has always acted like this to ensure it can be run as a cron job without emailing an admin on each run. PHPCS now has the -q for quiet mode, so I could decide to print a message when there are no errors, but I would only consider that in a major release.

The last major release added an error message when no sniffs were registered. This is an error state, as would be the checking of 0 files, so I could print a message if no files were checked at all. But I still couldn't tell you how many were skipped, and I wouldn't produce any output if at least one file was checked. Still, I would absolutely consider that if you think it would help you in this case.

jrfnl commented 7 years ago

This is an error state, as would be the checking of 0 files, so I could print a message if no files were checked at all.

Sounds like a good compromise. :+1:

williamdes commented 5 years ago
<arg name="extensions" value="php,json" />
gsherwood commented 4 years ago

From version 4, the following error will now be shown if no files were checked during a run:

ERROR: No files were checked
All specified files were excluded or did not match filtering rules
jrfnl commented 4 years ago

@gsherwood What would be the exit code when this happens ?

gsherwood commented 4 years ago

What would be the exit code when this happens ?

Exit code is 3 - same as the other error types