Avoid using static access to Exception

Question

I've just fired up PHPMD for the first time and, predictably, I've got an error I can't figure out. The error is

Avoid using static access to class 'InvalidArgumentException' in method 'setLang'.

and the code is

public function setLang($val0) {
    switch ($val0) {
    case ENG:
    case FRE:
    case SPA;
        $this->lang = $val0;
        break;
    default:
        throw new InvalidArgumentException("Invalid language choice.");
    }
}

I've tried a variety of different things but I think at the end of the day Exception is a static factory (???) so it must have static access. But, the PHPMD guys are for sure smarter than me so that wouldn't have fazed them.

Why does this warning exist, and how to solve it?

Answer 1

The idea behind this warning is that if you embed the class names inside your code with the new keyword it will be hard to swap out these classes in testing and mock or stub methods that the code under test might call on them. See the explanation in the PHPMD rules.

I think in your case this is false positive since exceptions usually doesn't have much behavior on their own but the class name (and class hierarchy behind it) of them is pretty much the only thing important about them.

If you want to get rid of the warning here, you can use the @SupressWarnings annotation here.

Answer 2

Ahh, after a bit of digging around I have found the answer, straight from the horse's mouth.

In the configuration files, located at yourphpdir\data\PHP_PMD\resources\rulesets, the cleancode.xml has CDATA comments which explain the setting.

This one says:

Static acccess causes inexchangable dependencies to other classes and leads to hard to test code. Avoid using static access at all costs and instead inject dependencies through the constructor. The only case when static access is acceptable is when used for factory methods.

The way to solve it is just pass the exception as a parameter, so it's declared outside of the class.

$foo->setLang("Oh noes!", new InvalidArgumentException("No lang for you."));

Answer 3

Because I use a lot of self:: for constants, change phpmd code to accept self:: and parent::.

In the program PHP/PMD/Rule/CleanCode/StaticAccess.php at line 36, change to:

if ($this->isReferenceInParameter($reference)
    || $reference->getImage() === 'self' 
    || $reference->getImage() === 'parent' 
    ) {
    continue;
}

Maybe you can use that to refine the code.

Answer 4

If you use an XML ruleset, you can exclude certain classes (full namespace) from this rule:

    <rule ref="rulesets/cleancode.xml">
        <exclude name="StaticAccess"/>
    </rule>
    <rule ref="rulesets/cleancode.xml/StaticAccess">
        <properties>
            <property name="exceptions" value="\Drupal\views\Views,\Drupal\Core\Access\AccessResult" />
        </properties>
    </rule>