Skip to content

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

Open
phpstan-bot wants to merge 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-zhyznk2
Open

Narrow to decimal-int-string/non-decimal-int-string from (string) (int) $x === $x comparisons#5805
phpstan-bot wants to merge 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-zhyznk2

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

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 === $sdecimal-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 staabm force-pushed the create-pull-request/patch-zhyznk2 branch from 700a601 to ff4a32d Compare June 6, 2026 13:41
Copy link
Copy Markdown
Contributor

@staabm staabm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Collaborator Author

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 (getDecimalIntegerStringCastedExprgetStringCastedExprgetIntCastedExpr) 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 intdecimal-int-string) and (string)$s === $s (type stringdecimal-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.

phpstan-bot and others added 2 commits June 7, 2026 11:08
…) (int) $x === $x` comparisons

- Add a special case to `TypeSpecifier::resolveNormalizedIdentical()` that detects
  the canonical decimal-int round-trip `(string) (int) $x === $x` (the same check
  `ConstantStringType::isDecimalIntegerString()` performs) and narrows `$x`.
- In the truthy branch `$x` is intersected with `AccessoryDecimalIntegerStringType`
  (`decimal-int-string`); in the falsey branch with its inverse
  (`non-decimal-int-string`). The truthy direction already worked through the
  generic identical fallback, but the falsey direction did not because the
  accessory type is non-removeable.
- Detection is symmetric in operand order and recognizes both the cast forms and
  the `strval()`/`intval()` function forms, in any mix (`strval((int) $x)`,
  `(string) intval($x)`, ...), via the new `getStringCastedExpr()` /
  `getIntCastedExpr()` helpers.
- Narrowing is only applied when the compared expression is already known to be a
  string, so `int|string` operands keep their int part in the falsey branch.
Instead of structurally requiring a `(string)` cast to wrap an `(int)` cast in
that exact order, strip the cast/`strval()`/`intval()` layers off one operand to
locate the compared value expression, then confirm the casts really compute the
int-then-string round-trip via `Type::toInteger()->toString()`. The AST is now
consulted only to match the value expression; the cast semantics are checked
with the type system's cast methods.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm staabm force-pushed the create-pull-request/patch-zhyznk2 branch from 9196d03 to 5c62c33 Compare June 7, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

narrow to decimal-int-string via cast

2 participants