phpcbf returns exit-code 1 even if all errors were fixed

[caugner]

Describe the bug
phpcbf returns exit-code 1 even if it fixed all errors (i.e. there are no remaining errors):

PHP_CodeSniffer/src/Runner.php

Lines 221 to 224 in 68e0b06

	if ($this->reporter->totalFixable === 0) {
	// PHPCBF fixed all fixable errors.
	return 1;
	}

This prevents phpcbf to be run using lint-staged as part of a husky git pre-commit hook.

Code sample
package.json:

{
  "devDependencies": {
    "husky": "^4.2.5",
    "lint-staged": "^10.2.13",
  },
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.php": "vendor/bin/phpcbf"
  }
}

Custom ruleset
(Not relevant)

To reproduce
Steps to reproduce the behavior:

1. yarn install (or npm install) with above package.json
2. Make change to a PHP file so that it diverges from the coding standard and git add it.
3. Run git commit -m "test"
4. See that lint-staged rejects the commit because phpcbf returns a non-zero exit-code:

$ git commit -m "test"
husky > pre-commit (node v12.18.3)
[STARTED] Preparing...
[SUCCESS] Preparing...
[STARTED] Hiding unstaged changes to partially staged files...
[SUCCESS] Hiding unstaged changes to partially staged files...
[STARTED] Running tasks...
[STARTED] Running tasks for *.php
[STARTED] vendor/bin/phpcbf
[FAILED] vendor/bin/phpcbf [FAILED]
[FAILED] vendor/bin/phpcbf [FAILED]
[SUCCESS] Running tasks...
[STARTED] Applying modifications...
[SKIPPED] Skipped because of errors from tasks.
[STARTED] Restoring unstaged changes to partially staged files...
[SKIPPED] Skipped because of errors from tasks.
[STARTED] Reverting to original state because of errors...
[SUCCESS] Reverting to original state because of errors...
[STARTED] Cleaning up...
[SUCCESS] Cleaning up...

✖ vendor/bin/phpcbf:
F 1 / 1 (100%)



PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
C:\path\to\file.php                               5      0
----------------------------------------------------------------------
A TOTAL OF 5 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------

Time: 160ms; Memory: 8MB


husky > pre-commit hook failed (add --no-verify to bypass)

Expected behavior
Like eslint --fix and prettier --write, phpcbf should return exit-code 0 if it was able to fix all issues.

Alternatively, it should be possible to trigger this behaviour with a new option --ignore-fixed-on-exit.

Versions (please complete the following information):

• OS: Windows 10
• PHP: 7.3.21
• PHPCS: 3.5.6
• Standard: PSR2

Additional context
Workaround:

bin/phpcbf

#!/usr/bin/env php
<?php

use PHP_CodeSniffer\Runner;

include_once __DIR__.'/../vendor/squizlabs/php_codesniffer/autoload.php';

$runner   = new Runner();
$exitCode = $runner->runPHPCBF();
if ($exitCode === 1) {
    $exitCode = 0;
}
exit($exitCode);

package.json

{
  "lint-staged": {
    "*.php": "bin/phpcbf"
  }
}

[gsherwood]

The exit code are going to be revised in version 4 so that 0 always means success. That's being tracked on issue #2898.

It's not something that I can fix before that due to the BC break, so I'm going to close this issue in favour of the 4.0 one. But this issue will be referenced over there to ensure it is revisited as part of that development.

Thanks for the detailed report.

[koshuang]

fyi, I have a temporary solution for this by just creating a script file and then always return exit code 0.

tools/scripts/phpcbf.sh

#!/bin/bash

vendor/bin/phpcbf $@

exit 0

package.json

{
  //...
  "lint-staged": {
    "*.php": "tools/scripts/phpcbf.sh"
  }
}

[caugner]

@koshuang I wouldn't recommend always returning 0, because an exit-code > 1 could mean there were unfixable errors, and your approach doesn't prevent these to be committed.

Try this instead:

#!/usr/bin/env bash

vendor/bin/phpcbf $@

status=$?

[ $status -eq 1 ] && exit 0 || exit $status

[nelson6e65]

I'm using a similar approach for lint-staged integration, using a composer script instead:

https://github.com/nelson6e65/php-code-sniffer-helpers

// composer.json
{
  "scripts": {
    "cs:fix-filtered": ["NelsonMartell\\PhpCodeSniffer\\ComposerScripts::phpcbf"]
  }
}