Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect behavior of TernaryFalseExpressionToIfRector #8367

Closed
gehrisandro opened this issue Dec 18, 2023 · 0 comments · Fixed by rectorphp/rector-src#5373
Closed

Incorrect behavior of TernaryFalseExpressionToIfRector #8367

gehrisandro opened this issue Dec 18, 2023 · 0 comments · Fixed by rectorphp/rector-src#5373
Labels

Comments

@gehrisandro
Copy link

Bug Report

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

Minimal PHP Code Causing Issue

See https://getrector.com/demo/66f82535-275a-4804-bdbe-cc6b10f9cabc

<?php

final class DemoFile
{
    public function run(bool $param): void
    {
        $param ? $this->a() : $this->b();
    }

    public function a(): void
    {

    }

    public function b(): void
    {

    }
}

Responsible rules

  • TernaryFalseExpressionToIfRector

Expected Behavior

Rector should not modify the code. This issue is not related to a specific rector version. Instead it depends on the phpstan version used.
I was able to trace this down to a change introduces in phpstan version 1.10.49. And specifically this PR: phpstan/phpstan-src#2778

The problem is basically that rector has changed the return type from a VoidType to NullType which is an instance of ConstantType which then leads to this unexpected refactor.

But I think this is a problem of the TernaryFalseExpressionToIfRector in general. Because if method b has null as return type instead of void it happens regardless of the phpstan version used, which is wrong too: https://getrector.com/demo/5d99559b-c987-4e15-9ae8-c44fd2d33023

In my opinion, the rector should never perform a refactor if the else of the Ternary operator is a function call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant