magento / marketplace-eqp

Magento 1.x Coding Standard
http://docs.magento.com/marketplace/user_guide/Resources/pdf/Extension_Quality_Program_Overview.pdf
MIT License
224 stars 68 forks source link

A false positive: «resource is a reserved word in PHP 7» when reserved words are used in namespaces #78

Closed shochdoerfer closed 6 years ago

shochdoerfer commented 6 years ago

EQP complains that this code is not valid PHP code due to the usage of the reserved word "resource":

namespace BitExpert\ForceCustomerLogin\Test\Unit\Model\Resource;

Since our tests execute fine I tend to believe that this is indeed valid PHP code. Reserved words cannot be used for class names, interface names or trait names but it seems to be fine for namespaces.

shochdoerfer commented 6 years ago

For this piece of code

<?php

namespace int;

namespace float;

namespace bool;

namespace string;

namespace true;

namespace false;

namespace null;

namespace resource;

namespace object;

namespace mixed;

namespace numeric;

will php -l return No syntax errors detected.

lenaorobei commented 6 years ago

@shochdoerfer try to add

<?php

namespace bool;

class false
{

}

$false = new false();

and you will see Fatal error: Cannot use 'false' as class name as it is reserved. It leads to an error during class instantiation. Please find more here: http://php.net/manual/en/reserved.other-reserved-words.php

shochdoerfer commented 6 years ago

I think this is related to a different problem. Using reserved words in namespaces is valid, using reserved words in class names, interface names or trait names is not. You test case combines both and thus it fails. Even if you would remove the namespace declaration your code would not be valid.

This code executes fine:

<?php

namespace int;

class A{}

namespace float;

class A{}

namespace bool;

class A{}

namespace string;

class A{}

namespace true;

class A{}

namespace false;

class A{}

namespace null;

class A{}

namespace resource;

class A{}

namespace object;

class A{}

namespace mixed;

class A{}

namespace numeric;

class A{}

namespace main;

$i = new \int\A();
$f = new \float\A();
$b = new \bool\A();
$s = new \string\A();
$t = new \true\A();
$f = new \false\A();
$n = new \null\A();
$r = new \resource\A();
$o = new \object\A();
$m = new \mixed\A();
$nu = new \numeric\A();

While this does not - your example without the namespace declaration:

<?php

class false
{
}

$false = new false();

Would you accept a PR to remove the namespaces check from the sniff?

lenaorobei commented 6 years ago

@shochdoerfer, unfortunately, we will not accept the pull request, because according to PHP documentation:

The following words cannot be used to name a class, interface or trait, and they are also prohibited from being used in namespaces.