ClassMethodAnalyzer::wasMethodRemoved() - Code Metrics - Inspection of "Dont treat case changes in method names as breakin..." - tomzx/php-semver-checker - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#96)

unknown

created 2018-02-08 08:34 UTC

ClassMethodAnalyzer::wasMethodRemoved() A

↳ Parent: ClassMethodAnalyzer

Complexity

Conditions	3
Paths	3

Size

Total Lines	12
Code Lines	5

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	7
CRAP Score	3

Importance

Changes

Metric	Value
dl	0
loc	12
ccs	7
cts	7
cp	1
rs	9.4285
c	0
b	0
f	0
cc	3
eloc	5
nc	3
nop	2
crap	3

<?php

namespace PHPSemVerChecker\Analyzer;

use PhpParser\Node\Stmt;
use PHPSemVerChecker\Comparator\Implementation;
use PHPSemVerChecker\Comparator\Signature;
use PHPSemVerChecker\Operation\ClassMethodAdded;
use PHPSemVerChecker\Operation\ClassMethodImplementationChanged;
use PHPSemVerChecker\Operation\ClassMethodOperationUnary;
use PHPSemVerChecker\Operation\ClassMethodParameterAdded;
use PHPSemVerChecker\Operation\ClassMethodParameterDefaultAdded;
use PHPSemVerChecker\Operation\ClassMethodParameterDefaultRemoved;
use PHPSemVerChecker\Operation\ClassMethodParameterDefaultValueChanged;
use PHPSemVerChecker\Operation\ClassMethodParameterNameChanged;
use PHPSemVerChecker\Operation\ClassMethodParameterRemoved;
use PHPSemVerChecker\Operation\ClassMethodParameterTypingAdded;
use PHPSemVerChecker\Operation\ClassMethodParameterTypingRemoved;
use PHPSemVerChecker\Operation\ClassMethodRemoved;
use PHPSemVerChecker\Report\Report;

class ClassMethodAnalyzer
{
    protected $context;
    protected $fileBefore;
    protected $fileAfter;

    /**
     * @param string $context
     * @param string $fileBefore
     * @param string $fileAfter
     */
    public function __construct($context, $fileBefore = null, $fileAfter = null)
    {
        $this->context = $context;
        $this->fileBefore = $fileBefore;
        $this->fileAfter = $fileAfter;
    }

    public function analyze(Stmt $contextBefore, Stmt $contextAfter)
    {
        $report = new Report();

        $methodsBefore = $contextBefore->getMethods();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
        $methodsAfter = $contextAfter->getMethods();
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

        $methodsBeforeKeyed = [];
        foreach ($methodsBefore as $method) {
            $methodsBeforeKeyed[strtolower($method->name)] = $method;
        }

        $methodsAfterKeyed = [];
        foreach ($methodsAfter as $method) {
            $methodsAfterKeyed[strtolower($method->name)] = $method;
        }

        // Here we only care about public methods as they are the only part of the API we care about

        $methodNamesNotAddedOrRemoved = [];

        foreach($methodsBefore as $methodBefore)

        {
            // Removed methods can either be implemented in parent classes or not exist anymore
            if($this->wasMethodRemoved($methodBefore, $methodsAfter))
            {
                $data = new ClassMethodRemoved($this->context, $this->fileBefore, $contextBefore, $methodBefore);
                $report->add($this->context, $data);
            } else {
                $methodNamesNotAddedOrRemoved[strtolower($methodBefore->name)] = true;
            }
        }

        foreach($methodsAfter as $methodAfter)

        {
            // Added methods implies MINOR BUMP
            if($this->wasMethodAdded($methodAfter, $methodsBefore))
            {
                $data = new ClassMethodAdded($this->context, $this->fileAfter, $contextAfter, $methodAfter);
                $report->add($this->context, $data);
            } else {
                $methodNamesNotAddedOrRemoved[strtolower($methodAfter->name)] = true;
            }
        }

        foreach (array_keys($methodNamesNotAddedOrRemoved) as $methodName) {

            /** @var \PhpParser\Node\Stmt\ClassMethod $methodBefore */
            $methodBefore = $methodsBeforeKeyed[$methodName];
            /** @var \PhpParser\Node\Stmt\ClassMethod $methodAfter */
            $methodAfter = $methodsAfterKeyed[$methodName];

            if (!$this->areMethodsEqual($methodBefore, $methodAfter)) {

                $paramsBefore = $methodBefore->params;
                $paramsAfter = $methodAfter->params;

                $signatureResult = Signature::analyze($paramsBefore, $paramsAfter);

                $changes = [
                    'parameter_added' => ClassMethodParameterAdded::class,
                    'parameter_removed' => ClassMethodParameterRemoved::class,
                    'parameter_renamed' => ClassMethodParameterNameChanged::class,
                    'parameter_typing_added' => ClassMethodParameterTypingAdded::class,
                    'parameter_typing_removed' => ClassMethodParameterTypingRemoved::class,
                    'parameter_default_added' => ClassMethodParameterDefaultAdded::class,
                    'parameter_default_removed' => ClassMethodParameterDefaultRemoved::class,
                    'parameter_default_value_changed' => ClassMethodParameterDefaultValueChanged::class,
                ];

                foreach ($changes as $changeType => $class) {
                    if (!$signatureResult[$changeType]) {
                        continue;
                    }
                    if (is_a($class, ClassMethodOperationUnary::class, true)) {
                        $data = new $class($this->context, $this->fileAfter, $contextAfter, $methodAfter);
                    } else {
                        $data = new $class(
                            $this->context,
                            $this->fileBefore,
                            $contextBefore,
                            $methodBefore,
                            $this->fileAfter,
                            $contextAfter,
                            $methodAfter
                        );
                    }
                    $report->add($this->context, $data);
                }

                // Difference in source code
                // Cast to array because interfaces do not have stmts (= null)
                if (!Implementation::isSame((array)$methodBefore->stmts, (array)$methodAfter->stmts)) {
                    $data = new ClassMethodImplementationChanged(
                        $this->context,
                        $this->fileBefore,
                        $contextBefore,
                        $methodBefore,
                        $this->fileAfter,
                        $contextAfter,
                        $methodAfter
                    );
                    $report->add($this->context, $data);
                }
            }
        }

        return $report;
    }

