Please start using PHP 5.5's "::class" instead of plain strings as FQCNs

[adragus-inviqa]

Steps to reproduce

1. Browse https://github.com/magento/magento2, and look for ::class

Expected result

1. Find something.

Actual result

1. Found nothing.

Even recent commits use plain strings. It's PHP 5.5+. Come on. Is there a reason why you haven't moved to using ::classalready? The more you use strings, the harder the change. I suggest you start using them in 3... 2... 1.

[Vinai]

💯

[dmitrii-fediuk]

#2794, #2962

[duffner]

💯 Let's keep Magento's code quality moving forward!!!! Definitely agree!

[ishakhsuvarov]

Thank you for this proposal.
Actually we already have this in works, most of the strings are going to be converted to the ::class notation and this change is going to be enforced with the static test.
I'll update this ticket with the internal issue ID and it's status in the nearest future.

[gamort]

Before rushing into using new[er] PHP features, please make sure you know what it is your doing.

The examples given are stupid. Using ::class is for when you are /outside/ of the class where you need the full name. When you are inside the class, you should use CLASS
http://php.net/manual/en/language.constants.predefined.php

IE:
$instanceName = ResultInterface::class;
BAD BAD BAD

What you might want, for clarity, could be:

   public function __construct(
        ObjectManagerInterface $objectManager,
        $instanceName = false
    ) {
        $this->objectManager = $objectManager;

// If the instance name was not specified, then use the class name and namespace as defined in this php file

       if ($instanceName) 
       {
          $this->instanceName = $instanceName;
       } else {
           $this->instanceName = __CLASS__;
      }
    }

---

Here I have assumed that if someone creates a subclass of Magento\Framework\View\TemplateEngine\Xhtml\ResultFactory it is preferred for the instanceName property to remain to be Magento\Framework\View\TemplateEngine\Xhtml\ResultFactory unless the constructor is specifically modified.

If it is preferred that the instance name FOR THIS CLASS should reflect the name of the actual class by default, then and only then does it become preferably to use ::class. However, in that case what it should be might be:
// If the instance name was not specified, then use the class name and namespace as defined for the class this object is actually being created from

       if ($instanceName) 
       {
          $this->instanceName = $instanceName;
       } else {
           $this->instanceName = static::class;
      }

As such, there is no reason to assume that there should even be a single instance of ::class being used anywhere in the code. Based on the current code, CLASS is more appropriate as it matches the current behaviour[to change the DEFAULT value of instanceName requires changing the constructor]

Moreover, since the Magento object manager passes the string value it thinks instance name should be to objects it creates - in general it won't make any difference how the class constructor assigns a default value. Changing thousands of files in a meaningless way is simply stupid. It was stupid with the first patch of this year[which modified every file to change the end year copyright date to 2016 for no reason] and it would be stupid here.

[adragus-inviqa]

Using ::Class is for when you are /outside/ of the class where you need the full name. When you are inside the class, you should use CLASS

I was more refering to the first case. And for test classes mostly and for wherever it's fit, not saying we should use ::class everywhere. Using anything indiscriminately is just plain stupid.

PS: I think you can get your point across without having to use the word "stupid" so many times. We get it: it's stupid.
PPS: Use formatting: __CLASS__ and triple back-ticks: https://guides.github.com/features/mastering-markdown/#examples:

public function __construct(
ObjectManagerInterface $objectManager,
$instanceName = false
) {
$this->objectManager = $objectManager;

// If the instance name was not specified, then use the class name and namespace as defined in this php file
if ($instanceName)
{
$this->instanceName = $instanceName;
} else {
$this->instanceName = __CLASS__;
}
}

[kandy]

Simple script with allow convert many place automatic

<?php

$tokens = token_get_all(file_get_contents($argv[1]));
$fs = false;
foreach ($tokens as $t) {
    $tt =  is_array($t) ? $t[1] : $t;
    if ($fs && !preg_match('~(\s|\\()+~', $tt)) {
        if ($tt == ')') {
            echo $tt;
        } else {
            echo  '\\' . trim($tt, "\\") . '::class';
        }
        $fs = false;
    } else {
        echo $tt;
    }
    if (in_array($tt, ['getObject', 'getMock', 'getMockBuilder'])) {
        $fs = true;
    }
}

[piotrekkaminski]

Internal issue MAGETWO-53555

[nevvermind]

You're still doing it: 3be18f3#diff-35047a76fe781049c19099b882f4d183R11

I've seen loads of snippets in magento SO site using strings, because I assume that's what they see in the codebase.

[tanberry]

http://devdocs.magento.com/guides/v2.0/coding-standards/code-standard-php.html

Please review and if you'd like to submit edits or enhancements, please create a PR on our devdocs repo! Thanks for all contributions.

[piotrekkaminski]

It should be fixed. Investigating why it appeared in recent commit.

[veloraven]

Closed as this issue has been already fixed in 2638640
If you still can reproduce it feel free to create new GitHub issue

[Ctucker9233]

Any plans to backport this issue or is it planned for 2.2 only?

[korostii]

As I understand it, no backport is planned "by default" and closed tasks are not tracked by the core team neither (see here for an detailed official Magento Inc.'s response about that).
In general, here's what you could do if you're interested to get it backported:
a) create a pull request pointing at 2.1-develop as described here
b) create a new GitHub Issue referencing the original one and asking to backport the fix into 2.1.
Additionally, mentioning a certain relevant Magento, Inc. employee somewhere in the discussion might help bringing attention to it.

[hostep]

I don't really see the point in backporting this? It brings no new features, it fixes no bugs, ...
If it does get backported, it will need its own version, just like what happened to version 2.1.5 where only the copyright headers were changed. This to avoid too much noise in the diffs between different versions where there are actual functional changes.

The only reason I can see for backporting this, is it will make backporting other fixes easier, as there will be less conflicts in the code.

Just my 2 cents :)

[korostii]

Personally, I don't really see the point in not backporting something right away (unless it contains backward-incompatible changes), even if it just fixes the code style.
Why keep the older branch outdated if you can spare a couple minutes to improve it as well, for the greater good? That goes for all those numerous issues which are announced as "fixed in develop" and then instantly closed.