Rector rewrites nullable null checks into fully-qualified instanceof checks

[loks0n]

Raised in review on #94: #94 (comment)

The code

packages/pools/src/Pools/Pool.php, in reclaim():

/**
 * @param  Connection<TResource>|null  $connection  Reclaims every active connection when null.
 */
public function reclaim(?Connection $connection = null): static
{
    if ($connection instanceof \Utopia\Pools\Connection) {
        return $this->push($connection);
    }

    foreach ($this->active as $active) {
        $this->push($active);
    }

    return $this;
}

The instanceof was not hand-written. It was produced by Rector from if ($connection !== null).

Positions

@abnegate: keep the null check instead of the type check. The parameter is declared ?Connection, so !== null says exactly what the branch means — "was a connection passed?" — and the rewritten form says something narrower than the intent.

The argument for the rule: !== null only proves not-null. If the runtime value is ever wider than the declaration — a docblock-only type, a mixed, a union with false in it — the null check passes values the body cannot handle, while instanceof proves the thing you are about to call methods on. The same file has a genuine instance of that case in pop(), where $this->adapter->pop() returns Connection|false|null and instanceof Connection is load-bearing, not cosmetic — the old code needed $connection === false || $connection === null to cover it. The rule cannot tell the two situations apart; it fires on any nullable object type.

The rule

Rector\CodeQuality\Rector\Identical\FlipTypeControlToUseExclusiveTypeRector — "Flip type control from null compare to use exclusive instanceof object". Its documented sample:

// before
function process(?DateTime $dateTime)
{
    if ($dateTime === null) {
        return;
    }
}

// after
function process(?DateTime $dateTime)
{
    if (! $dateTime instanceof DateTime) {
        return;
    }
}

It matches Identical and NotIdentical nodes where one side resolves to null, asks PHPStan's NullableTypeAnalyzer whether the other side is a nullable object type, and if so replaces the comparison with instanceof (wrapped in ! for the === null direction). It emits a FullyQualified name node, which is why the output is \Utopia\Pools\Connection and not Connection — even though Pool and Connection share the Utopia\Pools namespace and the short name is already in scope. Pint does not shorten it back; nothing in pint.json touches class references.

Interaction with PHPStan and types

Both forms narrow identically for PHPStan when the declared type really is ?Connection, so the rewrite buys no analysis strictness here. The value the rule adds is where the declaration and the runtime type diverge — precisely the case a stricter PHPStan level is supposed to eliminate. packages/pools/phpstan.neon runs level 8; the monorepo baseline phpstan.neon is level 5. So the rule's payoff varies by package: closer to redundant at level 8, more defensible at level 5, where nullable-object types are less reliably enforced.

Our setup

• rector.php at the repo root enables the rule via the codeQuality prepared set, alongside deadCode, typeDeclarations, earlyReturn, phpunitCodeQuality and withPhpSets(). The rule is not listed individually and there is no withSkip().
• rector/rector: ^2.4 is a root dev dependency; packages do not require it themselves.
• bin/monorepo check runs rector process --dry-run per package (--fix drops --dry-run), falling back to the root rector.php when a package has no rector.php of its own. No package currently has one.
• bin/monorepo check <package> runs in CI in .github/workflows/tests.yml, so a --dry-run diff fails the build. The rewrite is not advisory.
• On main today there are 159 instanceof uses across packages/*/src and none are fully-qualified, so this rewrite has not landed anywhere yet — feat(pools)!: constructor-only configuration, one acquisition budget #94 is the first place it surfaces.

Scope of the decision

The rule is configured monorepo-wide, so this is a monorepo-wide call, not a pools one: either every nullable-object null check becomes a fully-qualified instanceof, or the rule is skipped everywhere and the pop()-style cases are written by hand.