--no-patch option returns error code 1 on composer script

[gmponos]

Inside my composer I have added the following command/script

{
...
    "scripts": {
        "phpcbf": [
            "php vendor/bin/phpcbf mypath1 --standard=ruleset1.xml --extensions=php --no-patch",
            "php vendor/bin/phpcbf mypath2 --standard=ruleset2.xml --extensions=php --no-patch"
        ],
    }
...
}

When I execute

composer phpcbf I get

Script php vendor/bin/phpcbf mypath1 --standard=ruleset1.xml --extensions=php --no-patch handling the phpcbf event returned with error code 1

As a result the second script is never executed. If I remove the --no-patch option the second script is executed.

Version: 2.7.1

[aik099]

According to #666 you need to add --runtime-set ignore_warnings_on_exit 1 to when running phpcbf and then it will always return 0.

If I'm not mistaken the error code tells, that something was fixed.

[gmponos]

Sorry I closed it without testing it.
The problem continues.

Script php vendor/bin/phpcbf path --standard=ruleset1.xml --extensions=php --no-patch --runtime-set ignore_warnings_on_exit 1 handling the phpcbf event returned with error code 1

[gmponos]

Furthermore no file is fixed.

[gsherwood]

When using the --no-patch option, PHPCS doesn't know if any errors were actually fixed because there is no patch file to look at, so it has always return 1 (see https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/CLI.php#L230).

This isn't that useful because it means you can't actually check the return value of PHPCS. But also, patching using diff was a waste of time, so it was removed in 3.0a1. The 3.0 version no longer has a --no-patch option because it never uses patch and diff - it only overwrites files like --no-patch does.

I changed the return value in 3.0 to return the number of errors found, which is a little better but, on reflection, still not good. 3.0 has a concept of "total number of fixed errors" so I think I can probably get the return value working a bit better in there. I've already changed the return values of PHPCS in 3.0, so I don't think there is a problem changing PHPCBF a bit more as well.

But I don't want to mess with return values in the 2.x version, especially given I don't think they will be accurate. So for now, you'll need to ignore the return value.

I'll try and fix up the 3.0 version and commit that as a fix for this issue, but you'll need to upgrade to 3.0 before you can accurately check the return value of PHPCBF.

[gsherwood]

I've changed the PHPCBF exit codes to this:

• Exit code 0 is now used to indicate that no fixable errors were found, and so nothing was fixed
• Exit code 1 is now used to indicate that all fixable errors were fixed correctly
• Exit code 2 is now used to indicate that PHPCBF failed to fix some of the fixable errors it found
• Exit code 3 is now used for general script execution errors

Those mirror what the current 2.x version tries to do when using patch and diff, but it is hopefully a bit more accurate. And it's obviously a big improvement over both the --no-patch 2.x version and the 3.0.0RC2 version.

Do those exit codes look sane to you?