PHPCSStandards / PHP_CodeSniffer / 14730902924

79%

push

github

Fixer::fixFile(): bug fix for incorrect return value (#1048)

Over the years, I'd noticed on numerous occasions that `phpcbf` could update the file under scan, even when the end-result of the run was a "FAILED TO FIX".
In my opinion, if the fixer didn't manage to fix the file completely, the file should not be updated as the end-result may be worse than before.

The trouble was that this didn't _always_ happen, but only some of the time, so this needed some debugging to figure out under what exact conditions the file write would happen (and when it wouldn't happen).

To demonstrate the issue, run the following commands with the branch for this PR checked out and the change in the `Fixer` file reverted:
```bash
phpcbf --suffix=.fixed ./tests/Core/Fixer/Fixtures/test.inc --standard=./tests/Core/Fixer/FixFileReturnValueNotEnoughLoopsTest.xml
```
Now check the `./tests/Core/Fixer/Fixtures/` directory and take note that there is no `test.inc.fixed` file present.
Next, run:
```bash
phpcbf --suffix=.fixed ./tests/Core/Fixer/Fixtures/test.inc --standard=./tests/Core/Fixer/FixFileReturnValueConflictTest.xml
```
Now check the `./tests/Core/Fixer/Fixtures/` directory again and see that the `test.inc.fixed` file has been created.

If you run these commands with `-vv` you can see what is happening in the fixer, including seeing a _"=> Fixed file written to test.inc.fixed"_ debug notice at the end of the debug output for the second command, but not seeing it for the first command.

_(you can now delete the `test.inc.fixed` file)_

Turns out that if the last loop of the Fixer didn't make any fixes, the `Fixer::fixFile()` method always returned `true` (= everything fixed), even when there were no fixes made _due to the fixer having discarded the fixes as it detected a fixer conflict..._.
This, in turn, would then cause the `CBF` report, which triggers the fixer and checks the return value of the `fixFile()` method, to do a file write with the "fixed" content of the f... (continued)

1 of 1 new or added line in 1 file covered. (100.0%)

25065 of 32003 relevant lines covered (78.32%)

67.37 hits per line