Php-cs-fixer: Return error code on invalid file

[julienfalque]

Currently, when PHP CS Fixer processes a file and somehow introduces a syntax error in it (because of a bug), the command exits with 0. IMO the command should exit with an error code in such case.

Maybe we could also add a new option to make the command fail even for files that are invalid _before_ being processed. WDYT?

[SpacePossum]

The first case sounds like a bug to me :)

Second case, sounds logical to me, so +1 as well

[keradus]

Currently, when PHP CS Fixer processes a file and somehow introduces a syntax error in it (because of a bug), the command exits with 0.

we have the fail-safe mechanism, that if "fixing" process would corrupt the file, we don't apply it. For that, the whole CLI took worked. I agree it may make sense to raise warning in such case, maybe even we already have it, not sure

Maybe we could also add a new option to make the command fail even for files that are invalid before being processed. WDYT?

we already have:

4 Some files have invalid syntax (only in dry-run mode).

[SpacePossum]

we have the fail-safe mechanism, that if "fixing" process would corrupt the file, we don't apply it. For that, the whole CLI took worked.

If a fixer find something that it should fix and than fails and causes an internal exception, we indeed do not apply it, however the file is than not fixed so the whole CI didn't work as expected (the result is that a file still needs fixing which wasn't done).

[dmvdbrugge]

I agree with @SpacePossum here. In a pre-commit hook, I want to abort when a file could not be fixed (aka it failed fixing). If however the cli-tool says it succeeds (exit 0), how can I know that it failed?

[dmvdbrugge]

To add even more to this: on lint exception, --dry-run --diff exits with 0 as well even though it has a diff and thus tried to fix the file. (So the 8 flag should've been set at least imho.)

_Edit: the --stop-on-violation is ignored too on I and E cases_

[keradus]

If a fixer find something that it should fix and than fails and causes an internal exception, we indeed do not apply it, however the file is than not fixed so the whole CI didn't work as expected (the result is that a file still needs fixing which wasn't done).

We dont know that. Internal exception can happen on anything. But good reasoning for linting-after-fixing error

[SpacePossum]

First part (exit code on lint failure after fixing) has been covered, second FR is still up for discussion.

[keradus]

What is a 2nd FR part here?

[julienfalque]

The second part is about returning a non zero code when the tool tried to fix a file that already has invalid syntax by making it global behavior, i.e. not only in dry-run mode, either by default or using a new option.

I wanted to start a discussion because I think it can be useful but finding files with invalid syntax can be done with other tools such as PHPStan anyway so I'm fine if we don't change the current behavior :)