Skip to content

Commit 3a5091e

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

File tree

1 file changed

+10
-72
lines changed

1 file changed

+10
-72
lines changed

src/Analyzer/ClassMethodAnalyzer.php

Lines changed: 10 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Magento\SemanticVersionChecker\Operation\ClassMethodOptionalParameterAdded;
2121
use Magento\SemanticVersionChecker\Operation\ClassMethodOverwriteAdded;
2222
use Magento\SemanticVersionChecker\Operation\ClassMethodParameterTypingChanged;
23-
use Magento\SemanticVersionChecker\Operation\ClassMethodParameterTypingChangedNullable;
2423
use Magento\SemanticVersionChecker\Operation\ClassMethodReturnTypingChanged;
2524
use Magento\SemanticVersionChecker\Operation\ExtendableClassConstructorOptionalParameterAdded;
2625
use Magento\SemanticVersionChecker\Operation\Visibility\MethodDecreased as VisibilityMethodDecreased;
@@ -259,77 +258,29 @@ protected function reportChanged($report, $contextBefore, $contextAfter, $method
259258
$report->add($this->context, $data);
260259
$signatureChanged = true;
261260
} elseif ($signatureChanges['parameter_typing_changed']) {
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-
// $beforeParam->
270-
271-
$beforeType = $beforeParam->type;
272-
$afterType = $afterParam->type;
273-
// $beforeParam->default->
274-
275-
$beforeNullable = $this->isNullable($beforeType);
276-
$afterNullable = $this->isNullable($afterType);
277-
278-
$beforeTypeName = $this->getTypeName($beforeType);
279-
$afterTypeName = $this->getTypeName($afterType);
280-
281-
if ($beforeNullable !== $afterNullable && $beforeTypeName === $afterTypeName) {
282-
echo "️Nullable type change detected for parameter \${$beforeParam->var->name}:\n";
283-
echo "Before: " . ($beforeNullable ? '?' : '') . $beforeTypeName . "\n";
284-
echo "After: " . ($afterNullable ? '?' : '') . $afterTypeName . "\n";
285-
}
286-
287-
288-
$beforeDefaultIsNull = isset($beforeParam->default) && $beforeParam->default->value === null;
289-
print_r("Default value: $beforeParam->default->value\n");
290-
291-
// Case: type changed from no type but default null → ?Type
292-
if ($beforeType === null && $afterType instanceof \PhpParser\Node\NullableType && $beforeDefaultIsNull) {
293-
continue; // safe
294-
}
295-
echo "\nafter type nullable\n";
296-
var_dump($afterType instanceof \PhpParser\Node\NullableType);
297-
echo "\nbefore type nullable\n";
298-
var_dump($beforeType instanceof \PhpParser\Node\NullableType);
299-
300-
// Case: type changed from Type to ?Type (explicitly nullable)
301-
if (
302-
$beforeType instanceof \PhpParser\Node\Identifier &&
303-
$afterType instanceof \PhpParser\Node\NullableType &&
304-
$afterType->type instanceof \PhpParser\Node\Identifier &&
305-
$beforeType->name === $afterType->type->name &&
306-
$beforeDefaultIsNull
307-
) {
308-
continue; // safe
309-
}
310-
311-
$isSafeNullableChange = false;
312-
break;
313-
}
314-
if ($isSafeNullableChange) {
315-
// Treat as PATCH instead of MAJOR
316-
$data = new ClassMethodParameterTypingChangedNullable(
261+
$paramBefore = $paramsBefore[$signatureChanges['changed_param_index']];
262+
$paramAfter = $paramsAfter[$signatureChanges['changed_param_index']];
263+
264+
if (
265+
$paramAfter->type instanceof NullableType &&
266+
!($paramBefore->type instanceof NullableType)
267+
) {
268+
$data = new \Magento\SemanticVersionChecker\Operation\ClassMethodParameterTypingChangedNullable(
317269
$this->context,
318270
$this->fileAfter,
319271
$contextAfter,
320272
$methodAfter
321273
);
322-
// $report->add($this->context, $data);
274+
$report->add($this->context, $data);
323275
} else {
324276
$data = new ClassMethodParameterTypingChanged(
325277
$this->context,
326278
$this->fileAfter,
327279
$contextAfter,
328280
$methodAfter
329281
);
330-
282+
$report->add($this->context, $data);
331283
}
332-
$report->add($this->context, $data);
333284
$signatureChanged = true;
334285
}
335286

@@ -437,19 +388,6 @@ protected function reportChanged($report, $contextBefore, $contextAfter, $method
437388
}
438389
}
439390

440-
private function isNullable($type): bool {
441-
return $type instanceof \PhpParser\Node\NullableType;
442-
}
443-
444-
private function getTypeName($type): ?string {
445-
if ($type instanceof \PhpParser\Node\NullableType) {
446-
return $type->type instanceof \PhpParser\Node\Identifier ? $type->type->name : null;
447-
} elseif ($type instanceof \PhpParser\Node\Identifier) {
448-
return $type->name;
449-
}
450-
return null; // For union types or no type
451-
}
452-
453391
/**
454392
* Checks if return type declaration or annotation was changed
455393
*

0 commit comments

Comments
 (0)