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

Fix for Match to Switch when inside binary operation #263

Merged
merged 4 commits into from
Jan 11, 2025

Conversation

peterfox
Copy link
Contributor

@peterfox peterfox commented Jan 11, 2025

Changes

  • adds a new fixture for the switch to match statement rule in the downgrade to PHP 7.2 test
  • adds changes to the DowngradeMatchToSwitchRector rule.

Why

Currently this rule breaks code where the match statement exists within a binary operation. For example:

public function run($value, $booleanFlag)
{
    return match (true) {
        $value === 2 => true,
        default => false,
    } && $booleanFlag;
}

would become

public function run($value, $booleanFlag)
{
    switch (true) {
        case $value === 2:
            return true;
        default:
            return false;
    }
}

This would cause conditions related to the return statement to be lost. This happened in the Rector Laravel project. For now I've just separated the match statement and the binary op, but it would be nice to fix this either way.

This change fixes that by allowing the rest of the return statement to be retained with the case's return statement.

public function run($value, $booleanFlag)
    {
        switch (true) {
            case $value === 2:
                return true && $booleanFlag;
            default:
                return false && $booleanFlag;
        }
    }

@TomasVotruba
Copy link
Member

Looks good, thank you 👍

@samsonasik
Copy link
Member

Let's give it a try, thank you @peterfox

@samsonasik samsonasik merged commit dd8086f into rectorphp:main Jan 11, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants