Skip to content

Commit 8564363

Browse files
zakdmazakdma
committed
MC-25111: SVC false-positive: method return typing changed
1 parent 739b0d6 commit 8564363

File tree

17 files changed

+369
-19
lines changed

17 files changed

+369
-19
lines changed

‎src/Analyzer/ClassAnalyzer.php‎

Lines changed: 1 addition & 0 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\DependencyGraph;
1213
use PhpParser\Node\Stmt\Class_;
1314
use PHPSemVerChecker\Operation\ClassAdded;
1415
use PHPSemVerChecker\Operation\ClassRemoved;

‎src/Analyzer/ClassMethodAnalyzer.php‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
use Magento\SemanticVersionChecker\Operation\Visibility\MethodDecreased as VisibilityMethodDecreased;
2525
use Magento\SemanticVersionChecker\Operation\Visibility\MethodIncreased as VisibilityMethodIncreased;
2626
use PhpParser\Node\NullableType;
27-
use PhpParser\Node\Stmt;
27+
use PhpParser\Node\Name;
28+
use PhpParser\Node\Stmt\Class_;
2829
use PhpParser\Node\Stmt\ClassLike;
2930
use PhpParser\Node\Stmt\ClassMethod;
3031
use PHPSemVerChecker\Comparator\Implementation;
@@ -38,12 +39,6 @@
3839
use PHPSemVerChecker\Operation\ClassMethodParameterTypingRemoved;
3940
use PHPSemVerChecker\Operation\ClassMethodRemoved;
4041
use PHPSemVerChecker\Report\Report;
41-
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
42-
use PHPStan\PhpDocParser\Lexer\Lexer;
43-
use PHPStan\PhpDocParser\Parser\ConstExprParser;
44-
use PHPStan\PhpDocParser\Parser\PhpDocParser;
45-
use PHPStan\PhpDocParser\Parser\TokenIterator;
46-
use PHPStan\PhpDocParser\Parser\TypeParser;
4742

4843
/**
4944
* Class method analyzer.
@@ -429,9 +424,35 @@ private function getDocReturnDeclaration(ClassMethod $method)
429424
$result = implode('|', $parsedComment['return']);
430425

431426
return $result;
432-
} else {
433-
return ' ';
427+
} elseif ($this->dependencyGraph !== null) {
428+
/** @var Class_ $methodClass */
429+
$methodClass = $method->getAttribute('parent');
430+
if ($methodClass) {
431+
$ancestors = [];
432+
if (!empty($methodClass->extends)) {
433+
$ancestors[] = $methodClass->extends;
434+
}
435+
if (!empty($methodClass->implements)) {
436+
$ancestors = array_merge($ancestors, $methodClass->implements);
437+
}
438+
/** @var Name $ancestor */
439+
foreach ($ancestors as $ancestor) {
440+
$ancestorClass = $this->dependencyGraph->findEntityByName($ancestor->toString());
441+
if ($ancestorClass) {
442+
foreach ($ancestorClass->getMethodList() as $methodItem) {
443+
if ($method->name->toString() == $methodItem->name->toString()) {
444+
$result = $this->getDocReturnDeclaration($methodItem);
445+
if (!empty(trim($result))) {
446+
return $result;
447+
}
448+
}
449+
}
450+
}
451+
}
452+
}
434453
}
454+
455+
return ' ';
435456
}
436457

