Implement AccessoryDecimalIntegerStringType::tryRemove() and preserve accessories when re-unioning '0'#5816
Open
phpstan-bot wants to merge 9 commits into
Open
Conversation
staabm
reviewed
Jun 6, 2026
staabm
reviewed
Jun 6, 2026
…ve accessories when re-unioning `'0'` - `AccessoryDecimalIntegerStringType` previously used `NonRemoveableTypeTrait`, so removing the constant string `'0'` from a `decimal-int-string` left the type unchanged. `"0"` is the only falsy decimal integer string, so removing it now yields `decimal-int-string&non-falsy-string` (mirrors the existing handling in `AccessoryNonEmptyStringType`/`AccessoryNumericStringType`). - Generalized the `'0' | non-falsy-string` union simplification in `TypeCombinator::compareTypesInUnion()`. The old code matched only the bare `non-falsy-string` via `describe()` string comparison, so `'0' | (numeric-string&non-falsy-string)` and the decimal-int-string variant were left as unsimplified unions. New helper `downgradeNonFalsyStringToNonEmpty()` drops the `AccessoryNonFalsyStringType`, keeps every other accessory, and only re-adds `AccessoryNonEmptyStringType` when the remaining accessories do not already guarantee non-emptiness — so the round trip collapses back to `decimal-int-string` / `numeric-string` / `non-empty-string`. - Probed sibling accessory string types: `AccessoryLowercaseStringType`, `AccessoryUppercaseStringType` and `AccessoryLiteralStringType` can still be the empty string, so removing `'0'` must not make them non-falsy — they correctly remain `NonRemoveableTypeTrait`. The inverse `non-decimal-int-string` does not contain `'0'`, so its removal is a no-op.
…sy-string Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… for removing '0' Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
staabm
force-pushed
the
create-pull-request/patch-2f7geta
branch
from
4cf1a1b to
6746dc6
Compare
staabm
reviewed
Jun 7, 2026
…Remove() Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sory tryRemove()" This reverts commit a9e2c09.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removing the constant string
'0'from adecimal-int-stringdid not change thetype — PHPStan still considered the result possibly falsy, even though
"0"is theonly falsy decimal integer string. After
if ($s !== '0')(or any truthynarrowing),
$sshould bedecimal-int-string&non-falsy-string. This PR makes thatnarrowing work and also fixes the inverse simplification so unioning
'0'back incollapses to the original type.
Changes
src/Type/Accessory/AccessoryDecimalIntegerStringType.php: droppedNonRemoveableTypeTraitand implementedtryRemove(). For a non-inversedecimal-int-string, removing the constant'0'returnsstring & decimal-int-string & non-falsy-string. The check usesType::getConstantStrings()rather thaninstanceof ConstantStringType.src/Type/TypeCombinator.php: replaced thedescribe()-based'0' | …union-simplification branches with a new
downgradeNonFalsyStringToNonEmpty()helper that preserves all accessory refinements (numeric-string,
decimal-int-string, lowercase-string, …) while downgrading the non-falsy flag to
non-empty, only adding
AccessoryNonEmptyStringTypewhen no remaining accessoryalready implies non-emptiness. Bumped the corresponding
instanceof IntersectionTypebaseline count.
tests/PHPStan/Analyser/nsrt/decimal-int-string.php: added regression assertions.Root cause
Accessory string types encode refinements that interact with falsy-ness.
AccessoryNonEmptyStringTypeandAccessoryNumericStringTypealready convert to anon-falsy variant when
'0'is removed (theirtryRemove()), butAccessoryDecimalIntegerStringTypeusedNonRemoveableTypeTraitand so silentlyignored the removal — the same family bug, one sibling left unfixed.
The mirror operation (union) had a parallel gap:
compareTypesInUnion()onlyrecognized the exact
non-falsy-stringdescription when folding'0'back in, soany non-falsy string that also carried another accessory
(
numeric-string&non-falsy-string, the decimal-int variant, lowercase, …) was leftas an un-simplified
'0' | (…&non-falsy-string)union. Matching ondescribe()isexactly the fragile pattern called out in CLAUDE.md; the new helper works
structurally on the intersection members instead.
Test
tests/PHPStan/Analyser/nsrt/decimal-int-string.phpnow asserts:decimal-int-stringnarrows todecimal-int-string&non-falsy-stringafter!== '0',!= '0', and truthy checks; the negative branch is'0'.non-decimal-int-stringis unchanged by removing'0'(it never contains'0').'0' | …round-trips:decimal-int-string,numeric-stringandnon-empty-stringeach collapse back to themselves after$x !== '0' ? $x : '0'.All three round-trip and removal assertions fail on
2.2.xwithout the sourcechanges and pass with them. Full
make testsandmake phpstanare green.Fixes phpstan/phpstan#14785