-
-
Notifications
You must be signed in to change notification settings - Fork 416
[dead-code] Skip substr casting removal on PHP 7.x, as can return false|string #7449
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
Conversation
| return false; | ||
| } | ||
|
|
||
| return ! $this->phpVersionProvider->isAtLeastPhpVersion(PhpVersion::PHP_80); |
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.
PHPStan already detect it clearly when run on php 7.x
see https://phpstan.org/r/853f59a6-22ad-48ac-af93-66606b569613
on PHP 7.2 - 7.4 tab.
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.
But not on PHP 8.0+. This also allow to add our test with PHP 7.4 version.
465b05d to
fc45da9
Compare
| */ | ||
| $subClassName = (string) substr( | ||
|
|
||
| $subClassName = substr( |
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.
this cast is needed here as downgrade rule is not yet created.
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.
This should be fixed with incorrect input instead. This structure is clearly wrong.
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.
Ok, I will create separate PR target your branch for different logic to handle this.
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.
Great 👍 We'll need to understand what values are going in any why it returns false.
It should be fixed to return always string instead
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.
see #7450
fc45da9 to
0b21f6c
Compare
No description provided.