Skip to content

Commit c727314

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

File tree

5 files changed

+98
-17
lines changed

5 files changed

+98
-17
lines changed

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"laminas/laminas-stdlib": "^3.18",
1414
"nikic/php-parser": "^4.19",
1515
"phpstan/phpdoc-parser": "^0.5.5",
16-
"sabre/xml": "~4.0.5",
16+
"sabre/xml": "~4.0.6",
1717
"symfony/console": "~6.4.17",
1818
"symfony/string": "~6.4.15",
1919
"tomzx/php-semver-checker": "^0.16.0",

composer.lock

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Analyzer/ClassMethodAnalyzer.php

Lines changed: 20 additions & 6 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,25 @@ 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 (
264+
$signatureChanges['parameter_nullable_type_added'] ||
265+
$signatureChanges['parameter_nullable_type_removed']
266+
) {
267+
$data = new ClassMethodParameterTypingChangedNullable(
268+
$this->context,
269+
$this->fileAfter,
270+
$contextAfter,
271+
$methodAfter
272+
);
273+
} else {
274+
$data = new ClassMethodParameterTypingChanged(
275+
$this->context,
276+
$this->fileAfter,
277+
$contextAfter,
278+
$methodAfter
279+
);
280+
}
267281
$report->add($this->context, $data);
268282
$signatureChanged = true;
269283
}

src/Comparator/Signature.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,26 +130,39 @@ 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,
136+
'changed_param_index' => 0
134137
]);
135138
$lengthA = count($parametersA);
136139
$lengthB = count($parametersB);
137140

138141
$iterations = min($lengthA, $lengthB);
139142
for ($i = 0; $i < $iterations; ++$i) {
143+
144+
$typeBefore = $parametersA[$i]->type;
145+
$typeAfter = $parametersB[$i]->type;
140146
// Re-implement type checking to handle type changes as a single operation instead of both add and remove
141147
if (Type::get($parametersA[$i]->type) !== Type::get($parametersB[$i]->type)) {
142148
// This section changed from parent::analyze() to handle typing changes
143149
if ($parametersA[$i]->type !== null && $parametersB[$i]->type !== null) {
144150
$changes['parameter_typing_changed'] = true;
151+
// Custom: detect nullable added
152+
if ($typeBefore instanceof \PhpParser\Node\NullableType && !$typeAfter instanceof \PhpParser\Node\NullableType) {
153+
$changes['parameter_nullable_type_removed'] = true;
154+
$changes['changed_param_index'] = $i;
155+
} elseif (!$typeBefore instanceof \PhpParser\Node\NullableType && $typeAfter instanceof \PhpParser\Node\NullableType) {
156+
$changes['parameter_nullable_type_added'] = true;
157+
$changes['changed_param_index'] = $i;
158+
}
145159
} elseif ($parametersA[$i]->type !== null) {
146160
$changes['parameter_typing_removed'] = true;
147161
} elseif ($parametersB[$i]->type !== null) {
148162
$changes['parameter_typing_added'] = true;
149163
}
150164
}
151165
}
152-
153166
return $changes;
154167
}
155168
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
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+
/**
41+
* @var string
42+
*/
43+
protected $reason = 'Method parameter typing changed.';
44+
45+
/**
46+
* Returns level of error.
47+
*
48+
* @return mixed
49+
*/
50+
public function getLevel(): int
51+
{
52+
return $this->mapping[$this->getCode()];
53+
}
54+
}

0 commit comments

Comments
 (0)