Incorrect behavior of RemoveParentDelegatingConstructorRector

[franck-paul]

Bug Report

Subject	Details
Rector version	last dev-main
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.com/demo/46ca86bb-337a-4bad-81af-40d1ab7929c8

<?php
class InternalServerException
{
    private Throwable $stack = [];
    public function __construct(string $message = 'Server error', int $code = 500, ?Throwable $previous = null)
    {
        echo 'Error thrown : ' . $message . ' (' . $code . ')';
	if (!is_null($previous)) {
            $this->stack[] = $previous;
        }
    }
}

class BlogException extends InternalServerException
{
    public function __construct(string $message = 'Blog handling error', int $code = 570, ?Throwable $previous = null)
    {
        parent::__construct($message, $code, $previous);
    }
}

Responsible rules

• RemoveParentDelegatingConstructorRector

Expected Behavior

As the child constructor provide default parameters' values (which may be different than parent's ones) if necessary, I think that the child constructor is still relevant.

[samsonasik]

@franck-paul could you try provide a patch? Here the steps:

• Add fixture at https://github.com/rectorphp/rector-src/tree/main/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture
• Just using some native php extends seems fine, eg:

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;

class SkipWithDefaultParams extends \DateTime
{
    public function __construct(string $datetime = "tomorrow", ?\DateTimeZone $timezone = null)
    {
        parent::__construct($datetime, $timezone);
    }
}

changed from now to tomorrow.

• Apply the patch, eg at

https://github.com/rectorphp/rector-src/blob/d1ff945497efbdfca534b33443d4ac323dd3d9e3/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php#L189-L191

skip append $paramNames[] if the param->default instanceof \PhpParser\Node\Expr is exists.

[samsonasik]

nvm, I created PR:

• [DeadCode] Skip with different default param value child vs parent on RemoveParentDelegatingConstructorRector rector-src#7776

for it.