UndefinedVariable - Code Metrics - Inspection of "Fix #714 static:: and self:: properties access" - phpmd/phpmd - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — feature/issue-714-fix-self-and... ( 38aaf5 )

by Kyle

created 2020-05-16 16:27 UTC

UndefinedVariable C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	316
Duplicated Lines	10.76 %

Coupling/Cohesion

Components	1
Dependencies	9

Importance

Changes

Metric	Value
dl	34
loc	316
rs	6.4799
c	0
b	0
f	0
wmc	54
lcom	1
cbo	9

15 Methods

Rating	Name	Duplication	Size	Complexity
B	apply()	0	21	6
A	collect()	0	11	1
A	collectGlobalStatements()	0	10	3
A	collectExceptionCatches()	12	12	4
A	collectListExpressions()	10	10	3
B	collectForeachStatements()	0	18	7
A	collectClosureParameters()	0	8	2
A	checkVariableDefined()	0	6	2
A	collectParameters()	0	12	2
A	collectAssignments()	0	16	4
A	collectPropertyPostfix()	12	12	4
A	addVariableDefinition()	0	8	2
A	getNode()	0	8	2
B	getVariableImage()	0	28	9
A	isNameAllowedInContext()	0	8	3

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like UndefinedVariable often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use UndefinedVariable, and based on these observations, apply Extract Interface, too.

<?php
/**
 * This file is part of PHP Mess Detector.
 *
 * Copyright (c) Manuel Pichler <[email protected]>.
 * All rights reserved.
 *
 * Licensed under BSD License
 * For full copyright and license information, please see the LICENSE file.
 * Redistributions of files must retain the above copyright notice.
 *
 * @author Manuel Pichler <[email protected]>
 * @copyright Manuel Pichler. All rights reserved.
 * @license https://opensource.org/licenses/bsd-license.php BSD License
 * @link http://phpmd.org/
 */

namespace PHPMD\Rule\CleanCode;

use PDepend\Source\AST\ASTArray;
use PDepend\Source\AST\ASTArrayIndexExpression;
use PDepend\Source\AST\ASTMemberPrimaryPrefix;
use PDepend\Source\AST\ASTPropertyPostfix;
use PDepend\Source\AST\ASTUnaryExpression;
use PDepend\Source\AST\ASTVariable;
use PDepend\Source\AST\ASTVariableDeclarator;
use PDepend\Source\AST\State;
use PHPMD\AbstractNode;
use PHPMD\Node\AbstractCallableNode;
use PHPMD\Node\ASTNode;
use PHPMD\Node\MethodNode;
use PHPMD\Rule\AbstractLocalVariable;
use PHPMD\Rule\FunctionAware;
use PHPMD\Rule\MethodAware;

/**
 * This rule collects all undefined variables within a given function or method
 * that are used by any code in the analyzed source artifact.
 */
class UndefinedVariable extends AbstractLocalVariable implements FunctionAware, MethodAware
{
    /**
     * @var array Self reference class names.
     */
    protected $selfReferences = array('self', 'static');

    /**
     * Found variable images within a single method or function.
     *
     * @var array(string)
     */
    private $images = array();

    /**
     * This method checks that all local variables within the given function or
     * method are used at least one time.
     *
     * @param \PHPMD\AbstractNode $node
     * @return void
     */
    public function apply(AbstractNode $node)
    {
        $this->images = array();

        $this->collect($node);

        foreach ($node->findChildrenOfType('Class') as $class) {
            foreach ($class->getMethods() as $method) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                $this->collect(new MethodNode($method));
            }
        }

