Skip to content

Commit 81898d6

Browse files
committed
Report/Code: fix fatal potential fatal error when combined with Diff report
Okay, so this is an awkward one. If both the `Code` + the `Diff` report were requested + caching was turned on + the _order_ of the requested reports was `Code,Diff` (i.e. first the code report), the following fatal error which would occur: ``` Fatal error: Uncaught Error: Call to a member function getFixableCount() on null in path/to/PHP_CodeSniffer/src/Fixer.php:144 Stack trace: #0 path/to/PHP_CodeSniffer/src/Reports/Diff.php(73): PHP_CodeSniffer\Fixer->fixFile() #1 path/to/PHP_CodeSniffer/src/Reporter.php(285): PHP_CodeSniffer\Reports\Diff->generateFileReport(Array, Object(PHP_CodeSniffer\Files\LocalFile), true, 150) #2 path/to/PHP_CodeSniffer/src/Runner.php(659): PHP_CodeSniffer\Reporter->cacheFileReport(Object(PHP_CodeSniffer\Files\LocalFile)) #3 path/to/PHP_CodeSniffer/src/Runner.php(400): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile)) #4 path/to/PHP_CodeSniffer/src/Runner.php(119): PHP_CodeSniffer\Runner->run() #5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS() #6 {main} thrown in path/to/PHP_CodeSniffer/src/Fixer.php on line 144 ``` To reproduce the issue (on `master`): 1. Create a small test file like: ```php <?php echo 'hello'.callMe( $p ); ``` 2. Run the following command to initialize the cache: ```bash phpcs -ps ./test.php --standard=PSR2 --report=Code,Diff --cache ``` All should be fine. 3. Now run the same command again and see the fatal error. Some investigating later, it turns out that both the `Code` report, as well as the `Diff` report, re-parse the current file, with the `Diff` report initializing the fixer once the file has reparsed, but the `Code` report not doing so (as it doesn't need the fixer). The problem with that is that the `Code` report basically leaves the `Fixer` in an invalid state, leading to the above error. While it is up for debate whether reports should ever (be allowed to) re-parse files, for now, let's fix the fatal. Re-evaluating the report setup should definitely be done, but should probably wait until the Reports classes have test coverage.
1 parent 50f03b3 commit 81898d6

File tree

1 file changed

+4
-0
lines changed

1 file changed

+4
-0
lines changed

src/Reports/Code.php

+4
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false,
5555

5656
try {
5757
$phpcsFile->parse();
58+
59+
// Make sure the fixer is aware of the reparsed file to prevent a race-condition
60+
// with the Diff report also re-parsing the file.
61+
$phpcsFile->fixer->startFile($phpcsFile);
5862
} catch (Exception $e) {
5963
// This is a second parse, so ignore exceptions.
6064
// They would have been added to the file's error list already.

0 commit comments

Comments
 (0)