Fixer::fixFile(): bug fix for incorrect return value
Description
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:
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:
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 file.
This PR fixes this snafu by also checking the conflict state when determining the return value for the method.
Includes tests specifically for this issue.
Suggested changelog entry
Fixed: the file under scan would sometimes be updated with partial fixes, even though the file "failed to fix".
Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)