For class names that appear in routing.yml files, core sometimes prefixes these class names with "\" and sometimes doesn't.

For example, in system.routing.yml, observe that the first _controller uses "\Drupal" while the second in just uses "Drupal"

system.cron:
  path: '/cron/{key}'
  defaults:
    _controller: '\Drupal\system\CronController::run'
  options:
    no_cache: TRUE
  requirements:
    _access_system_cron: 'TRUE'

system.admin_compact_page:
  path: '/admin/compact/{mode}'
  defaults:
    _controller: 'Drupal\system\Controller\SystemController::compactPage'
    mode: 'off'
  requirements:
    _permission: 'access administration pages'

I found this to be a problem with class names appearing in _controller, _form, and _title_callback

The attached patch makes all class names start with "\Drupal".

CommentFileSizeAuthor
#8 3051231-8-prefix-class-names-with-backslash.patch21.37 KBTR
#2 3051231-patch-applied.png30.87 KBcindytwilliams
prefix-class-names-with-backslash.patch6.78 KBTR
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR created an issue. See original summary.

cindytwilliams’s picture

cindytwilliams CreditAttribution: cindytwilliams at Kanopi Studios commented
Status: Needs review » Reviewed & tested by the community
FileSize
3051231-patch-applied.png30.87 KB

Patch applies cleanly, and updates all instances of "Drupal" in system.routing.yml with "\Drupal". Navigations and links continue to work as expected. Marking RTBC.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Category: Bug report » Task
Priority: Normal » Minor
Issue tags: +Trivial patch of the month, +code cleanup

HAH! This has bothered me so many times :D 🤓

TR’s picture

TR CreditAttribution: TR commented

All I can say is "Attention to detail does not necessarily make code good. But INattention to detail frequently makes code bad."

Thanks for the "Trivial patch of the month" tag. I will treasure it forever. Although you know the issue is four months old ...

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs review

Are there other core routing declarations with the same problem? If so we should update them all in this issue.

TR’s picture

TR CreditAttribution: TR commented

@catch: This covers everything in all core *.routing.yml files - any key that might take a class name as a value in these files has been checked.

Do you mean other non-routing places, such as core.services.yml? (Yes, this and other services.yml files also have a similar problem)

Or are you talking about other types of non-routing yml files, like yml used for plugin discovery (*.links.action.yml, *.links.menu.yml, etc.) and yml migration templates? (Yes, these all also show a similar problem)

I am concerned that if we expand this to service declarations it will become too large and affect too many things. Especially since core.services.yml seems to have chosen a standard of NOT using fully qualified the class names, which is opposite of what we normally do in *.routing.yml etc.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Needs work

#4: well said!

@catch specifically asked about other *.routing.yml files, not *.services.yml. There are indeed many more. For example in editor.routing.yml, quickedit.routing.yml, and so on. We can quite easily fix all of those here too.

I should've checked that — I was a bit too excited to see this finally get fixed :D

TR’s picture

TR CreditAttribution: TR commented
Status: Needs work » Needs review
FileSize
3051231-8-prefix-class-names-with-backslash.patch21.37 KB

The source of my confusion is that I thought the original patch covered all *.routing.yml files, but now I see it did not, even though that was my intention.

Here is a new patch which this time really does fix all the *.routing.yml files.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Heh :)

Thanks for doing this! 👍

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 3d27d76 and pushed to 8.8.x. Thanks!

  • catch committed 3d27d76 on 8.8.x 
    Issue #3051231 by TR, cindytwilliams: Class names in *.routing.yml...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.