fix ifinite loop caused by php brace matcher in embedded lexer context

[haidubogdan]

This pull request tries to fix issue #7803 .

Php brace matcher used for highlighting (if, foreach ) statements like <?php if ($x): ?> ...<?php endif;?> loops through tokens until it finds the opening parenthesis.
The current implementation can trigger a infinite loop for a embedded usage of Php lexer as there is no exit mechanism beside the PhpTokenId.TOKEN check.

[tmysik]

Approved, thank you.

[matthiasblaesing]

@haidubogdan It would be helpful to see an example that causes a loop. While I agree, that a test would be the best case, I think a review can substitute that. Infinite loops are really ugly to test for (at least the failing variant).

[haidubogdan]

@matthiasblaesing
Sure, a mock module with php embedding would be enough?
I didn't work with functional test netbeans yet. But I can check some triggers action on a thread for brace matcher.

[matthiasblaesing]

@haidubogdan I'd like to "see" the problem, so instructions how to reproduce (if necessary with code) would be great.

[haidubogdan]

@matthiasblaesing, @tmysik

I've created this branch master...haidubogdan:netbeans:t_php_bracematcher_infinite_loop_test

It's just for simulating the infinite loop.

Context:

I have a custom language support (text/x-php-test).
I include embedding on the lexer site for some tokens using PHPTokenId.languageInPHP() interpolated in {{ }}
The snippet code is :{{(TestClass^:)}} .
I've put dump of a counter in PHPBracesMatcher on line 119
Doing the test for the class EmbeddedBraceMatchingTest will output the infinite counter progress.

[matthiasblaesing]

@haidubogdan thank you. It makes sense that this was not caught before as for "normal" PHP situations this problem can't manifest. The scanning stops when the first token with a ID <> PHPTokenId.PHP_TOKEN is found. That will be PHP_OPENTAG. That will always be present as without it we don't get into PHP mode. This needs the embedding case to happen. The problem manifests because LexUtilities.findPreviousToken will report the first token if the target set is not found.

So the change makes sense to me. An alternative I would see is to guard the ts.movePrevious inside the do-while loop like this:

                        if(! ts.movePrevious()) {
                            break;
                        }

The movePrevious will report false and exit the loop for the first element in the token sequence.

What do you think?

[haidubogdan]

@haidubogdan thank you. It makes sense that this was not caught before as for "normal" PHP situations this problem can't manifest. The scanning stops when the first token with a ID <> PHPTokenId.PHP_TOKEN is found. That will be PHP_OPENTAG. That will always be present as without it we don't get into PHP mode. This needs the embedding case to happen. The problem manifests because LexUtilities.findPreviousToken will report the first token if the target set is not found.

So the change makes sense to me. An alternative I would see is to guard the ts.movePrevious inside the do-while loop like this:

                        if(! ts.movePrevious()) {
                            break;
                        }

The movePrevious will report false and exit the loop for the first element in the token sequence.

What do you think?

@matthiasblaesing
Seems cleaner :) .
I will use this solution.

[matthiasblaesing]

@haidubogdan thanks for the update. Looks cleaner and I appreciate the added test!
@tmysik would you mind having a second look? The implementation changed and a test was added. I would be inclined to merge.

[tmysik]

Looks good to me, just a minor comment added (related to the coding style). Thank you

[matthiasblaesing]

@haidubogdan Thank you!