Skip to content

Commit 1b3b365

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

File tree

1 file changed

+35
-37
lines changed

1 file changed

+35
-37
lines changed

src/Analyzer/ClassMethodAnalyzer.php

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -259,61 +259,59 @@ protected function reportChanged($report, $contextBefore, $contextAfter, $method
259259
$report->add($this->context, $data);
260260
$signatureChanged = true;
261261
} elseif ($signatureChanges['parameter_typing_changed']) {
262-
$paramsBefore = $methodBefore->params;
263-
$paramsAfter = $methodAfter->params;
264-
265-
$isSafeNullableChange = false;
266-
// Check if only difference is added explicit nullable `?` prefix
267-
foreach ($paramsBefore as $index => $paramBefore) {
268-
$paramAfter = $paramsAfter[$index] ?? null;
269-
if (!$paramAfter) {
270-
continue;
262+
// Compare each param to detect if it's only an implicit-nullable to explicit-nullable change
263+
$isSafeNullableChange = true;
264+
$paramCount = min(count($paramsBefore), count($paramsAfter));
265+
for ($i = 0; $i < $paramCount; $i++) {
266+
$beforeParam = $paramsBefore[$i];
267+
$afterParam = $paramsAfter[$i];
268+
269+
$beforeType = $beforeParam->type;
270+
$afterType = $afterParam->type;
271+
272+
$beforeDefaultIsNull = isset($beforeParam->default) && $beforeParam->default->value === null;
273+
print_r("Default value: $beforeParam->default->value\n");
274+
275+
// Case: type changed from no type but default null → ?Type
276+
if ($beforeType === null && $afterType instanceof \PhpParser\Node\NullableType && $beforeDefaultIsNull) {
277+
continue; // safe
271278
}
272-
273-
$beforeType = $paramBefore->type;
274-
$afterType = $paramAfter->type;
275-
echo "\nBefore type\n";
276-
print_r($beforeType);
277-
echo "\nAfter type\n";
278-
print_r($afterType);
279-
echo "\nBefore Instance of Name\n";
280-
print_r($beforeType instanceof \PhpParser\Node\Name);
281-
echo "\nAfter Instance of NullableType\n";
282-
print_r($afterType instanceof \PhpParser\Node\NullableType);
283-
echo "\nAfter Instance of Name\n";
284-
echo "Beforetype is ".$beforeType->toString(). " Aftertype is ".$afterType->toString()."\n";
285-
286-
287-
echo "\n----------------------\n";
288-
echo "\nBefore Instance of NullableType\n";
289-
print_r($beforeType instanceof \PhpParser\Node\NullableType);
290-
291-
echo "\n----------------------\n";
292-
if ($beforeType && $afterType &&
293-
$beforeType instanceof \PhpParser\Node\Name &&
279+
echo "\nafter type nullable\n";
280+
var_dump($afterType instanceof \PhpParser\Node\NullableType);
281+
echo "\nbefore type nullable\n";
282+
var_dump($beforeType instanceof \PhpParser\Node\NullableType);
283+
284+
// Case: type changed from Type to ?Type (explicitly nullable)
285+
if (
286+
$beforeType instanceof \PhpParser\Node\Identifier &&
294287
$afterType instanceof \PhpParser\Node\NullableType &&
295-
$afterType->type instanceof \PhpParser\Node\Name &&
296-
$beforeType->toString() === $afterType->type->toString()) {
297-
$isSafeNullableChange = true;
298-
} else {
299-
$isSafeNullableChange = false;
300-
break;
288+
$afterType->type instanceof \PhpParser\Node\Identifier &&
289+
$beforeType->name === $afterType->type->name &&
290+
$beforeDefaultIsNull
291+
) {
292+
continue; // safe
301293
}
294+
295+
$isSafeNullableChange = false;
296+
break;
302297
}
303298
if ($isSafeNullableChange) {
299+
// Treat as PATCH instead of MAJOR
304300
$data = new ClassMethodParameterTypingChangedNullable(
305301
$this->context,
306302
$this->fileAfter,
307303
$contextAfter,
308304
$methodAfter
309305
);
306+
// $report->add($this->context, $data);
310307
} else {
311308
$data = new ClassMethodParameterTypingChanged(
312309
$this->context,
313310
$this->fileAfter,
314311
$contextAfter,
315312
$methodAfter
316313
);
314+
317315
}
318316
$report->add($this->context, $data);
319317
$signatureChanged = true;

0 commit comments

Comments
 (0)