    private function areMethodsEqual(Stmt\ClassMethod $methodBefore, Stmt\ClassMethod $methodAfter)
    {
        if ($methodBefore == $methodAfter) {
            return true;
        };

        return strtolower($methodBefore->name) === strtolower($methodAfter->name)
            && $methodBefore->isPrivate() === $methodAfter->isPrivate()
            && $methodBefore->isAbstract() === $methodAfter->isAbstract()
            && $methodBefore->isFinal() === $methodAfter->isFinal()
            && $methodBefore->isProtected() === $methodAfter->isProtected()
            && $methodBefore->isPublic() === $methodAfter->isPublic()
            && $methodBefore->isStatic() === $methodAfter->isStatic()
            && $methodBefore->getReturnType() === $methodAfter->getReturnType()
                // statements are objects, cannot be compared with ===
            && $methodBefore->getStmts() == $methodAfter->getStmts()
            && $methodBefore->getParams() === $methodAfter->getParams()
            && $methodBefore->returnsByRef() === $methodAfter->returnsByRef()
            && $methodBefore->getType() === $methodAfter->getType()
            && $methodBefore->getAttributes() === $methodAfter->getAttributes();
    }

    private function wasMethodAdded(Stmt\ClassMethod $method, $methodsAfter)
    {
        foreach($methodsAfter as $methodAfter)
        {
            if(strtolower($method->name) == strtolower($methodAfter->name))
            {
                return false;
            }
        }

        return true;
    }

    private function wasMethodRemoved(Stmt\ClassMethod $method, $methodsBefore)
    {
        foreach($methodsBefore as $methodBefore)
        {
            if(strtolower($method->name) == strtolower($methodBefore->name))
            {
                return false;
            }
        }

        return true;
    }
}


