Tokenizer/PHP: fix handling of "DNF look-a-likes" in named parameters

[jrfnl]

Description

The last parameter in a function call using named arguments could be confused with a return type by the tokenizer layer handling type declarations.

The net effect of this was that the close parenthesis of the function call would be retokenized to T_TYPE_CLOSE_PARENTHESIS, which is incorrect and would lead to sniffs incorrectly acting on that information.

Fixed now.

Includes tests.

Suggested changelog entry

• The tokenizer could inadvertently mistake the last parameter in a function call using named arguments for a DNF type.

Related issues/external references

Fixes #504
Fixes #505

Types of changes

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

[fredden]

It would be good to get some tests added as part of this to safeguard that | is tokenised as T_BITWISE_OR, and & as T_BITWISE_AND and ) as T_CLOSE_PARENTHESIS when not part of a disjunctive normal form type declaration. The tests being added here only seem to be safeguarding one side of this bug.

[jrfnl]

It would be good to get some tests added as part of this to safeguard that | is tokenised as T_BITWISE_OR, and & as T_BITWISE_AND and ) as T_CLOSE_PARENTHESIS when not part of a disjunctive normal form type declaration. The tests being added here only seem to be safeguarding one side of this bug.

@fredden The test method these test cases feed into already does a check for tokens between the parentheses:

PHP_CodeSniffer/tests/Core/Tokenizer/PHP/DNFTypesTest.php

Lines 31 to 85 in 8f0d8de

	public function testNormalParentheses($testMarker, $skipCheckInside=false)
	{
	$tokens = $this->phpcsFile->getTokens();
	$openPtr = $this->getTargetToken($testMarker, [T_OPEN_PARENTHESIS, T_TYPE_OPEN_PARENTHESIS]);
	$opener = $tokens[$openPtr];
	$this->assertSame('(', $opener['content'], 'Content of type open parenthesis is not "("');
	$this->assertSame(T_OPEN_PARENTHESIS, $opener['code'], 'Token tokenized as '.$opener['type'].', not T_OPEN_PARENTHESIS (code)');
	$this->assertSame('T_OPEN_PARENTHESIS', $opener['type'], 'Token tokenized as '.$opener['type'].', not T_OPEN_PARENTHESIS (type)');
	$closePtr = $opener['parenthesis_closer'];
	$closer = $tokens[$closePtr];
	$this->assertSame(')', $closer['content'], 'Content of type close parenthesis is not ")"');
	$this->assertSame(T_CLOSE_PARENTHESIS, $closer['code'], 'Token tokenized as '.$closer['type'].', not T_CLOSE_PARENTHESIS (code)');
	$this->assertSame('T_CLOSE_PARENTHESIS', $closer['type'], 'Token tokenized as '.$closer['type'].', not T_CLOSE_PARENTHESIS (type)');
	for ($i = ($openPtr + 1); $i < $closePtr; $i++) {
	// If there are ampersands, make sure these are tokenized as bitwise and.
	if ($skipCheckInside === false && $tokens[$i]['content'] === '&') {
	$this->assertSame(T_BITWISE_AND, $tokens[$i]['code'], 'Token tokenized as '.$tokens[$i]['type'].', not T_BITWISE_AND (code)');
	$this->assertSame('T_BITWISE_AND', $tokens[$i]['type'], 'Token tokenized as '.$tokens[$i]['type'].', not T_BITWISE_AND (type)');
	}
	}
	$before = $this->phpcsFile->findPrevious(Tokens::$emptyTokens, ($openPtr - 1), null, true);
	if ($before !== false && $tokens[$before]['content'] === '|') {
	$this->assertSame(
	T_BITWISE_OR,
	$tokens[$before]['code'],
	'Token before tokenized as '.$tokens[$before]['type'].', not T_BITWISE_OR (code)'
	);
	$this->assertSame(
	'T_BITWISE_OR',
	$tokens[$before]['type'],
	'Token before tokenized as '.$tokens[$before]['type'].', not T_BITWISE_OR (type)'
	);
	}
	$after = $this->phpcsFile->findNext(Tokens::$emptyTokens, ($closePtr + 1), null, true);
	if ($after !== false && $tokens[$after]['content'] === '|') {
	$this->assertSame(
	T_BITWISE_OR,
	$tokens[$after]['code'],
	'Token after tokenized as '.$tokens[$after]['type'].', not T_BITWISE_OR (code)'
	);
	$this->assertSame(
	'T_BITWISE_OR',
	$tokens[$after]['type'],
	'Token after tokenized as '.$tokens[$after]['type'].', not T_BITWISE_OR (type)'
	);
	}
	}//end testNormalParentheses()

Additionally, the original PR added tests with DNF types to both the union type as well as the intersection type test files: https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/461/files#diff-28273165881fe5292d6b2e5599fc0d72b4e80be50e07273d2d96e3a052dfe328

What additional tests did you have in mind ?

[jrfnl]

@fredden Was this what you had in mind ?

[fredden]

@fredden Was this what you had in mind ?

No, but that looks like it covers the concern I had. I was expecting to see some changes to test files named "bitwise" and "parenthesis", but I didn't pause to check what test files already existed. From what I could see, it appeared that this was asserting that the new test cases were not DNF but nothing suggested that they are bitwise operators. I think this is adequately covered now. Thanks for updating this.