Skip to content

Commit e64773c

Browse files
zakdmazakdma
authored
Merge pull request #23 from magento-tsg/MC-35405
[Arrows] MC-35405: SVC false-positive: overriding public method in child class
2 parents c4eb675 + a0a1c0f commit e64773c

File tree

16 files changed

+1368
-613
lines changed

16 files changed

+1368
-613
lines changed

‎.github/workflows/php.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
- uses: actions/checkout@v2
1616

1717
- name: Update PHP
18-
run: sudo update-alternatives --set php /usr/bin/php7.2
18+
run: sudo update-alternatives --set php /usr/bin/php7.4
1919

2020
- name: Validate composer
2121
run: composer validate

‎src/Analyzer/ClassMethodAnalyzer.php

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace Magento\SemanticVersionChecker\Analyzer;
1111

12+
use Magento\SemanticVersionChecker\ClassHierarchy\Entity;
1213
use Magento\SemanticVersionChecker\Comparator\Signature;
1314
use Magento\SemanticVersionChecker\Comparator\Visibility;
1415
use Magento\SemanticVersionChecker\Operation\ClassConstructorLastParameterRemoved;
@@ -117,22 +118,46 @@ protected function reportAddedNode($report, $fileAfter, $classAfter, $methodAfte
117118
$class = $this->dependencyGraph->findEntityByName((string) $classAfter->namespacedName);
118119

119120
if ($class !== null) {
120-
foreach ($class->getExtends() as $entity) {
121-
$methods = $entity->getMethodList();
122-
// checks if the method is already exiting in parent class
123-
if (isset($methods[$methodAfter->name->toString()])) {
124-
$report->add(
125-
$this->context,
126-
new ClassMethodOverwriteAdded($this->context, $fileAfter, $classAfter, $methodAfter)
127-
);
128-
return;
129-
}
121+
$methodOverwritten = $this->searchMethodExistsRecursive($class, $methodAfter->name->toString());
122+
if ($methodOverwritten) {
123+
$report->add(
124+
$this->context,
125+
new ClassMethodOverwriteAdded($this->context, $fileAfter, $classAfter, $methodAfter)
126+
);
127+
128+
return;
130129
}
131130
}
132131

133132
$report->add($this->context, new ClassMethodAdded($this->context, $fileAfter, $classAfter, $methodAfter));
134133
}
135134

135+
/**
136+
* Check if there is such method in class inheritance chain.
137+
*
138+
* @param Entity $class
139+
* @param string $methodName
140+
* @return boolean
141+
*/
142+
private function searchMethodExistsRecursive($class, $methodName)
143+
{
144+
/** @var Entity $entity */
145+
foreach ($class->getExtends() as $entity) {
146+
$methods = $entity->getMethodList();
147+
// checks if the method is already exiting in parent class
148+
if (isset($methods[$methodName])) {
149+
return true;
150+
}
151+
152+
$result = $this->searchMethodExistsRecursive($entity, $methodName);
153+
if ($result) {
154+
return true;
155+
}
156+
}
157+
158+
return false;
159+
}
160+
136161
/**
137162
* Create and report a ClassMethodRemoved operation
138163
*

‎tests/Unit/Console/Command/CompareSourceCommandApiClassesTest.php

Lines changed: 463 additions & 436 deletions
Large diffs are not rendered by default.

‎tests/Unit/Console/Command/CompareSourceCommandNonApiClassesTest.php

Lines changed: 193 additions & 166 deletions
Large diffs are not rendered by default.

‎tests/Unit/Console/Command/CompareSourceCommandTest/_files/api-class/added-method-subclass-overwrite-transition/source-code-after/ApiClass.php

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
/**
3+
*
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
namespace Test\Vcs;
8+
9+
/**
10+
* @api
11+
*/
12+
class ApiClass extends BaseNonApiClass
13+
{
14+
public function testFunction(): string
15+
{
16+
return 'ApiClass';
17+
}
18+
19+
protected function testFunctionProtected(): string
20+
{
21+
return 'ApiClass';
22+
}
23+
24+
public function __construct()
25+
{
26+
parent::__construct();
27+
}
28+
29+
public function __destruct()
30+
{
31+
32+
}
33+
34+
public function __call(string $name, array $arguments)
35+
{
36+
37+
}
38+
39+
public static function __callStatic(string $name, array $arguments)
40+
{
41+
42+
}
43+
44+
public function __get(string $name)
45+
{
46+
return null;
47+
}
48+
49+
public function __set(string $name, $value) : void
50+
{
51+
52+
}
53+
54+
public function __isset(string $name ) : bool
55+
{
56+
return false;
57+
}
58+
59+
public function __unset(string $name ) : void
60+
{
61+
62+
}
63+
64+
public function __sleep() : array
65+
{
66+
return [];
67+
}
68+
69+
public function __wakeup() : void
70+
{
71+
72+
}
73+
74+
public function __serialize() : array
75+
{
76+
return [];
77+
}
78+
79+
public function __unserialize(array $data ) : void
80+
{
81+
82+
}
83+
84+
public function __toString() : string
85+
{
86+
return '';
87+
}
88+
89+
public function __invoke( ...$values)
90+
{
91+
92+
}
93+
94+
public function __set_state(array $properties ) : object
95+
{
96+
return new \stdClass();
97+
}
98+
99+
public function __clone()
100+
{
101+
return;
102+
}
103+
104+
public function __debugInfo() : array
105+
{
106+
return [];
107+
}
108+
}

‎tests/Unit/Console/Command/CompareSourceCommandTest/_files/api-class/added-method-subclass-overwrite-transition/source-code-after/BaseApiClass.php

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
<?php
2+
3+
namespace Test\Vcs;
4+
5+
/**
6+
* @api
7+
*/
8+
class BaseApiClass
9+
{
10+
public function testFunction(): string
11+
{
12+
return 'baseApiClass';
13+
}
14+
15+
protected function testFunctionProtected(): string
16+
{
17+
return 'baseApiClass';
18+
}
19+
20+
public function __construct()
21+
{
22+
23+
}
24+
25+
public function __destruct()
26+
{
27+
28+
}
29+
30+
public function __call(string $name, array $arguments)
31+
{
32+
33+
}
34+
35+
public static function __callStatic(string $name, array $arguments)
36+
{
37+
38+
}
39+
40+
public function __get(string $name)
41+
{
42+
return null;
43+
}
44+
45+
public function __set(string $name, $value) : void
46+
{
47+
48+
}
49+
50+
public function __isset(string $name ) : bool
51+
{
52+
return false;
53+
}
54+
55+
public function __unset(string $name ) : void
56+
{
57+
58+
}
59+
60+
public function __sleep() : array
61+
{
62+
return [];
63+
}
64+
65+
public function __wakeup() : void
66+
{
67+
68+
}
69+
70+
public function __serialize() : array
71+
{
72+
return [];
73+
}
74+
75+
public function __unserialize(array $data ) : void
76+
{
77+
78+
}
79+
80+
public function __toString() : string
81+
{
82+
return '';
83+
}
84+
85+
public function __invoke( ...$values)
86+
{
87+
88+
}
89+
90+
public function __set_state(array $properties ) : object
91+
{
92+
return new \stdClass();
93+
}
94+
95+
public function __clone()
96+
{
97+
return;
98+
}
99+
100+
public function __debugInfo() : array
101+
{
102+
return [];
103+
}
104+
}

‎tests/Unit/Console/Command/CompareSourceCommandTest/_files/api-class/added-method-subclass-overwrite-transition/source-code-after/BaseNonApiClass.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Test\Vcs;
4+
5+
class BaseNonApiClass extends BaseApiClass
6+
{
7+
}

‎tests/Unit/Console/Command/CompareSourceCommandTest/_files/api-class/added-method-subclass-overwrite-transition/source-code-before/ApiClass.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
/**
3+
*
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
namespace Test\Vcs;
8+
9+
/**
10+
* @api
11+
*/
12+
class ApiClass extends BaseNonApiClass
13+
{
14+
}

0 commit comments

Comments
 (0)