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 DowngradeThrowExprRector #8671

Closed
defunctl opened this issue Jun 12, 2024 · 6 comments · Fixed by rectorphp/rector-downgrade-php#224
Closed

Incorrect behavior of DowngradeThrowExprRector #8671

defunctl opened this issue Jun 12, 2024 · 6 comments · Fixed by rectorphp/rector-downgrade-php#224
Labels

Comments

@defunctl
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/f00e4092-b508-48dc-a41a-ebe1139e81a7

<?php declare(strict_types=1);

use \DB;
use \Repository_Exception;

final readonly class DemoFile
{
        private string $table = 'test';
    
	public function find( int $id ): array {
		return DB::table( $this->table )
			->where( 'id', $id )
			->get( ARRAY_A ) ?: throw new Repository_Exception( sprintf( 'Unable to find ID: %s', $id ) );
	}
}

Responsible rules

  • DowngradeReadonlyPropertyRector

  • DowngradeReadonlyClassRector

Expected Behavior

I believe the DowngradeThrowExprRector::refactorTernary() should have been used here?

@defunctl defunctl added the bug label Jun 12, 2024
@samsonasik
Copy link
Member

Confirmed the error https://3v4l.org/koQgQ#v7.4.33

@defunctl
Copy link
Author

Thanks @samsonasik!

Do you know which version this will be released in?

@samsonasik
Copy link
Member

This should be included in next release, you can use dev-main while wait for that.

@defunctl
Copy link
Author

@samsonasik I don't think this should run any statements twice and should instead find a way to assign a variable to a returned value?

For example, I am running database queries and throwing if something went wrong, which means my queries are being run twice once the code is downgraded.

Should I open a new issue?

@samsonasik
Copy link
Member

That's out of scope of downgrade task, more and more we tweak language origin, that may cause unwanted bug, but feel free to create PR improvement for that on CallLike in ternary, or create your own custom rule for that :)

@samsonasik
Copy link
Member

@defunctl Rector 1.1.1 tagged https://github.com/rectorphp/rector/releases/tag/1.1.1

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