From 7eef328f4703620a6e7ebecb66b5fab9ca5015e3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 5 Sep 2021 04:18:18 +0200 Subject: [PATCH] PHP 8.1 | Runner::processChildProcs(): fix passing null to non-nullable bug The `DummyFile::__construct()` method expects a `string` as the first parameter, subsequently passes it to the `File::setContent()` method, which also expects a `string`, which then passes it to the `Common::detectLineEndings()` method, which again expects a `string`. Within the `Common::detectLineEndings()` method, the string is then passed to the PHP native `preg_match()` function, which (again) expects as `string` for the `$subject` parameter. When using PHPCS with parallel processing turned on, on a system which allows for parallel processing, the `DummyFile` class was, however, being instantiated in the `Runner::processChildProcs()` method with `null` as the content of the first parameter, which on PHP 8.1 leads to `preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated` deprecation notices. This deprecation notice was then caught as a `RuntimeException` in the `File::setContent()` method and passed on to the `File::addWarningOnLine()`, which called the `File::addMessage()` method. The `File::addMessage()` parameter then ran into trouble as the `$this->config` property has not been set up yet, as `DummyFile::__construct()` calls `File::setContent()` before calling the `parent::__construct()` method, leading to the `Undefined array key "cache"` notices which were making the build fail. Fixed now by passing an empty string instead of `null` as the `$content` for the `DummyFile` in the `Runner::processChildProcs()` method. This then leaves one more issue: the `DummyFile::__construct()` method contains a conditional code block which was only run when `$content !== null`. As this conditional code block is also not necessary to be run when an empty string would be passed to the constructor, changing this condition to `$content !== ''` makes that the condition can still match and maintains the efficiency tweak the condition was safeguarding. --- src/Files/DummyFile.php | 2 +- src/Runner.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Files/DummyFile.php b/src/Files/DummyFile.php index 3275bf0920..601430301d 100644 --- a/src/Files/DummyFile.php +++ b/src/Files/DummyFile.php @@ -38,7 +38,7 @@ public function __construct($content, Ruleset $ruleset, Config $config) // This is done by including: phpcs_input_file: [file path] // as the first line of content. $path = 'STDIN'; - if ($content !== null) { + if ($content !== '') { if (substr($content, 0, 17) === 'phpcs_input_file:') { $eolPos = strpos($content, $this->eolChar); $filename = trim(substr($content, 17, ($eolPos - 17))); diff --git a/src/Runner.php b/src/Runner.php index 03238cd59d..253ec91f9f 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -732,7 +732,7 @@ private function processChildProcs($childProcs) if (isset($childOutput) === false) { // The child process died, so the run has failed. - $file = new DummyFile(null, $this->ruleset, $this->config); + $file = new DummyFile('', $this->ruleset, $this->config); $file->setErrorCounts(1, 0, 0, 0); $this->printProgress($file, $totalBatches, $numProcessed); $success = false; @@ -756,7 +756,7 @@ private function processChildProcs($childProcs) } // Fake a processed file so we can print progress output for the batch. - $file = new DummyFile(null, $this->ruleset, $this->config); + $file = new DummyFile('', $this->ruleset, $this->config); $file->setErrorCounts( $childOutput['totalErrors'], $childOutput['totalWarnings'],