Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use class names as string literals #36

Open
larsroettig opened this issue May 22, 2018 · 4 comments
Open

Do not use class names as string literals #36

larsroettig opened this issue May 22, 2018 · 4 comments
Labels
experimental Should be implemented with "experimental" severity to try it out

Comments

@larsroettig
Copy link
Contributor

Rule

Warning for String-Representation in $this->createMock for Unit Test.

Reason

if you use String Representation typos can happen also it not nice for IDEs Usage Analyse

Implementation

Bad Example:

$this->createMock('\Foo\Bar\Baz');

Recommendation Example:

$this->createMock(\Foo\Bar\Baz::class);
@lenaorobei
Copy link

This issue is related not only to unit tests but to code in general. It is already implemented in EQP coding standard, as well as in Magento 2 core static checks.

https://github.com/magento/marketplace-eqp/blob/master/MEQP2/Sniffs/Classes/NameResolutionSniff.php

https://github.com/magento/magento2/blob/2.2-develop/dev/tests/static/framework/Magento/Sniffs/LiteralNamespaces/LiteralNamespacesSniff.php

@larsroettig
Copy link
Contributor Author

Hi @lenaorobei, cool thank you maybe we can discuss this at the community Meeting.

@schmengler schmengler added the on agenda of hangout This issue will be discussed in the next open hangout label May 28, 2018
@schmengler
Copy link
Collaborator

It should definitely become a rule not to use class names as string literals. What we need is a decision how to best integrate this rule into extdn-phpcs. I would rather not rely on a Magento code base being available and class_exists/interface_exists then only work with names from the extension itself.

@schmengler schmengler changed the title Warning for String-Representation in $this->createMock Do not use class names as string literals May 28, 2018
@schmengler
Copy link
Collaborator

A discussed idea was to look for arguments of certain methods like “ObjectManager::create()” but there are many possibilities where a class name can be used as string.

We will try to implement the rule with a regular expression instead, using a low warning level, and try out if any false positives occur. The MEQP rule can be included as an addition – if there is no Magento installation, it will just find nothing.

@schmengler schmengler added experimental Should be implemented with "experimental" severity to try it out and removed on agenda of hangout This issue will be discussed in the next open hangout labels Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Should be implemented with "experimental" severity to try it out
Projects
None yet
Development

No branches or pull requests

3 participants