Fix tokenizer bug - Heredoc in if condition broke scope mapping

[fredden]

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 for use and namespace. 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

	// Is this an opening condition ?
	if (isset($this->scopeOpeners[$tokenType]) === true) {
	if ($opener === null) {
	if ($tokenType === T_USE) {
	// PHP use keywords are special because they can be
	// used as blocks but also inline in function definitions.
	// So if we find them nested inside another opener, just skip them.
	continue;
	}
	if ($tokenType === T_NAMESPACE) {
	// PHP namespace keywords are special because they can be
	// used as blocks but also inline as operators.
	// So if we find them nested inside another opener, just skip them.
	continue;
	}

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

	// Found another opening condition but still haven't
	// found our opener, so we are never going to find one.

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

• Fix bug in tokenizer where heredocs were not handled correctly within scope statements (like if conditions or function definitions).

Related issues/external references

Fixes #149

Types of changes

• Bug fix (non-breaking change which fixes an issue)

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.

[jrfnl]

Thanks for this PR @fredden !

I haven't done a proper review yet, but am wondering if squizlabs/PHP_CodeSniffer#3750 could be addressed as well as it seems like a related case ?

For now, I've just left a few small notes inline.

I would love to see more extensive tests for this method, preferably pulled & merged before this PR, but will accept this PR with the current tests as it's better than none anyway.

[jrfnl]

Could you please change this to use the Tokenizer specific testcase I introduced in #314 ? That should allow code coverage to be recorded for this.

[jrfnl]

Is this @covers tag correct ? The change is in the Tokenizer::recurseScopeMap() method ?

[jrfnl]

Suggested change

	* @return array<string, array<string, int>>
	* @return array<string, array<string, int|string>>

[fredden]

This code change introduces a bug where context conditions are no longer being added applied to the heredoc. More tests covering the existing behaviour are required to safeguard changes in the tokenizer.

I'll mark this pull request as draft until we have better confidence in the tests for the tokenizer.