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

elseif vs else if node structure #948

Closed
pmjones opened this issue Sep 24, 2023 · 9 comments
Closed

elseif vs else if node structure #948

pmjones opened this issue Sep 24, 2023 · 9 comments

Comments

@pmjones
Copy link

pmjones commented Sep 24, 2023

This came up in pmjones/php-styler#4 earlier today. PHP-Styler rebuilds this original code using else if:

if ($foo) {
    $foo = 'bar';
} else if ($bar) {
    $foo = 'baz';
}

...to this:

if ($foo) {
    $foo = 'bar';
} else {
    if ($bar) {
        $foo = 'baz';
    }
}

This is because PHP-Parser sees a second If_ as part of the Else_, rather than an Elseif_ that is part of the If_.

How difficult would it be for PHP-Parser to recognize else if as elseif in these conditions? (My guess is that it would involve substantial effort.)

@nikic
Copy link
Owner

nikic commented Sep 24, 2023

It's not difficult, but then I'll get complaints that the distinction between elseif and else if gets lost ;)

You should be able to easily print these without the extra nesting, like this:

if (\count($node->stmts) === 1 && $node->stmts[0] instanceof Stmt\If_) {
// Print as "else if" rather than "else { if }"
return 'else ' . $this->p($node->stmts[0]);
}

@pmjones
Copy link
Author

pmjones commented Sep 24, 2023

but then I'll get complaints

Man, I hear you. :-)

You should be able to easily print these

Excellent, I will try that -- thanks!

@pmjones
Copy link
Author

pmjones commented Sep 24, 2023

Thanks @nikic that did the trick.

@pmjones pmjones closed this as completed Sep 24, 2023
@pmjones
Copy link
Author

pmjones commented May 9, 2024

Update: it turns out that was not quite enough. When something like this occurs ...

if (true) {
    if (true) {
        // foo
    } else {
        // bar
    }
} else {
    if (true) {
        // baz
    } else {
        // dib
    }
}

... the final else (with //dib) disappears.

Instead, this mitigation seems to handle it properly (no disappearing else):

if (
    count($node->stmts) === 1
    && $node->stmts[0] instanceof Stmt\If_
    && ! $node->stmts[0]->elseifs
    && ! $node->stmts[0]->else
) {
    // treat as `elseif`
}

Thanks again for the help, and for this project.

@staabm
Copy link
Contributor

staabm commented May 9, 2024

Fyi we tried similar things in phpstan, but all suggested solutions until now did not work in all cases.

See phpstan/phpstan#10113

@pmjones
Copy link
Author

pmjones commented May 9, 2024

@staabm Thanks for that -- so, no reliable solution?

@staabm
Copy link
Contributor

staabm commented May 9, 2024

Until now, I think no reliable solution, right.

@pmjones
Copy link
Author

pmjones commented May 9, 2024

All right, I may have to go back to pmjones/php-styler#4 and tell him there's nothing to do, other than manually change else if to elseif -- unless someone here can think of other solutions.

@pmjones
Copy link
Author

pmjones commented May 9, 2024

@staabm @herndlm If you recall, can you say what the failure modes were? That is, under what conditions did the else if-to-elseif conversion fail to honor the original logic?

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

No branches or pull requests

3 participants