Mark TestCase::$backupGlobals as nullable

[morozov]

TestCase::$backupGlobals is a nullable boolean:

phpunit/src/Framework/TestCase.php

Line 1200 in a4baf33

if ($this->backupGlobals === null && $backupGlobals !== null) {

Fixes #4307.

[codecov]

Codecov Report

Merging #4308 into 8.5 will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##                8.5    #4308   +/-   ##
=========================================
  Coverage     84.22%   84.22%           
  Complexity     3937     3937           
=========================================
  Files           154      154           
  Lines          9784     9784           
=========================================
  Hits           8241     8241           
  Misses         1543     1543           

Impacted Files	Coverage Δ	Complexity Δ
src/Framework/TestCase.php	81.69% <ø> (ø)	340.00 <0.00> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4baf33...91a29cf. Read the comment docs.

[sebastianbergmann]

@Ocramius roave/backward-compatibility-check considers the change from bool to bool|null (which should be ?bool) to be a breaking change:

[BC] CHANGED: Type documentation for property PHPUnit\Framework\TestCase#$backupGlobals changed from bool to bool|null

I am not 100% convinced that this is correct.

[Ocramius]

@sebastianbergmann any reads from that property can no longer operate under the assumption that a boolean is contained in there: I am aware that phpunit documented it as bool while it stores a null when not initialized, but that's not something the tool can understand, as it only relies on the documented type.

[Ocramius]

Note: the tool is saying (in the message) that the inferred type (from docblock) changed, which is why the wording "documentation" appears in the report.

It cannot understand if this is an actual change in type, so it assumes that the actual type changed.

[morozov]

As per the above, it's not a BC break. The property documented as bool could also contain NULL. Now, this is documented but there's no change in the behavior observed by the API consumers.

[sebastianbergmann]

@morozov When (not if) I merge this then I'll update the --from parameter for roave/backward-compatibility-check in the CI pipeline.