Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
We have some left behind get_called_class()
and incorrect get_class($this)
calls that there is a keyword static::class
that has replaced the need for that call to get the late binding class.
@see https://secure.php.net/get_called_class
Proposed resolution
Replace get_called_class()
and get_class($this)
with static::class
Remaining tasks
TBD
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
TBD
Comment | File | Size | Author |
---|---|---|---|
#24 | raw-interdiff-14-23.txt | 18.18 KB | jungle |
#23 | 2619482-23.patch | 120.6 KB | ravi.shankar |
#14 | interdiff_11-14.txt | 102.72 KB | ankitsingh0188 |
#14 | 2619482-14.patch | 119.9 KB | ankitsingh0188 |
#11 | 2619482-interdiff-10-11.txt | 2.83 KB | cburschka |
Comments
Comment #2
joelpittetThere are some legit calls, I think I got all the outliers.
Comment #4
joelpittetNM, render arrays... that's why fails. These may be the only 3 hold outs.
Comment #5
joelpittetNope, I'm wrong:) They are all correct.
Comment #7
joelpittetYes I know testbot! chill out;)
Comment #8
cburschkaSorry if I misunderstand the purpose of this old issue. I was about to create one with basically the same subject, before looking it up. I think that the problem was (and is) valid; the approach just wasn't right.
We can't use the keyword 'static' inside a string, since it will not be resolved correctly. Instead of replacing
[get_called_class(), 'method']
with'static::method'
the patch should have replaced it withstatic::class . '::method'
, which is indeed already used in a lot of places in core.Per this StackOverflow answer,
get_called_class()
is fully identical tostatic::class
(in PHP 5.5+), and the latter has a minor performance benefit.in 9.0.x, get_called_class() is still used in 18 files (14 of them in the form
[get_called_class(), '...']
), whilestatic::class
is used in 51 files (11 of them in the formstatic::class . '::...'
).Just for consistency (if negligible performance), we might want to look at this again.
Comment #9
cburschkaComment #10
cburschkaBad patch.
This one replaces
get_called_class()
withstatic::class
everywhere.A further replacement we might consider is replacing
static::class . '::method'
with[static::class, 'method']
everywhere. The two are semantically equivalent and the latter is used far more commonly in core.Comment #11
cburschkaget_class($this)
can also be replaced withstatic::class
.(There is no such replacement for
get_class()
in general.)Comment #12
daffie CreditAttribution: daffie commentedTalked to @alexpott on slack about this issue. He is ok with both changes:
get_called_class()
tostatic::class
andget_class($this)
tostatic::class
.The IS needs an update.
The changes in the patch look good. But when I do a code base search for "get_class($this)" I still get a 167 results after I applied the patch.
Comment #13
ankitsingh0188I am working on this.
Comment #14
ankitsingh0188Comment #15
siddhant.bhosale CreditAttribution: siddhant.bhosale as a volunteer and at QED42 commentedComment #17
cburschkaThe webdriver tests have always been a bit brittle, might be a random failure. Retesting.
Comment #18
siddhant.bhosale CreditAttribution: siddhant.bhosale as a volunteer and at QED42 commentedThe patch is applied successfully and looks good to be merged.
Comment #19
daffie CreditAttribution: daffie commentedUpdated the IS.
Comment #20
cburschkaConfirmed that all changes in the patch replace instances of '
get_called_class()/get_class($this)
withstatic::class
, and that there aren't any of them left after applying it.Comment #21
catchNeeds a re-roll.
Comment #22
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedWorking on this.
Comment #23
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedAdded reroll of patch #14.
Comment #24
jungleNo leftovers.
Comment #25
catchCommitted eafefd8 and pushed to 9.1.x. Thanks!