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

bug: AlternativeSyntaxAnalyzer - fix for nested else #6495

Merged
merged 1 commit into from Jul 20, 2022

Conversation

jrmajor
Copy link
Contributor

@jrmajor jrmajor commented Jul 17, 2022

Fixes #6484.

@jrmajor jrmajor changed the title AlternativeSyntaxAnalyzer - fix for nested else bug: AlternativeSyntaxAnalyzer - fix for nested else Jul 17, 2022
@coveralls
Copy link

coveralls commented Jul 17, 2022

Coverage Status

Coverage increased (+0.008%) to 92.897% when pulling 1ae8b3e on jrmajor:bugfix/bug-6484 into fab645b on FriendsOfPHP:master.

@SpacePossum
Copy link
Contributor

Hi, thanks for this PR!

Maybe we can make the src/Tokenizer/Analyzer/AlternativeSyntaxAnalyzer.php::findAlternativeSyntaxBlockEnd a bit more easy to read now we indeed have to change it to fix the bug.
I was thinking something like:

    public function findAlternativeSyntaxBlockEnd(Tokens $tokens, int $index): int
    {
        if (!isset($tokens[$index])) {
            throw new \InvalidArgumentException("There is no token at index {$index}.");
        }

        if (!$this->isStartOfAlternativeSyntaxBlock($tokens, $index)) {
            throw new \InvalidArgumentException("Token at index {$index} is not the start of an alternative syntax block.");
        }

        $startTokenKind = $tokens[$index]->getId();
        $endTokenKinds = self::ALTERNATIVE_SYNTAX_BLOCK_EDGES[$startTokenKind];

        $findKinds = [[$startTokenKind]];
        foreach ($endTokenKinds as $endTokenKind) {
            $findKinds[] = [$endTokenKind];
        }

        while (true) {
            $index = $tokens->getNextTokenOfKind($index, $findKinds);

            if ($tokens[$index]->isGivenKind($endTokenKinds)) {
                return $index;
            }

            if ($this->isStartOfAlternativeSyntaxBlock($tokens, $index)) {
                $index = $this->findAlternativeSyntaxBlockEnd($tokens, $index);
            }
        }
    }

also, maybe we can update the signature:

private function isStartOfAlternativeSyntaxBlock(Tokens $tokens, $index): bool

into:

private function isStartOfAlternativeSyntaxBlock(Tokens $tokens, int $index): bool

@jrmajor
Copy link
Contributor Author

jrmajor commented Jul 18, 2022

@SpacePossum Thanks for the review, I appreciate your patience with my PRs! I've applied your suggestions and added you as co-author, as you've done most of the work there :)

Co-authored-by: SpacePossum <possumfromspace@gmail.com>
@julienfalque julienfalque merged commit f63eb5d into PHP-CS-Fixer:master Jul 20, 2022
@julienfalque
Copy link
Member

Thank you @jrmajor.

@SpacePossum
Copy link
Contributor

thanks @jrmajor , nice fix! :)

@jrmajor jrmajor deleted the bugfix/bug-6484 branch July 21, 2022 11:54
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 this pull request may close these issues.

Nested alternative syntax if-else conditions
4 participants