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

[SwitchDeclarationSniff] Handle nested switch #3186

Merged
merged 3 commits into from
Feb 1, 2021
Merged

[SwitchDeclarationSniff] Handle nested switch #3186

merged 3 commits into from
Feb 1, 2021

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Dec 15, 2020

This fix the issue reported here: #2800
I added tests.

Edit: Also close #3192

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jan 3, 2021

Friendly ping @jrfnl @gsherwood.

This PR was working for

switch ($foo) {
    case 1:
        switch ($bar) {
            case 1:
                return;
            default:
                return;
        }
    case 2:
        return 2;
}

But not for

switch ($foo) {
    case 1:
        switch ($bar) {
            case 1:
                return;
            default:
                throw new \Exception();
        }
    case 2:
        return 2;
}

After more investigation, it seems that the T_COLON of default has a scope_opener value and the scope_closer is the index of the throw token. So findStartOfStatement is returning new because it's the first token after a scope_closer.

Seems like the same happens with

switch ($foo) {
    case 1:
        switch ($bar) {
            case 1:
                return;
            default:
                return 3;
        }
    case 2:
        return 2;
}

Here findStartOfStatement is returning 3 because the T_RETURN token is the scope closer of the T_COLON of default.

So I made a minimal reproducer:

switch ($foo) {
    case 1:
        return 2;
}

When using findStartOfStatement on the token 2, it return the index of the token 2 instead of the index of the token return.

I made an issue to fix to relate this: #3192

And I added a commit we should fix the issue.

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Jan 18, 2021
@gsherwood gsherwood added this to the 3.6.0 milestone Jan 18, 2021
gsherwood added a commit that referenced this pull request Feb 1, 2021
@gsherwood gsherwood merged commit 15afb2f into squizlabs:master Feb 1, 2021
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Feb 1, 2021
@gsherwood
Copy link
Member

Thanks a lot for writing that code to check all the branches. Looks to work nicely.

Thanks for fixing that bug in findStartOfStatement() as well. I'm a bit surprised it's never been a problem before.

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

Successfully merging this pull request may close these issues.

findStartOfStatement doesn't work correctly inside switch
2 participants