Fix tokenizer bug - Heredoc in if
condition broke scope mapping
#295
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I have been looking into #149. I started by creating a failing test which demonstrated the problem, and then worked on making that test pass. This pull request should fix the problem described in #149.
When the tokenizer creates its map of scopes, goes through each token in turn until it finds another scope opener. It then uses recursion to map each set of open/close parenthesises to the corresponding opener (eg,
if
statement). The case of a heredoc embedded within an open/close pair of parenthesises was not handled. There are already special cases foruse
andnamespace
. I have added to this list so that heredocs are also acceptable in this context.PHP_CodeSniffer/src/Tokenizers/Tokenizer.php
Lines 1128 to 1144 in c4251a1
If the scope opener isn't handled, the code assumes that a closer will never be found, and that there must be a parse error. This isn't the case with a heredoc. Adding support for heredocs seems to make sense in this context.
PHP_CodeSniffer/src/Tokenizers/Tokenizer.php
Lines 1194 to 1195 in c4251a1
The tests here could be much more robust / extensive. I've opted for a simple test here, and will leave writing very verbose tests for #146 for now. If it would be preferable to add more tests here, please let me know.
Suggested changelog entry
if
conditions orfunction
definitions).Related issues/external references
Fixes #149
Types of changes
PR checklist