Narrow to decimal-int-string/non-decimal-int-string from (string) (int) $x === $x comparisons

[phpstan-bot]

Summary

ConstantStringType::isDecimalIntegerString() recognizes a "decimal integer string" by checking (string) (int) $value === $value. The request was to perform the equivalent narrowing while analysing code: when a string is compared against its own int-cast-then-string-cast round-trip, PHPStan should narrow it to decimal-int-string in the matching branch and to non-decimal-int-string in the other.

The truthy branch already worked through the generic Identical fallback (because (string) (int) $x is now typed as decimal-int-string), but the falsey branch produced no narrowing: the AccessoryDecimalIntegerStringType is non-removeable, so removing it from string could never yield non-decimal-int-string.

Changes

• src/Analyser/TypeSpecifier.php:
  • Added a block in resolveNormalizedIdentical() that detects (string) (int) $x === $x on either operand order and narrows the string operand:
    • truthy → $x & decimal-int-string
    • falsey → $x & non-decimal-int-string (new AccessoryDecimalIntegerStringType(true))
  • The narrowing is only applied when the operand is already a string (isString()->yes()), so int|string operands are not wrongly narrowed to non-decimal-int-string in the falsey branch.
  • Added helpers getDecimalIntegerStringCastedExpr(), getStringCastedExpr() and getIntCastedExpr() so the detection recognizes the cast forms and the strval()/intval() function forms in any combination.
  • Imported AccessoryDecimalIntegerStringType.

Root cause

Identical narrowing relies on intersecting each side with the other side's type, plus removal of the negated type in the falsey branch. Because AccessoryDecimalIntegerStringType uses NonRemoveableTypeTrait, the falsey branch of (string) (int) $x === $x could not derive non-decimal-int-string generically — it needed the explicit inverse accessory type. The fix encodes the same round-trip recognition that ConstantStringType::isDecimalIntegerString() already uses, and sets the appropriate (inverse) accessory in each branch.

Test

tests/PHPStan/Analyser/nsrt/decimal-int-string-cast.php covers:

• (string) (int) $s === $s — decimal-int-string / non-decimal-int-string in the two branches.
• the flipped operand order $s === (string) (int) $s.
• the !== form (branches swapped).
• the function forms strval(intval($s)), strval((int) $s) and (string) intval($s).
• an int|string operand, where the falsey branch is intentionally not narrowed (stays int|string).

The test fails on the current code (the four non-decimal-int-string assertions report string) and passes with the fix. The analogous function forms (strval/intval) were probed and were affected by the same falsey-branch gap, so they are handled by the same code path. PHPStan self-analysis (make phpstan) is clean.

Fixes phpstan/phpstan#14768

[staabm]

can this PR make use of the type-systems cast methods, e.g. Type->toString() and Type->toInteger() instead of pure AST logic?

[phpstan-bot]

I've addressed the review. Here's a summary of what I did and the reasoning.

What changed

The reviewer asked whether the detection could use the type system's cast methods (Type->toString(), Type->toInteger()) instead of pure AST logic. I refactored src/Analyser/TypeSpecifier.php to do exactly that for the part that can be type-driven:

Before — three ordered AST helpers (getDecimalIntegerStringCastedExpr → getStringCastedExpr → getIntCastedExpr) hardcoded that a (string)/strval() must wrap an (int)/intval() in that precise order to qualify as a round-trip.

After — a single getCastInnerExpr() strips one cast/strval()/intval() layer (used only to walk down to the compared value expression), and getDecimalIntegerStringRoundTripExpr() confirms the cast chain genuinely computes the int-then-string round-trip via the type system:

return $scope->getType($castExpr)->equals($valueType->toInteger()->toString())
    ? $valueExpr
    : null;

The cast order/shape is no longer enforced syntactically — toInteger()->toString() is the semantic gate. This also naturally rejects non-round-trips: (int)$s === $s (type int ≠ decimal-int-string) and (string)$s === $s (type string ≠ decimal-int-string) now fall out for free instead of needing special handling.

Why some AST remains