Pull Request — master (#96)

ClassMethodAnalyzer::wasMethodRemoved() A

Complexity

Size

Duplication

Code Coverage

Importance

Available Fixes

Available Fixes

1			<?php
2
3			namespace PHPSemVerChecker\Analyzer;
4
5			use PhpParser\Node\Stmt;
6			use PHPSemVerChecker\Comparator\Implementation;
7			use PHPSemVerChecker\Comparator\Signature;
8			use PHPSemVerChecker\Operation\ClassMethodAdded;
9			use PHPSemVerChecker\Operation\ClassMethodImplementationChanged;
10			use PHPSemVerChecker\Operation\ClassMethodOperationUnary;
11			use PHPSemVerChecker\Operation\ClassMethodParameterAdded;
12			use PHPSemVerChecker\Operation\ClassMethodParameterDefaultAdded;
13			use PHPSemVerChecker\Operation\ClassMethodParameterDefaultRemoved;
14			use PHPSemVerChecker\Operation\ClassMethodParameterDefaultValueChanged;
15			use PHPSemVerChecker\Operation\ClassMethodParameterNameChanged;
16			use PHPSemVerChecker\Operation\ClassMethodParameterRemoved;
17			use PHPSemVerChecker\Operation\ClassMethodParameterTypingAdded;
18			use PHPSemVerChecker\Operation\ClassMethodParameterTypingRemoved;
19			use PHPSemVerChecker\Operation\ClassMethodRemoved;
20			use PHPSemVerChecker\Report\Report;
21
22			class ClassMethodAnalyzer
23			{
24			protected $context;
25			protected $fileBefore;
26			protected $fileAfter;
27
28			/**
29			* @param string $context
30			* @param string $fileBefore
31			* @param string $fileAfter
32			*/
33	98		public function __construct($context, $fileBefore = null, $fileAfter = null)
34			{
35	98		$this->context = $context;
36	98		$this->fileBefore = $fileBefore;
37	98		$this->fileAfter = $fileAfter;
38	98		}
39
40	98		public function analyze(Stmt $contextBefore, Stmt $contextAfter)
41	1		{
42	98		$report = new Report();
43
44	98		$methodsBefore = $contextBefore->getMethods();
			0 ignored issues – show Bug introduced 2016-01-15 23:43 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `PhpParser\Node\Stmt` as the method `getMethods()` does only exist in the following sub-classes of `PhpParser\Node\Stmt`: `PhpParser\Node\Stmt\ClassLike`, `PhpParser\Node\Stmt\Class_`, `PhpParser\Node\Stmt\Interface_`, `PhpParser\Node\Stmt\Trait_`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
45	98		$methodsAfter = $contextAfter->getMethods();
			0 ignored issues – show Bug introduced 2016-01-15 23:43 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `PhpParser\Node\Stmt` as the method `getMethods()` does only exist in the following sub-classes of `PhpParser\Node\Stmt`: `PhpParser\Node\Stmt\ClassLike`, `PhpParser\Node\Stmt\Class_`, `PhpParser\Node\Stmt\Interface_`, `PhpParser\Node\Stmt\Trait_`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
46
47	98		$methodsBeforeKeyed = [];
48	98		foreach ($methodsBefore as $method) {
49	91		$methodsBeforeKeyed[strtolower($method->name)] = $method;
50	98		}
51
52	98		$methodsAfterKeyed = [];
53	98		foreach ($methodsAfter as $method) {
54	91		$methodsAfterKeyed[strtolower($method->name)] = $method;
55	98		}
56
57			// Here we only care about public methods as they are the only part of the API we care about
58
59	98		$methodNamesNotAddedOrRemoved = [];
60
61	98	View Code Duplication	foreach($methodsBefore as $methodBefore)
			0 ignored issues – show Duplication introduced 2018-02-08 08:36 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
62			{
63			// Removed methods can either be implemented in parent classes or not exist anymore
64	91		if($this->wasMethodRemoved($methodBefore, $methodsAfter))
65	91		{
66	7		$data = new ClassMethodRemoved($this->context, $this->fileBefore, $contextBefore, $methodBefore);
67	7		$report->add($this->context, $data);
68	7		} else {
69	84		$methodNamesNotAddedOrRemoved[strtolower($methodBefore->name)] = true;
70			}
71	98		}
72
73	98	View Code Duplication	foreach($methodsAfter as $methodAfter)
			0 ignored issues – show Duplication introduced 2018-02-08 08:36 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
74			{
75			// Added methods implies MINOR BUMP
76	91		if($this->wasMethodAdded($methodAfter, $methodsBefore))
77	91		{
78	7		$data = new ClassMethodAdded($this->context, $this->fileAfter, $contextAfter, $methodAfter);
79	7		$report->add($this->context, $data);
80	7		} else {
81	84		$methodNamesNotAddedOrRemoved[strtolower($methodAfter->name)] = true;
82			}
83	98		}
84
85	98		foreach (array_keys($methodNamesNotAddedOrRemoved) as $methodName) {
86
87			/** @var \PhpParser\Node\Stmt\ClassMethod $methodBefore */
88	84		$methodBefore = $methodsBeforeKeyed[$methodName];
89			/** @var \PhpParser\Node\Stmt\ClassMethod $methodAfter */
90	84		$methodAfter = $methodsAfterKeyed[$methodName];
91
92	84		if (!$this->areMethodsEqual($methodBefore, $methodAfter)) {
93
94	62		$paramsBefore = $methodBefore->params;
95	62		$paramsAfter = $methodAfter->params;
96
97	62		$signatureResult = Signature::analyze($paramsBefore, $paramsAfter);
98
99			$changes = [
100	62		'parameter_added' => ClassMethodParameterAdded::class,
101	62		'parameter_removed' => ClassMethodParameterRemoved::class,
102	62		'parameter_renamed' => ClassMethodParameterNameChanged::class,
103	62		'parameter_typing_added' => ClassMethodParameterTypingAdded::class,
104	62		'parameter_typing_removed' => ClassMethodParameterTypingRemoved::class,
105	62		'parameter_default_added' => ClassMethodParameterDefaultAdded::class,
106	62		'parameter_default_removed' => ClassMethodParameterDefaultRemoved::class,
107	62		'parameter_default_value_changed' => ClassMethodParameterDefaultValueChanged::class,
108	62		];
109
110	62		foreach ($changes as $changeType => $class) {
111	62		if (!$signatureResult[$changeType]) {
112	62		continue;
113			}
114	56		if (is_a($class, ClassMethodOperationUnary::class, true)) {
115	42		$data = new $class($this->context, $this->fileAfter, $contextAfter, $methodAfter);
116	42		} else {
117	14		$data = new $class(
118	14		$this->context,
119	14		$this->fileBefore,
120	14		$contextBefore,
121	14		$methodBefore,
122	14		$this->fileAfter,
123	14		$contextAfter,
124			$methodAfter
125	14		);
126			}
127	56		$report->add($this->context, $data);
128	62		}
129
130			// Difference in source code
131			// Cast to array because interfaces do not have stmts (= null)
132	62		if (!Implementation::isSame((array)$methodBefore->stmts, (array)$methodAfter->stmts)) {
133	6		$data = new ClassMethodImplementationChanged(
134	6		$this->context,
135	6		$this->fileBefore,
136	6		$contextBefore,
137	6		$methodBefore,
138	6		$this->fileAfter,
139	6		$contextAfter,
140			$methodAfter
141	6		);
142	6		$report->add($this->context, $data);
143	6		}
144	62		}
145	98		}
146
147	98		return $report;
148			}
149
150	84		private function areMethodsEqual(Stmt\ClassMethod $methodBefore, Stmt\ClassMethod $methodAfter)
151			{
152	84		if ($methodBefore == $methodAfter) {
153	21		return true;
154			};
155
156	63		return strtolower($methodBefore->name) === strtolower($methodAfter->name)
157	63		&& $methodBefore->isPrivate() === $methodAfter->isPrivate()
158	63		&& $methodBefore->isAbstract() === $methodAfter->isAbstract()
159	63		&& $methodBefore->isFinal() === $methodAfter->isFinal()
160	63		&& $methodBefore->isProtected() === $methodAfter->isProtected()
161	63		&& $methodBefore->isPublic() === $methodAfter->isPublic()
162	63		&& $methodBefore->isStatic() === $methodAfter->isStatic()
163	63		&& $methodBefore->getReturnType() === $methodAfter->getReturnType()
164			// statements are objects, cannot be compared with ===
165	63		&& $methodBefore->getStmts() == $methodAfter->getStmts()
166	63		&& $methodBefore->getParams() === $methodAfter->getParams()
167	63		&& $methodBefore->returnsByRef() === $methodAfter->returnsByRef()
168	63		&& $methodBefore->getType() === $methodAfter->getType()
169	63		&& $methodBefore->getAttributes() === $methodAfter->getAttributes();
170			}
171
172	91		private function wasMethodAdded(Stmt\ClassMethod $method, $methodsAfter)
173			{
174	91		foreach($methodsAfter as $methodAfter)
175			{
176	84		if(strtolower($method->name) == strtolower($methodAfter->name))
177	84		{
178	84		return false;
179			}
180	7		}
181
182	7		return true;
183			}
184
185	91		private function wasMethodRemoved(Stmt\ClassMethod $method, $methodsBefore)
186			{
187	91		foreach($methodsBefore as $methodBefore)
188			{
189	84		if(strtolower($method->name) == strtolower($methodBefore->name))
190	84		{
191	84		return false;
192			}
193	7		}
194
195	7		return true;
196			}
197			}
198

tomzx / php-semver-checker

Pull Request — master (#96)

ClassMethodAnalyzer::wasMethodRemoved() A

Complexity

Size

Duplication

Code Coverage

Importance

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like