[Polyfill] Add UnwrapFutureCompatibleIfRector#2569
Conversation
| private const FUNCTIONS_BY_VERSION = [ | ||
| '5.6' => ['session_abort'], | ||
| ]; |
There was a problem hiding this comment.
@staabm Here is just the code example you showed me. I need your help to complete list as far as possible.
There was a problem hiding this comment.
I need to think about how to compile such method most easily
There was a problem hiding this comment.
After thinking about it I feel we could compile this list from exisiting polyfil libs, e.g.
https://github.com/symfony/polyfill-php56
https://github.com/symfony/polyfill-php70
https://github.com/symfony/polyfill-php71
https://github.com/symfony/polyfill-php72
https://github.com/symfony/polyfill-php73
https://github.com/symfony/polyfill-php74
There was a problem hiding this comment.
Another thing I checked via github code-search
https://github.com/search?l=PHP&q=function_exists&type=Code
There we can see most examples are like
if( ! function_exists( 'supernova_comment' ) ){
function supernova_comment($comment, $args, $depth) {
// impl
}
}
In these cases we could eleminate the code when we are sure that the function beeing checked is one from the php standard library (so semantically the code is polyfilling, not e.g. guarding against „function already exist“ error because of multiple including the same file)
There was a problem hiding this comment.
After thinking about it I feel we could compile this list from exisiting polyfil libs, e.g.
Great!
Another thing I checked via github code-search
Try to keep issues separated, so we don't get stuck with complexity. This is another use case.
There was a problem hiding this comment.
There are many new PRs merged every day so there are new conflicts to rebase on.
I'll merge this now to prevent that.
You can send the function list as a new PR targeting this class 👍
There was a problem hiding this comment.
@staabm Any update on this? The rule is pretty lonely with just 1 function now :D
There was a problem hiding this comment.
Was on vacation till yesterday.. will need a few more days to catch up
TomasVotruba
force-pushed
the
unwrap-compat
branch
from
ef014c5 to
87ad2d6
Compare
TomasVotruba
force-pushed
the
unwrap-compat
branch
from
87ad2d6 to
8501bc2
Compare
| continue; | ||
| } | ||
|
|
||
| if (! $this->phpVersionProvider->isAtLeast($version)) { |
There was a problem hiding this comment.
I guess it would also be helpfull to remove code when explicit php version checks are used (might be a separTe rector then?)
There was a problem hiding this comment.
Sure! I though of that too.
Could you help by making a new issue with minimal code (3-lines) in diff a format?
TomasVotruba
force-pushed
the
unwrap-compat
branch
from
8501bc2 to
25b1a0e
Compare
TomasVotruba
force-pushed
the
unwrap-compat
branch
from
25b1a0e to
a37a7d1
Compare
rectorphp/rector-src@89a684f [Core] Apply Scope refresh for Namespace_ and FileWithoutNamespace (#2569)
2 participants
Closes #2540