437458
/**

‎src/Analyzer/Factory/NonApiAnalyzerFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class NonApiAnalyzerFactory implements AnalyzerFactoryInterface
2828
public function create(DependencyGraph $dependencyGraph = null): AnalyzerInterface
2929
{
3030
$analyzers = [
31-
new ClassAnalyzer(),
31+
new ClassAnalyzer(null, null, null, $dependencyGraph),
3232
new InterfaceAnalyzer(),
3333
new TraitAnalyzer(),
3434
];

‎src/Analyzer/MethodDocBlockAnalyzer.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535
* - method param typehint moved from in-line to doc block
3636
* - method return typehint moved from doc block to in-line
3737
* - method return typehint moved from in-line to doc block
38+
*
39+
* TODO: this class should be rewritten using new possibility added by
40+
* Magento\SemanticVersionChecker\Visitor\NameResolver
41+
* Now all information (and resolved typed) about DocBlock params and return type exists in
42+
* method node 'docCommentParsed' attribute
3843
*/
3944
class MethodDocBlockAnalyzer
4045
{

‎src/ClassHierarchy/StaticAnalyzerFactory.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
namespace Magento\SemanticVersionChecker\ClassHierarchy;
1111

1212
use Magento\SemanticVersionChecker\Helper\Node as NodeHelper;
13+
use Magento\SemanticVersionChecker\Visitor\ParentConnector;
1314
use PhpParser\NodeTraverser;
14-
use PhpParser\NodeVisitor\NameResolver;
15+
use Magento\SemanticVersionChecker\Visitor\NameResolver;
1516
use PhpParser\ParserFactory;
1617

1718
/**
@@ -31,6 +32,7 @@ public function create(): StaticAnalyzer
3132
);
3233
$nodeTraverser = new NodeTraverser();
3334

35+
$nodeTraverser->addVisitor(new ParentConnector());
3436
$nodeTraverser->addVisitor(new NameResolver());
3537

3638
return new StaticAnalyzer($parser, $dependencyInspectionVisitor, $nodeTraverser);

‎src/Scanner/ScannerRegistryFactory.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Magento\SemanticVersionChecker\Visitor\ApiClassVisitor;
1717
use Magento\SemanticVersionChecker\Visitor\ApiInterfaceVisitor;
1818
use Magento\SemanticVersionChecker\Visitor\ApiTraitVisitor;
19+
use Magento\SemanticVersionChecker\Visitor\ParentConnector;
1920
use PhpParser\Lexer\Emulative;
2021
use PhpParser\NodeTraverser;
2122
use Magento\SemanticVersionChecker\Visitor\NameResolver;
@@ -38,6 +39,7 @@ private function buildFullScanner()
3839
$traverser = new NodeTraverser();
3940
$apiVisitors = [
4041
new NameResolver(),
42+
new ParentConnector(),
4143
new ClassVisitor($registry),
4244
new InterfaceVisitor($registry),
4345
new FunctionVisitor($registry),
@@ -59,6 +61,7 @@ private function buildApiScanner(DependencyGraph $dependencyGraph = null)
5961
$nodeHelper = new NodeHelper();
6062
$apiVisitors = [
6163
new NameResolver(),
64+
new ParentConnector(),
6265
new ApiClassVisitor($registry, $nodeHelper, $dependencyGraph),
6366
new ApiInterfaceVisitor($registry, $nodeHelper, $dependencyGraph),
6467
new ApiTraitVisitor($registry, $nodeHelper, $dependencyGraph),

‎src/Visitor/NameResolver.php‎

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use PhpParser\Node;
1111
use PhpParser\Node\Name;
1212
use PhpParser\Node\Stmt\ClassMethod;
13+
use PhpParser\NodeAbstract;
1314
use PhpParser\NodeVisitor\NameResolver as ParserNameResolver;
1415
use PhpParser\BuilderHelpers;
1516
use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
@@ -24,10 +25,29 @@
2425
use PHPStan\PhpDocParser\Parser\TypeParser;
2526

2627
/**
27-
* Extended Name Resolver that parse and resolve also docblock hintings
28+
* Extended Name Resolver that parse and resolve also docblock params and return type hinting.
2829
*/
2930
class NameResolver extends ParserNameResolver
3031
{
32+
/**
33+
* Internal types that should not be resolved for docblock
34+
*
35+
* @var string[]
36+
*/
37+
private $internalTypes = [
38+
'string',
39+
'integer',
40+
'float',
41+
'double',
42+
'boolean',
43+
'bool',
44+
'array',
45+
'object',
46+
'null',
47+
'resource',
48+
'$this',
49+
];
50+
3151
/**
3252
* @inheritDoc
3353
*/
@@ -85,14 +105,10 @@ private function parseType($type)
85105
$result = [];
86106
if ($type instanceof UnionTypeNode) {
87107
foreach ($type->types as $typeNode) {
88-
$normalizedType = BuilderHelpers::normalizeType((string)$typeNode);
89-
$resolvedType = $this->resolveType($normalizedType);
90-
$result[] = $resolvedType;
108+
$result[] = $this->normalizeAndResolve($typeNode);
91109
}
92110
} else {
93-
$normalizedType = BuilderHelpers::normalizeType((string)$type);
94-
$resolvedType = $this->resolveType($normalizedType);
95-
$result[] = $resolvedType;
111+
$result[] = $this->normalizeAndResolve($type);
96112
}
97113

98114
uasort(
@@ -105,11 +121,28 @@ function ($elementOne, $elementTwo) {
105121
return $result;
106122
}
107123

124+
/**
125+
* @param TypeNode $type
126+
* @return NodeAbstract
127+
*/
128+
private function normalizeAndResolve($type)
129+
{
130+
$normalizedType = BuilderHelpers::normalizeType((string)$type);
131+
132+
if (in_array(strtolower((string)$type), $this->internalTypes)) {
133+
$resolvedType = $normalizedType;
134+
} else {
135+
$resolvedType = $this->resolveType($normalizedType);
136+
}
137+
138+
return $resolvedType;
139+
}
140+
108141
/**
109142
* Resolve type from Relative to FQCN
110143
*
111144
* @param $node
112-
* @return Name|Node\NullableType|Node\UnionType
145+
* @return NodeAbstract
113146
*/
114147
private function resolveType($node)
115148
{

‎src/Visitor/ParentConnector.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
8+
namespace Magento\SemanticVersionChecker\Visitor;
9+
10+
use PhpParser\NodeVisitorAbstract;
11+
use PhpParser\Node;
12+
13+
/**
14+
* Create parent reference for nodes. Parent reference can be found in 'parent' node attribute.
15+
*/
16+
class ParentConnector extends NodeVisitorAbstract
17+
{
18+
/**
19+
* Stack of nodes that used to create parent references
20+
*
21+
* @var array
22+
*/
23+
private $stack;
24+
25+
/**
26+
* @inheritDoc
27+
*/
28+
public function beginTraverse(array $nodes)
29+
{
30+
$this->stack = [];
31+
}
32+
33+
/**
34+
* @inheritDoc
35+
*/
36+
public function enterNode(Node $node)
37+
{
38+
if (!empty($this->stack)) {
39+
$node->setAttribute('parent', $this->stack[count($this->stack) - 1]);
40+
}
41+
$this->stack[] = $node;
42+
}
43+
44+
/**
45+
* @inheritDoc
46+
*/
47+
public function leaveNode(Node $node)
48+
{
49+
array_pop($this->stack);
50+
}
51+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ public function changesDataProvider()
207207
],
208208
'Patch change is detected.'
209209
],
210+
'docblock-return-type-not-changed' => [
211+
$pathToFixtures . '/docblock-return-type-not-changed/source-code-before',
212+
$pathToFixtures . '/docblock-return-type-not-changed/source-code-after',
213+
[
214+
'Suggested semantic versioning change: NONE'
215+
],
216+
'Patch change is detected.'
217+
],
210218
];
211219
}
212220
}

‎tests/Unit/Console/Command/CompareSourceCommandTest/_files/non-api-class/docblock-return-type-not-changed/source-code-after/InterfaceNs/TestInterface.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Test\Vcs\InterfaceNs;
7+
8+
interface TestInterface
9+
{
10+
/**
11+
* @return null|int|\Test\Vcs\Path\TestClass2
12+
*/
13+
public function testInterfaceMethod1();
14+
15+
/**
16+
* @return null|int|\Test\Vcs\Path\TestClass2
17+
*/
18+
public function testInterfaceMethod2();
19+
}

0 commit comments

Comments
 (0)