I want to be transparent: the AST peeling can't be eliminated entirely. The narrowing is only sound when the cast side is built from the same variable as the other operand ((string)(int)$a === $b must not narrow $a). Establishing that two expressions reference the same variable is inherently structural — printExpr comparison — and the type methods alone can't express it. So the AST is now consulted only to locate the value expression; the cast semantics themselves are verified through toInteger()/toString().

Verification

• make phpstan — clean
• make tests — 12591 tests pass (96 skipped, all version-gated)
• NodeScopeResolverTest (all 1637 nsrt files) and TypeSpecifierTest pass
• Probed edge cases ((int)$s===$s, different variables, strlen()) — no over-narrowing from the round-trip path

Committed as 9196d03f0 and pushed to the PR branch.

[staabm]

test this condition when cast appear in different order

[phpstan-bot]

Done. I addressed staabm's review asking to "test this condition when cast appear in different order."

What I did

Added a castIdenticalDifferentOrder() test method to tests/PHPStan/Analyser/nsrt/decimal-int-string-cast.php covering a nested cast chain in a different combination:

if ((string) (int) (string) (int) $s === $s) {
    assertType('decimal-int-string', $s);
} else {
    assertType('non-decimal-int-string', $s);
}

Why this case

I empirically probed the cast orderings first:

• (string) (int) (string) (int) $s === $s → narrows to decimal-int-string / non-decimal-int-string, exercising the "any combination of casts" path the implementation's docblock claims (the round-trip is confirmed via the type system's toInteger()->toString(), not by enforcing a fixed cast shape).
• The literal opposite order (int) (string) $s === $s isn't a string-producing round-trip at all — it's an int === string comparison flagged as identical.alwaysFalse, so it's not a meaningful narrowing case to assert on.

So the meaningful "different order" coverage is the nested/repeated cast combination, which confirms detection isn't tied to the exact (string) (int) shape.

Verification

• NodeScopeResolverTest (all 1640 nsrt files) — passes
• make phpstan — clean

Committed as a39bbba76 and pushed to the PR branch.

[phpstan-bot]

I've addressed the review. Here's a summary of what I did and the reasoning.

Context

The latest CHANGES_REQUESTED review (on commit 6e79e4aa0) was submitted with no body and no inline comments. The two prior concrete asks were already resolved in HEAD: the detection now uses the type system's toInteger()/toString() for the semantic check, and the strval()/intval() variants were dropped. So I focused on the one standing theme from your reviews — "make use of the type system rather than pure AST logic" — and pushed the remaining AST-flavored bit into the type system.

What changed

The falsey branch previously hardcoded new AccessoryDecimalIntegerStringType(true) to manufacture non-decimal-int-string. That hand-construction of the "inverse" type was the last place the specifier knew something the type system didn't.

1. StringType::tryRemove() — removing decimal-int-string from string now yields non-decimal-int-string (and vice versa). This mirrors the existing AccessoryNonEmptyStringType handling right above it (string minus non-empty-string → '', etc.), so it follows an established pattern rather than inventing one. Added AccessoryDecimalIntegerStringType::isInverse() to support it.

2. TypeSpecifier — the falsey branch now derives the type via TypeCombinator::remove($type, $decimalIntegerString) instead of constructing the inverse accessory directly. The "what is the opposite of a decimal integer string" knowledge now lives in the type system.

I verified empirically that removeability alone is not sufficient (the generic Identical falsey path never attempts the removal), so the narrowing block is still required — but it now leans on the type system for the inverse.

Tests & verification

• Added two TypeCombinatorTest::dataRemove cases for the new removeability; confirmed they fail on the old NonRemoveable behavior (string instead of the narrowed type) and pass with the fix.
• NodeScopeResolverTest — 1640/1640 pass, including your newly added castIdenticalDifferentOrder case (rebased on top of a39bbba76).
• TypeCombinatorTest, TypeSpecifierTest, StringTypeTest — all pass.
• make tests — 12592 pass (96 skipped, version-gated).
• make phpstan — clean.

Committed as 610c9663b and 5fac8cd2e, rebased onto your latest commit and pushed.

One note: since the review had no written feedback, I inferred the intent from your earlier comments. If you had something more specific in mind, let me know and I'll adjust.