Problem/Motivation

I found plenty of plugin managers which use string literals for class names.
These should use ::class instead.

Steps to reproduce

Proposed resolution

Use ::class to refer to classes and interfaces.
For now the scope is plugin managers.

In case of name clash of the class alias (e.g. attribute vs annotation) we can use other shortcuts like `Annotation\Something`, where we might import the respective Annotation namespace.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3494360

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

donquixote created an issue. See original summary.

donquixote’s picture

donquixote commented
Status: Active » Needs review

donquixote opened merge request !10582

donquixote’s picture

donquixote commented

In case of name clash of the class alias (e.g. attribute vs annotation) we can use other shortcuts like `Annotation\Something`, where we might import the respective Annotation namespace.

Too bad...

Namespaced classes/interfaces/traits should be referenced with use statements

Alternative is to use an alias.

An example where the namespace import is used is Drush commands, where we have `use Drush\Attributes as CLI;` and then `#[CLI\Command(...)]`.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

> I believe part of the reason that the Annotation class references weren't changed to ::class strings was that they were going to be gone with the removal of annotation discovery anyway, but since not all the plugins have been converted, it's probably going to be a while for that.

Exactly this. That was done on purpose, as we otherwise have two classes with the same name that we have to alias. That means we also need to adjust the use statements when we remove tem, which will make the cleanup more complex, with more risk of conflicts with other issues.

donquixote’s picture

donquixote commented

Exactly this. That was done on purpose, as we otherwise have two classes with the same name that we have to alias.

I removed the questionable commit.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appear to be some open threads. Believe it’s close

donquixote’s picture

donquixote commented

I removed imports reordering in those files where no new imports are added.

donquixote’s picture

donquixote commented
Status: Needs work » Needs review
godotislate’s picture

godotislate
he/him
commented

Looks like everything was addressed. PHP Unit Build job needs a re-run and we're probably there.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

So applied the MR and did a search for "$namespaces, $module_handler, 'Drupal\Core\" to make sure those were converted and all instances were.

@godotislate there is 1 thread I wasn't 100% should be resolved but your comment in #10 made think it maybe could? Could you close it.

There are 2 threads but really it's 1.

Rest of the feedback appears to be addressed so going to mark it.

godotislate’s picture

godotislate
he/him
commented

@godotislate there is 1 thread I wasn't 100% should be resolved but your comment in #10 made think it maybe could? Could you close it.

It's not my MR, so I can't close it. But the question was resolved.

smustgrave’s picture

smustgrave commented

Thanks for checking!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 84af964 and pushed to 11.x. Thanks!

  • alexpott committed 84af9646 on 11.x 
    Issue #3494360 by donquixote, smustgrave, godotislate, berdir: Use ::...

Status: Fixed » Closed (fixed)

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