Skip to content

Commit

Permalink
PHP 8.1 | Runner::processChildProcs(): fix passing null to non-nullab…
Browse files Browse the repository at this point in the history
…le 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.
  • Loading branch information
jrfnl committed Sep 5, 2021
1 parent b10c327 commit 7eef328
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/Files/DummyFile.php
Expand Up @@ -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)));
Expand Down
4 changes: 2 additions & 2 deletions src/Runner.php
Expand Up @@ -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;
Expand All @@ -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'],
Expand Down

0 comments on commit 7eef328

Please sign in to comment.