-
Notifications
You must be signed in to change notification settings - Fork 575
Narrow to decimal-int-string/non-decimal-int-string from (string) (int) $x === $x comparisons
#5805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
phpstan-bot
wants to merge
6
commits into
phpstan:2.2.x
Choose a base branch
from
phpstan-bot:create-pull-request/patch-zhyznk2
base: 2.2.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b9a1824
Narrow to `decimal-int-string`/`non-decimal-int-string` from `(string…
phpstan-bot 5d957a6
Confirm the decimal-int-string round-trip through the type system
phpstan-bot 6e79e4a
Drop strval()/intval() forms from decimal-int-string round-trip detec…
phpstan-bot a39bbba
Cover decimal-int-string round-trip with casts in a different combina…
phpstan-bot 610c966
Make decimal-int-string accessory removeable from string
phpstan-bot 5fac8cd
Derive non-decimal-int-string narrowing through TypeCombinator::remove
phpstan-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace DecimalIntStringCast; | ||
|
|
||
| use function PHPStan\Testing\assertType; | ||
|
|
||
| class Foo | ||
| { | ||
|
|
||
| public function castIdentical(string $s): void | ||
| { | ||
| if ((string) (int) $s === $s) { | ||
| assertType('decimal-int-string', $s); | ||
| } else { | ||
| assertType('non-decimal-int-string', $s); | ||
| } | ||
| } | ||
|
|
||
| public function castIdenticalFlipped(string $s): void | ||
| { | ||
| if ($s === (string) (int) $s) { | ||
| assertType('decimal-int-string', $s); | ||
| } else { | ||
| assertType('non-decimal-int-string', $s); | ||
| } | ||
| } | ||
|
|
||
| public function castIdenticalDifferentOrder(string $s): void | ||
| { | ||
| // the casts can appear in any combination as long as the chain | ||
| // computes the int-then-string round-trip | ||
| if ((string) (int) (string) (int) $s === $s) { | ||
| assertType('decimal-int-string', $s); | ||
| } else { | ||
| assertType('non-decimal-int-string', $s); | ||
| } | ||
| } | ||
|
|
||
| public function castNotIdentical(string $s): void | ||
| { | ||
| if ((string) (int) $s !== $s) { | ||
| assertType('non-decimal-int-string', $s); | ||
| } else { | ||
| assertType('decimal-int-string', $s); | ||
| } | ||
| } | ||
|
|
||
| public function notAlwaysString(int|string $s): void | ||
| { | ||
| if ((string) (int) $s === $s) { | ||
| assertType('decimal-int-string', $s); | ||
| } else { | ||
| // $s can still be an int here, so we cannot narrow to non-decimal-int-string | ||
| assertType('int|string', $s); | ||
| } | ||
| } | ||
|
|
||
| } | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test this condition when cast appear in different order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 totests/PHPStan/Analyser/nsrt/decimal-int-string-cast.phpcovering a nested cast chain in a different combination:Why this case
I empirically probed the cast orderings first:
(string) (int) (string) (int) $s === $s→ narrows todecimal-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'stoInteger()->toString(), not by enforcing a fixed cast shape).(int) (string) $s === $sisn't a string-producing round-trip at all — it's anint === stringcomparison flagged asidentical.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) — passesmake phpstan— cleanCommitted as
a39bbba76and pushed to the PR branch.