pestle icon indicating copy to clipboard operation
pestle copied to clipboard

Use PHP 5.5 ::class constant to reference class names in generated code.

Open Vinai opened this issue 7 years ago • 2 comments

In many places in Magento 2 modules class names are referenced.
For example, a CRUD model references the resource model

    protected function _construct()
    {
        $this->_init('Example\FooBar\Model\ResourceModel\Thing');
    }

Since PHP 5.5 the ::class constant is available, which has the following benefits over using strings to refer to classes:

  • IDEs can understand it is a class, so they can provide
    • Autocompletion
    • Show errors if the class doesn't exist
    • Apply changes to the class name during refactorings
    • Support navigation to the class
  • Static code analysis tools (phpcs, phpmd, pdepend, IDEs, ...) can correctly find all usages of the class
  • Class imports and aliasing can be used to make the code more readable

It would be nice if the generated code would use the ::class constant when referring to class names instead of strings, for example:

    protected function _construct()
    {
        $this->_init(\Example\FooBar\Model\ResourceModel\Thing::class);
    }

or, in most cases, if the referring class is already in a related namespace, only the sub-namespaces would be enough. The following example assumes the current class is in the Example\FooBar\Model namespace:

    protected function _construct()
    {
        $this->_init(ResourceModel\Thing::class);
    }

Vinai avatar Apr 26 '17 05:04 Vinai

It's probably time for this as well -- best approach here is probably to use the magento2:generate:full-module command to generate a module, then identify the class strings, and then replace them with the ::class constant.

Also, and this is purely a taste issue, I think we'll go with full class names in the code instead of the partial class names. The problem/trade-off with something like

protected function _construct()
{
    $this->_init(ResourceModel\Thing::class);
}

is, by itself, you can't tell if ResourceModel is relative to the current current namespace or if ResourceModel is an alias imported via a use statement. That, plus when you're exploring a codebase it's not always obvious what namespace you're currently in. I realize these trade-offs are biased towards someone who spends more time exploring code than writing it, and who uses a programmer's text editor vs. fully featured IDE, but, well, what's the point of having your own open source project if you don't impose your own biases every so often :)

astorm avatar Apr 26 '17 16:04 astorm

As long as the ::class constant is used instead of strings I'm happy with either. The IDE makes importing classes fun, so no problem with that.

On a somewhat related side note, however, it would be nice to have descriptive "base names" of classes, so that they still read well if imported. For example, instead of using \Magento\Framework\ObjectManager\ConfigInterface the core team should have used \Magento\Framework\ObjectManagerConfigInterface.

Both work equally well when the fully qualified class name is written, but when the class is imported the latter produces much more readable code.

Vinai avatar May 04 '17 07:05 Vinai