        foreach ($node->findChildrenOfType('Variable') as $variable) {
            if (!$this->isNotSuperGlobal($variable)) {
                $this->addVariableDefinition($variable);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            }
            if (!$this->checkVariableDefined($variable, $node)) {

                $this->addViolation($variable, array($this->getVariableImage($variable)));
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            }
        }
    }

    /**
     * Collect variables defined inside a PHPMD entry node (such as MethodNode).
     *
     * @param AbstractNode $node
     */
    private function collect(AbstractNode $node)
    {
        $this->collectPropertyPostfix($node);
        $this->collectClosureParameters($node);

        $this->collectForeachStatements($node);

        $this->collectListExpressions($node);

        $this->collectAssignments($node);

        $this->collectParameters($node);
        $this->collectExceptionCatches($node);

        $this->collectGlobalStatements($node);
    }

    /**
     * Stores the given literal node in an global of found variables.
     *
     * @param \PHPMD\Node\AbstractNode $node
     * @return void
     */
    private function collectGlobalStatements(AbstractNode $node)
    {
        $globalStatements = $node->findChildrenOfType('GlobalStatement');

        foreach ($globalStatements as $globalStatement) {
            foreach ($globalStatement->getChildren() as $variable) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                $this->addVariableDefinition($variable);
            }
        }
    }

    /**
     * Stores the given literal node in an catch of found variables.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @return void
     */
    private function collectExceptionCatches(AbstractCallableNode $node)
    {
        $catchStatements = $node->findChildrenOfType('CatchStatement');

        foreach ($catchStatements as $catchStatement) {
            foreach ($catchStatement->getChildren() as $children) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                if ($children instanceof ASTVariable) {
                    $this->addVariableDefinition($children);
                }
            }
        }
    }

    /**
     * Stores the given literal node in an internal list of found variables.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @return void
     */
    private function collectListExpressions(AbstractCallableNode $node)
    {
        $lists = $node->findChildrenOfType('ListExpression');

        foreach ($lists as $listExpression) {
            foreach ($listExpression->getChildren() as $variable) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                $this->addVariableDefinition($variable);
            }
        }
    }

    /**
     * Stores the given literal node in an internal foreach of found variables.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @return void
     */
    private function collectForeachStatements(AbstractCallableNode $node)
    {
        $foreachStatements = $node->findChildrenOfType('ForeachStatement');

        foreach ($foreachStatements as $foreachStatement) {
            foreach ($foreachStatement->getChildren() as $children) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                if ($children instanceof ASTVariable) {
                    $this->addVariableDefinition($children);
                } elseif ($children instanceof ASTUnaryExpression) {
                    foreach ($children->getChildren() as $refChildren) {
                        if ($refChildren instanceof ASTVariable) {
                            $this->addVariableDefinition($refChildren);
                        }
                    }
                }
            }
        }
    }

    /**
     * Stores the given literal node in an internal closure of found variables.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @return void
     */
    private function collectClosureParameters(AbstractCallableNode $node)
    {
        $closures = $node->findChildrenOfType('Closure');

        foreach ($closures as $closure) {
            $this->collectParameters($closure);
        }
    }

    /**
     * Check if the given variable was defined in the current context before usage.
     *
     * @param \PHPMD\Node\ASTNode $variable
     * @param \PHPMD\Node\AbstractCallableNode $parentNode
     * @return bool
     */
    private function checkVariableDefined(ASTNode $variable, AbstractCallableNode $parentNode)
    {
        $image = $this->getVariableImage($variable);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

        return isset($this->images[$image]) || $this->isNameAllowedInContext($parentNode, $variable);
    }

    /**
     * Collect parameter names of method/function.
     *
     * @param \PHPMD\Node\AbstractNode $node
     * @return void
     */
    private function collectParameters(AbstractNode $node)
    {
        // Get formal parameter container
        $parameters = $node->getFirstChildOfType('FormalParameters');

        // Now get all declarators in the formal parameters container
        $declarators = $parameters->findChildrenOfType('VariableDeclarator');

        foreach ($declarators as $declarator) {
            $this->addVariableDefinition($declarator);
        }
    }

    /**
     * Collect assignments of variables.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @return void
     */
    private function collectAssignments(AbstractCallableNode $node)
    {
        foreach ($node->findChildrenOfType('AssignmentExpression') as $assignment) {
            $variable = $assignment->getChild(0);

            if ($variable->getNode() instanceof ASTArray) {
                foreach ($variable->findChildrenOfType('Variable') as $unpackedVariable) {
                    $this->addVariableDefinition($unpackedVariable);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                }

                continue;
            }

            $this->addVariableDefinition($variable);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        }
    }

    /**
     * Collect postfix property.
     *
     * @param \PHPMD\Node\AbstractNode $node
     * @return void
     */
    private function collectPropertyPostfix(AbstractNode $node)
    {
        $properties = $node->findChildrenOfType('PropertyPostfix');

        foreach ($properties as $property) {
            foreach ($property->getChildren() as $children) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                if ($children instanceof ASTVariable) {
                    $this->addVariableDefinition($children);
                }
            }
        }
    }

    /**
     * Add the variable to images.
     *
     * @param ASTVariable|ASTPropertyPostfix|ASTVariableDeclarator $variable
     * @return void
     */
    private function addVariableDefinition($variable)
    {
        $image = $this->getVariableImage($variable);

        if (!isset($this->images[$image])) {
            $this->images[$image] = $variable;
        }
    }

    /**
     * Return the PDepend node of ASTNode PHPMD node.
     *
     * Or return the input as is if it's not an ASTNode PHPMD node.
     *
     * @param mixed $node
     *
     * @return \PDepend\Source\AST\ASTArtifact|\PDepend\Source\AST\ASTNode
     */
    private function getNode($node)
    {
        if ($node instanceof ASTNode) {
            return $node->getNode();
        }

        return $node;
    }

    /**
     * Get the image of the given variable node.
     *
     * Prefix self:: and static:: properties with "::".
     *
     * @param ASTVariable|ASTPropertyPostfix|ASTVariableDeclarator $variable
     *
     * @return string
     */
    private function getVariableImage($variable)
    {
        $image = $variable->getImage();

        if ($image === '::') {
            return $image.$variable->getChild(1)->getImage();
        }

        $base = $variable;
        $parent = $this->getNode($variable->getParent());

        while ($parent && $parent instanceof ASTArrayIndexExpression && $parent->getChild(0) === $base->getNode()) {
            $base = $parent;
            $parent = $this->getNode($base->getParent());
        }

        if ($parent && $parent instanceof ASTPropertyPostfix) {
            $parent = $parent->getParent();

            if ($parent instanceof ASTMemberPrimaryPrefix &&
                in_array($parent->getChild(0)->getImage(), $this->selfReferences)
            ) {
                return "::$image";
            }
        }

        return $image;
    }

    /**
     * Checks if a short name is acceptable in the current context.
     *
     * @param \PHPMD\Node\AbstractCallableNode $node
     * @param \PHPMD\Node\ASTNode $variable
     *
     * @return boolean
     */
    private function isNameAllowedInContext(AbstractCallableNode $node, ASTNode $variable)
    {
        return (
            $node instanceof MethodNode &&
            $variable->getImage() === '$this' &&
            ($node->getModifiers() & State::IS_STATIC) === 0
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
        );
    }
}


phpmd / phpmd