Runner: allow suppressing "No files were checked" error #1291
Conversation
schlndh
force-pushed
the
feature-allowSupressingNoFilesWereCheckedError
branch
from
7dd1a98 to
61fcf51
Compare
|
@schlndh Thanks for this PR. Some feedback:
|
src/Runner.php
Outdated
|
|
||
| $numFiles = count($todo); | ||
| if ($numFiles === 0) { | ||
| if ($numFiles === 0 && $this->config->allowEmptyFileList === false) { |
There was a problem hiding this comment.
This is wrong. I believe the flag (whatever it's going to be called) should only affect the exit code, not the error message.
The way this is coded now, if the flag is set, PHPCS would try to process an empty file list, while there is nothing to do.
There was a problem hiding this comment.
I use --report=json to achieve baseline-like behavior (gist: I'm not sure whether it's up-to-date). Therefore, I want phpcs to output JSON instead of nothing to stdout. Though, if necessary I could handle the empty output as well.
There was a problem hiding this comment.
That's not a reason to break things for everyone else.
There was a problem hiding this comment.
I'm not sure I understand. I'm adding a new, non-default option to suppress the "No files were checked" error. I suppose that some people might prefer to have the error message and a 0 exit code, but I'm not breaking anything for them: they can simply keep the default behavior.
I'm definitely interested in not having the error message at all. Would you be willing to accept multiple configuration values? Something like empty_file_list: prohibit (default, error + exit code), warn (error message only, 0 exit code), allow (no error message, 0 exit code)?
I plan to use it as a config option via |
And that's perfectly possible with a config flag, just as well as with a CLI option, so I don't see why that would be a reason to have a CLI option. |
|
For reference for anyone coming across this PR and wanting to understand the larger context: |
Fixes squizlabs/PHP_CodeSniffer#3556 The "No files were checked" error can be undesirable in situations where an external tools passes a list of files to phpcs to check, without necessarily knowing whether the files are excluded or not. Consider the following scenarios: - CI running only on changed files. - Git hooks. - Phpstorm PHPCS inspection (currently it gives an error notification on excluded files).
schlndh
force-pushed
the
feature-allowSupressingNoFilesWereCheckedError
branch
from
0cdbdec to
fbb065b
Compare
2 participants
Description
Fixes squizlabs/PHP_CodeSniffer#3556
Suggested changelog entry
Added
--allow-empty-file-listargument to suppress "No files were checked error".Related issues/external references
Fixes squizlabs/PHP_CodeSniffer#3556
Types of changes
PR checklist