Skip to content

Commit 868178b

Browse files
committed
AC-14557:: False positives in the backward-incompatible changes report (SVC)
1 parent b01df4e commit 868178b

File tree

3 files changed

+84
-10
lines changed

3 files changed

+84
-10
lines changed

src/Analyzer/ClassMethodAnalyzer.php

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
use PHPSemVerChecker\Operation\ClassMethodParameterTypingRemoved;
4242
use PHPSemVerChecker\Operation\ClassMethodRemoved;
4343
use PHPSemVerChecker\Report\Report;
44+
use Magento\SemanticVersionChecker\Operation\ClassMethodParameterTypingChangedNullable;
4445

4546
/**
4647
* Class method analyzer.
@@ -258,12 +259,24 @@ protected function reportChanged($report, $contextBefore, $contextAfter, $method
258259
$report->add($this->context, $data);
259260
$signatureChanged = true;
260261
} elseif ($signatureChanges['parameter_typing_changed']) {
261-
$data = new ClassMethodParameterTypingChanged(
262-
$this->context,
263-
$this->fileAfter,
264-
$contextAfter,
265-
$methodAfter
266-
);
262+
263+
if ($signatureChanges['parameter_nullable_type_added'] ||
264+
$signatureChanges['parameter_nullable_type_removed']
265+
) {
266+
$data = new ClassMethodParameterTypingChangedNullable(
267+
$this->context,
268+
$this->fileAfter,
269+
$contextAfter,
270+
$methodAfter
271+
);
272+
} else {
273+
$data = new ClassMethodParameterTypingChanged(
274+
$this->context,
275+
$this->fileAfter,
276+
$contextAfter,
277+
$methodAfter
278+
);
279+
}
267280
$report->add($this->context, $data);
268281
$signatureChanged = true;
269282
}
@@ -455,8 +468,7 @@ private function isReturnsEqualByNullability(ClassMethod $before, ClassMethod $a
455468
*/
456469
private function getDocReturnDeclaration(ClassMethod $method)
457470
{
458-
if (
459-
($parsedComment = $method->getAttribute('docCommentParsed'))
471+
if (($parsedComment = $method->getAttribute('docCommentParsed'))
460472
&& isset($parsedComment['return'])
461473
) {
462474
if ($parsedComment['return'][0] instanceof NullableType) {

src/Comparator/Signature.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,26 +130,35 @@ public static function analyze(array $parametersA, array $parametersB): array
130130
$changes = array_merge($changes, [
131131
'parameter_typing_added' => false,
132132
'parameter_typing_removed' => false,
133-
'parameter_typing_changed' => false
133+
'parameter_typing_changed' => false,
134+
'parameter_nullable_type_added' => false,
135+
'parameter_nullable_type_removed' => false
134136
]);
135137
$lengthA = count($parametersA);
136138
$lengthB = count($parametersB);
137139

138140
$iterations = min($lengthA, $lengthB);
139141
for ($i = 0; $i < $iterations; ++$i) {
142+
$typeBefore = $parametersA[$i]->type;
143+
$typeAfter = $parametersB[$i]->type;
140144
// Re-implement type checking to handle type changes as a single operation instead of both add and remove
141145
if (Type::get($parametersA[$i]->type) !== Type::get($parametersB[$i]->type)) {
142146
// This section changed from parent::analyze() to handle typing changes
143147
if ($parametersA[$i]->type !== null && $parametersB[$i]->type !== null) {
144148
$changes['parameter_typing_changed'] = true;
149+
// Custom: detect nullable added
150+
if ($typeBefore instanceof \PhpParser\Node\NullableType && !$typeAfter instanceof \PhpParser\Node\NullableType) {
151+
$changes['parameter_nullable_type_removed'] = true;
152+
} elseif (!$typeBefore instanceof \PhpParser\Node\NullableType && $typeAfter instanceof \PhpParser\Node\NullableType) {
153+
$changes['parameter_nullable_type_added'] = true;
154+
}
145155
} elseif ($parametersA[$i]->type !== null) {
146156
$changes['parameter_typing_removed'] = true;
147157
} elseif ($parametersB[$i]->type !== null) {
148158
$changes['parameter_typing_added'] = true;
149159
}
150160
}
151161
}
152-
153162
return $changes;
154163
}
155164
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Magento\SemanticVersionChecker\Operation;
11+
12+
use PHPSemVerChecker\Operation\ClassMethodOperationUnary;
13+
use PHPSemVerChecker\SemanticVersioning\Level;
14+
15+
class ClassMethodParameterTypingChangedNullable extends ClassMethodOperationUnary
16+
{
17+
/**
18+
* @var array
19+
*/
20+
protected $code = [
21+
'class' => ['M113', 'M114', 'M115'],
22+
'interface' => ['M116'],
23+
'trait' => ['M117', 'M118', 'M119']
24+
];
25+
26+
/**
27+
* @var array
28+
*/
29+
private $mapping = [
30+
'M113' => Level::PATCH,
31+
'M114' => Level::MAJOR,
32+
'M115' => Level::PATCH,
33+
'M116' => Level::MAJOR,
34+
'M117' => Level::MAJOR,
35+
'M118' => Level::MAJOR,
36+
'M119' => Level::MAJOR
37+
];
38+
39+
/**
40+
* @var string
41+
*/
42+
protected $reason = 'Method parameter typing changed.';
43+
44+
/**
45+
* Returns level of error.
46+
*
47+
* @return mixed
48+
*/
49+
public function getLevel(): int
50+
{
51+
return $this->mapping[$this->getCode()];
52+
}
53+
}

0 commit comments

Comments
 (0)