phpcbf does not report errors

[kkmuffme]

When a sniff has a PHP error, phpcbf just says "No fixable errors were found", whereas phpcs shows the sniff file + line + error message.

[jrfnl]

Too little information to go on. Also, not all errors are fixable.

[kkmuffme]

With PHP 8.1:

PHPCS says:

FILE: /some/file.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 | ERROR | An error occurred during processing; checking has been aborted. The error message was: trim(): Passing null to parameter #1 ($string) of type string is deprecated in
   |       | /my/directory/composer/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php on line 280
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 471ms; Memory: 50.01MB

PHPCBF says:

No fixable errors were found

However with PHP 7.4 phpcbf does find and fix errors. phpcbf aborts due to the error in that sniff file, without any indication that it encountered an error. But without running phpcs you would never know about it.

[jrfnl]

@kkmuffme This is not an issue in PHP_CodeSniffer, but this is a known issue with WordPressCS and has already been fixed. Unfortunately, the release has been delayed for reasons. Until the new release has been tagged, run WPCS on PHP 7.4.

[kkmuffme]

I know. This is just an example, I have about 20 more from different sniff providers.

The problem is not with the WPCS sniff, but that phpcbf does not output this error when it aborts, due to an internal error in a sniff.
We use phpcbf in CI with a slightly different ruleset than phpcs. We didn't realize that phpcbf wasn't running/auto-fixing for months, due to an error in a sniff that we don't have in our phpcs ruleset. And phpcbf just reported "No fixable errors were found" instead of giving an error.

[jrfnl]

We didn't realize that phpcbf wasn't running/auto-fixing for months, due to an error in a sniff that we don't have in our phpcs ruleset.

Just like phpcs, phpcbf will only run the sniffs which are in your ruleset, so this sounds unlikely.

All the same, your point about improving the user facing information when a PHP error occurs in one of the sniffs running fixers is well taken and something, which I agree, should be looked into.

[kkmuffme]

Just like phpcs, phpcbf will only run the sniffs which are in your ruleset, so this sounds unlikely.

I think you misunderstood me:
phpcbf ran, but output No fixable errors were found even though there were fixable errors. phpcbf just didn't find any, because it aborted due to a PHP error in a sniff.
What I want to say is: if any sniff has a PHP error, phpcbf will report No fixable errors were found, which is wrong.
And this led to phpcbf not fixing anything for us, as it aborted before it could fix anything, due to a PHP error in a sniff.

[jrfnl]

@kkmuffme I didn't misunderstand you.

[gsherwood]

I get what you're saying and it's pretty easy to replicate undesired behaviour with a poorly coded sniff. PHPCBF does throw an exception of type PHP_CodeSniffer\Exceptions\RuntimeException like PHPCS does, but it's unhandled. For my version of PHP 8.1, that means the script ends with a fatal error and a non-zero exit code (255 in this case). I get the exact same behaviour on PHP 7.4.

I definitely don't get a message that no fixable errors were found; PHPCBF dies well before that.

I agree that improvement would be good here, but I feel like there might be a solution there somewhere with how the script is being run or your PHP settings. Is is possible that fatal errors are being suppressed in some way?

[rayshahriar]

With PHP 8.1:

PHPCS says:

FILE: /some/file.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 1 | ERROR | An error occurred during processing; checking has been aborted. The error message was: trim(): Passing null to parameter #1 ($string) of type string is deprecated in
   |       | /my/directory/composer/vendor/wp-coding-standards/wpcs/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php on line 280
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 471ms; Memory: 50.01MB

PHPCBF says:

No fixable errors were found

However with PHP 7.4 phpcbf does find and fix errors. phpcbf aborts due to the error in that sniff file, without any indication that it encountered an error. But without running phpcs you would never know about it.

I was getting a similar message. Downgrading to PHP 8.0 seemed to clear the issue for now.

[kkmuffme]

@gsherwood

PHPCBF does throw an exception of type

Where exactly? Could you send me a link to the specific line of the phpcbf sourcewhere that should happen in that case? bc I definitely doesn't happen for me for notices caused by PHP 8.1 specific issues.

with how the script is being run

just basic CLI "php /.../phpcbf ..."

Is is possible that fatal errors are being suppressed in some way?

Fatal errors cannot be suppressed to not stop code execution in PHP

[jrfnl]

Fatal errors cannot be suppressed to not stop code execution in PHP

You may want to have a look at set_error_handler().... The fatals you are talking about are all catchable exceptions, so if PHPCS is run with a wrapper around it (like some IDEs do), this could well be happening.

[kkmuffme]

I think this was a misunderstanding: a "Fatal error" in my mind was an uncatchable exeption (or only catchable via register shutdown function last_error), like E_COMPILE_ERROR.

But @gsherwood you are right: we have a custom set_exception_handler that catches phpcbf errors (and logs them in sentry, which we didn't check for the CI pipeline...)
If that error would be handled in phpcbf like it is in phpcs, that would be great.

Issue is found though. Should I leave this open for that phpcbf change or close it?

[jrfnl]

Marking as duplicate of #2871