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.
From #drupal-contribute:
FrobinRobin
12:18 hi all, in D8 - why is "Drupal::moduleHandler" sometimes escaped? i.e. "\Drupal::moduleHandler->someMethod()"
12:20 well, I guess its any class method but thats the one I'm looking at..https://drupal.org/files/drupal-2045931-replace-module-exists-calls-15.patch
berdir
12:21 FrobinRobin: it's not escaping. It means to use that class from the global namespace and not the one the class is in
FrobinRobin
12:23 thanks berdir, Why would it be used sometimes and not others? examples in that patch (Drupal::moduleHandler()->moduleExists)
berdir
12:23 FrobinRobin: because procedural code (.module etc.) is in the global namespace too, so it's not necessary)
FrobinRobin
12:23 ahhh nice one berdir, that was a very quick and easy answer!
12:24 berdir++
webchick
12:24 FrobinRobin: (love that name) It's good to hear the kinds of things that trip you up though as you're first encountering D8. I can definitely see why that would be confusing.
berdir
12:24 webchick: yep, not the first time..
Ok, so let's fix it. :)
Comment | File | Size | Author |
---|---|---|---|
#23 | drupal-slash-2053489-23.patch | 15.74 KB | klausi |
#22 | drupal-slash-2053489-22.patch | 15.54 KB | klausi |
#20 | drupal-slash-2053489-20.patch | 14.46 KB | tim.plunkett |
#15 | 2053489_15.patch | 504.25 KB | chx |
#13 | webchick_happiness+=1200.patch | 504.24 KB | chx |
Comments
Comment #1
webchickAlthough, as a counter-point:
Though as a counter-counter point, that's great that it works even if you forget to put \ in front of it, but it'd still be less confusing if it were consistent throughout core, and the \ed version is the only one that can be.
Comment #2
webchickSeriously, now, this is getting absurd. From https://drupal.org/node/2082661:
Drupal 7:
Drupal 8:
D7 developers are not going to know whether they're in a namespace or not, at least until much later and they understand that stuff. Let's just standardize on \ everywhere.
Comment #3
damiankloip CreditAttribution: damiankloip commentedThat is a good example of how things are at the moment!
Sounds good to me though. If we're going down the route of just one, that's our only choice.
Comment #4
mtiftI definitely think this is a good idea.
Imagine that you are new to object-oriented programming and you are wondering when to use "\Drupal" vs "Drupal." What do you do? Search the web for "Drupal \Drupal"? While that does produce some helpful results, it seems just as likely that someone might not know to search for that and would instead be wondering if they should use \Drupal::moduleHandle, \Drupal::entityManager, \Drupal::config, etc.
For whatever it's worth, I know for a fact that I have seen people ask this question in issue queues, and I have not been able to find many examples by searching. So not to pick on anybody, but here is one I remember: #1158322-152: Add backtrace to all errors
I'm not saying developers could not figure this out -- just that it might be complicated to know how to find the answer to the question. So if we can make one thing less confusing about Drupal 8, I'm all for it.
Comment #5
tim.plunkettI wholeheartedly agree.
We either have
or in authorize.php, Drupal:: at the beginning of the line.
So it was pretty easy to replace them all.
git diff --color-words
showed that only \ was added.It's trivial enough to reroll this patch, but often a "needs review" issue will get more attention than an "active" one, so why not.
Comment #6
longwaveThe alternative of course is to
use Drupal;
in namespaced code where needed, and then drop the backslash everywhere.Comment #8
tim.plunkettNope, that leads to silly things like
Comment #9
longwaveWhy is that silly? Why have one standard for top-level classes, and another standard for namespaced classes? You also don't need the leading backslash in use statements.
Also note core/lib/Drupal/Core/Config/Schema/ArrayElement.php which already has five use statements like this.
Comment #10
webchickWe agreed over in #1614186: Coding standards for using "native" PHP classes like stdClass in namespaced code not to "use" anything without a backslash in their fully-qualified name. "Drupal" falls under this, so does "Exception" and "ArrayAccess." So this is merely bringing this part of the code in-line with the coding standard.
Comment #11
chx CreditAttribution: chx commentedThis patch comes in two parts because I was getting a request entity too large otherwise.
Comment #13
chx CreditAttribution: chx commentedSorry, botched up the regex, the correct one is
(?<!\\)Drupal::
and the replacement (in phpstorm at least)\\\\Drupal::
Hope this works.Comment #15
chx CreditAttribution: chx commentedOMG core changed since I started rolling it :P
Comment #17
tim.plunkett#15: 2053489_15.patch queued for re-testing.
Comment #18
webchickIt's green! GREEEEENNN!!
Committed and pushed to 8.x. Thanks!
I'm going to combine the change notice for this one with #1614186: Coding standards for using "native" PHP classes like stdClass in namespaced code which, miraculously, doesn't fail to apply with this patch. So marking fixed.
Comment #19
webchick.
Comment #20
tim.plunkett#1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed reintroduced a couple dozen Drupal:: calls with no leading \.
Comment #21
andypostAnother one added in #2062023: Replace user_access() calls with $account->hasPermission() in statistics module
Comment #22
klausiAdded those two.
Verified that we are doing nothing else here but adding back slashes to Drupal::.
Comment #23
klausiGnah, wrong patch format in #22.
Comment #24
alexpottCommitted 3742257 and pushed to 8.x. Thanks!
Comment #25
cosmicdreams CreditAttribution: cosmicdreams commentedrelated follow up #2100329: Use \Drupal instead of Drupal to make IDEs and static code analyse tools happy