ElseExpression::apply() - Code Metrics - Inspection of "test" - SomeBlackMagic/phpmd - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Pull Request — master (#18)

by Andru

created 2019-01-18 11:52 UTC

ElseExpression::apply() A

↳ Parent: ElseExpression

Complexity

Conditions	5
Paths	5

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	10
CRAP Score	5.0187

Importance

Changes

Metric	Value
cc	5
nc	5
nop	1
dl	0
loc	20
ccs	10
cts	11
cp	0.9091
crap	5.0187
rs	9.2888
c	0
b	0
f	0

<?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 PHPMD\AbstractNode;
use PHPMD\AbstractRule;
use PHPMD\Node\ASTNode;
use PHPMD\Rule\FunctionAware;
use PHPMD\Rule\MethodAware;

/**
 * Check if there is an else expression somewhere in the method/function and
 * warn about it.
 *
 * Object Calisthenics teaches us, that an else expression can always be
 * avoided by simple guard clause or return statements.
 */
class ElseExpression extends AbstractRule implements MethodAware, FunctionAware
{
    /**
     * This method checks if a method/function uses an else expression and add a violation for each one found.
     *
     * @param \PHPMD\AbstractNode $node
     * @return void
     */
    public function apply(AbstractNode $node)
    {
        foreach ($node->findChildrenOfType('ScopeStatement') as $scope) {
            $parent = $scope->getParent();

            if (false === $this->isIfOrElseIfStatement($parent)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
                continue;
            }

            if (false === $this->isElseScope($scope, $parent)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
                continue;
            }

            if (true === $this->hasYieldOrYieldFrom($scope)) {
                continue;
            }

            $this->addViolation($scope, array($node->getImage()));
        }
    }

    /**
     * Whether the given scope is an else clause
     *
     * @param $scope
     * @param ASTNode $parent
     * @return bool
     */
    private function isElseScope($scope, ASTNode $parent)
    {
        return (
            count($parent->getChildren()) === 3 &&
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 { }
            $scope->getNode() === $parent->getChild(2)->getNode()
        );
    }

    /**
     * Whether the parent node is an if or an elseif clause
     *
     * @param ASTNode $parent
     * @return bool
     */
    private function isIfOrElseIfStatement(ASTNode $parent)
    {
        return ($parent->getName() === "if" || $parent->getName() === "elseif");
    }

    /**
     * Whether the given scope has a yield or yield from clause
     *
     * @param $scope
     * @return bool
     */
    private function hasYieldOrYieldFrom($scope)
    {
        return (bool) $scope->getFirstChildOfType('YieldStatement');
    }
}


1		<?php
2		/**
3		* This file is part of PHP Mess Detector.
4		*
5		* Copyright (c) Manuel Pichler <[email protected]>.
6		* All rights reserved.
7		*
8		* Licensed under BSD License
9		* For full copyright and license information, please see the LICENSE file.
10		* Redistributions of files must retain the above copyright notice.
11		*
12		* @author Manuel Pichler <[email protected]>
13		* @copyright Manuel Pichler. All rights reserved.
14		* @license https://opensource.org/licenses/bsd-license.php BSD License
15		* @link http://phpmd.org/
16		*/
17
18		namespace PHPMD\Rule\CleanCode;
19
20		use PHPMD\AbstractNode;
21		use PHPMD\AbstractRule;
22		use PHPMD\Node\ASTNode;
23		use PHPMD\Rule\FunctionAware;
24		use PHPMD\Rule\MethodAware;
25
26		/**
27		* Check if there is an else expression somewhere in the method/function and
28		* warn about it.
29		*
30		* Object Calisthenics teaches us, that an else expression can always be
31		* avoided by simple guard clause or return statements.
32		*/
33		class ElseExpression extends AbstractRule implements MethodAware, FunctionAware
34		{
35		/**
36		* This method checks if a method/function uses an else expression and add a violation for each one found.
37		*
38		* @param \PHPMD\AbstractNode $node
39		* @return void
40		*/
41	4	public function apply(AbstractNode $node)
42		{
43	4	foreach ($node->findChildrenOfType('ScopeStatement') as $scope) {
44	4	$parent = $scope->getParent();
45
46	4	if (false === $this->isIfOrElseIfStatement($parent)) {
		0 ignored issues – show Bug introduced 2019-01-15 20:34 UTC by Report Bug Copy Issue Report It seems like `$parent` defined by `$scope->getParent()` on line `44` can be `null`; however, `PHPMD\Rule\CleanCode\Els...isIfOrElseIfStatement()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
47		continue;
48		}
49
50	4	if (false === $this->isElseScope($scope, $parent)) {
		0 ignored issues – show Bug introduced 2019-01-15 20:34 UTC by Report Bug Copy Issue Report It seems like `$parent` defined by `$scope->getParent()` on line `44` can be `null`; however, `PHPMD\Rule\CleanCode\ElseExpression::isElseScope()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
51	4	continue;
52		}
53
54	3	if (true === $this->hasYieldOrYieldFrom($scope)) {
55	1	continue;
56		}
57
58	2	$this->addViolation($scope, array($node->getImage()));
59		}
60	4	}
61
62		/**
63		* Whether the given scope is an else clause
64		*
65		* @param $scope
66		* @param ASTNode $parent
67		* @return bool
68		*/
69	4	private function isElseScope($scope, ASTNode $parent)
70		{
71		return (
72	4	count($parent->getChildren()) === 3 &&
		0 ignored issues – show Documentation Bug introduced 2019-01-15 20:34 UTC by Report Bug Copy Issue Report The method `getChildren` does not exist on `object<PHPMD\Node\ASTNode>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: 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 { } Loading history...
73	4	$scope->getNode() === $parent->getChild(2)->getNode()
74		);
75		}
76
77		/**
78		* Whether the parent node is an if or an elseif clause
79		*
80		* @param ASTNode $parent
81		* @return bool
82		*/
83	4	private function isIfOrElseIfStatement(ASTNode $parent)
84		{
85	4	return ($parent->getName() === "if" \|\| $parent->getName() === "elseif");
86		}
87
88		/**
89		* Whether the given scope has a yield or yield from clause
90		*
91		* @param $scope
92		* @return bool
93		*/
94	3	private function hasYieldOrYieldFrom($scope)
95		{
96	3	return (bool) $scope->getFirstChildOfType('YieldStatement');
97		}
98		}
99

SomeBlackMagic / phpmd

Pull Request — master (#18)

ElseExpression::apply() A

Complexity

Size

Duplication

Code Coverage

Importance

Duplication Side-by-Side

Filter issues like