FunctionToConstantFixer - handle get_class() -> __CLASS__ as well #2944
Conversation
| @@ -31,8 +31,9 @@ | |||
| * @var string[] | |||
| */ | |||
| private static $availableFunctions = [ | |||
| 'phpversion' => 'PHP_VERSION', | |||
| 'get_class' => 'T_CLASS', | |||
There was a problem hiding this comment.
I think the constant you meant is __CLASS__?
There was a problem hiding this comment.
rotf, yeah thanks :)
| @@ -126,6 +126,30 @@ public function provideTestCases() | |||
| '<?php echo phpversion(); echo php_sapi_name(); echo M_PI;', | |||
| ['functions' => ['pi', 'phpversion']], | |||
| ], | |||
| 'get_class => T_CLASS' => [ | |||
There was a problem hiding this comment.
Please add a test case with a trait instead of a class and one with a call to get_class() from outside a class. I think the code should not be changed for the latter as the result is not the same (false + warning vs. "").
There was a problem hiding this comment.
Trait context can be fixed as well without any side effects: https://3v4l.org/i6M96
Checking that the call happening in a method makes sense (we don't do this, but I'll fix this), @SpacePossum is it possible or needs significant efforts?
There was a problem hiding this comment.
https://3v4l.org/pm1jC lemme check
There was a problem hiding this comment.
TBH I think it is an odd thing that this wasn't patched in PHP to act the same when they added the warning.
Not sure checking the calling get_class outside class "" vs. false is worth the effort. The reason for that is that it makes no sense anyway to do it, as all it is is a very expensive way to get false or ''.
There was a problem hiding this comment.
LGTM, skipping kalessil/phpinspectionsea#425 then as well.
There was a problem hiding this comment.
👍 regarding get_class() from outside a class. I would still add a test case with a trait though.
|
Awesome, thanks @SpacePossum ! |
SpacePossum
changed the title
keradus
changed the title
|
Thank you @SpacePossum. |
ping @kalessil :)