Narrow to decimal-int-string/non-decimal-int-string from (string) (int) $x === $x comparisons#5805
Conversation
…) (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.
700a601 to
ff4a32d
Compare
staabm
left a comment
There was a problem hiding this comment.
can this PR make use of the type-systems cast methods, e.g. Type->toString() and Type->toInteger() instead of pure AST logic?
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>
|
I've addressed the review. Here's a summary of what I did and the reasoning. What changedThe reviewer asked whether the detection could use the type system's cast methods ( Before — three ordered AST helpers ( After — a single return $scope->getType($castExpr)->equals($valueType->toInteger()->toString())
? $valueExpr
: null;The cast order/shape is no longer enforced syntactically — Why some AST remainsI 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 ( Verification
Committed as |
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 astringis compared against its own int-cast-then-string-cast round-trip, PHPStan should narrow it todecimal-int-stringin the matching branch and tonon-decimal-int-stringin the other.The truthy branch already worked through the generic
Identicalfallback (because(string) (int) $xis now typed asdecimal-int-string), but the falsey branch produced no narrowing: theAccessoryDecimalIntegerStringTypeis non-removeable, so removing it fromstringcould never yieldnon-decimal-int-string.Changes
src/Analyser/TypeSpecifier.php:resolveNormalizedIdentical()that detects(string) (int) $x === $xon either operand order and narrows the string operand:$x & decimal-int-string$x & non-decimal-int-string(new AccessoryDecimalIntegerStringType(true))string(isString()->yes()), soint|stringoperands are not wrongly narrowed tonon-decimal-int-stringin the falsey branch.getDecimalIntegerStringCastedExpr(),getStringCastedExpr()andgetIntCastedExpr()so the detection recognizes the cast forms and thestrval()/intval()function forms in any combination.AccessoryDecimalIntegerStringType.Root cause
Identicalnarrowing relies on intersecting each side with the other side's type, plus removal of the negated type in the falsey branch. BecauseAccessoryDecimalIntegerStringTypeusesNonRemoveableTypeTrait, the falsey branch of(string) (int) $x === $xcould not derivenon-decimal-int-stringgenerically — it needed the explicit inverse accessory type. The fix encodes the same round-trip recognition thatConstantStringType::isDecimalIntegerString()already uses, and sets the appropriate (inverse) accessory in each branch.Test
tests/PHPStan/Analyser/nsrt/decimal-int-string-cast.phpcovers:(string) (int) $s === $s—decimal-int-string/non-decimal-int-stringin the two branches.$s === (string) (int) $s.!==form (branches swapped).strval(intval($s)),strval((int) $s)and(string) intval($s).int|stringoperand, where the falsey branch is intentionally not narrowed (staysint|string).The test fails on the current code (the four
non-decimal-int-stringassertions reportstring) 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