Skip to content

Commit

Permalink
Fixed detection of T_NULLABLE for arrow functions (ref #2708)
Browse files Browse the repository at this point in the history
  • Loading branch information
gsherwood committed Nov 19, 2019
1 parent 0b498ad commit c58e746
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/Standards/Squiz/Tests/PHP/DisallowInlineIfUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ function foo(string $bar, array $baz, ?MyClass $object) : MyClass {}
class Example {
public ?int $scalarType;
}

$a = fn(?\DateTime $x) : ?\DateTime => $x;
12 changes: 10 additions & 2 deletions src/Tokenizers/PHP.php
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,7 @@ protected function tokenize($string)
}

if ($tokenType === T_FUNCTION
|| $tokenType === T_FN
|| isset(Util\Tokens::$methodPrefixes[$tokenType]) === true
) {
if (PHP_CODESNIFFER_VERBOSITY > 1) {
Expand Down Expand Up @@ -1285,6 +1286,11 @@ function return types. We want to keep the parenthesis map clean,
&& $token[0] === T_STRING
&& strtolower($token[1]) === 'fn'
) {
// Modify the original token stack so that
// future checks (like looking for T_NULLABLE) can
// detect the T_FN token more easily.
$tokens[$stackPtr][0] = T_FN;

$finalTokens[$newStackPtr] = [
'content' => $token[1],
'code' => T_FN,
Expand Down Expand Up @@ -1702,8 +1708,10 @@ protected function processAdditional()
if ($this->tokens[$x]['code'] === T_OPEN_PARENTHESIS) {
$ignore = Util\Tokens::$emptyTokens;
$ignore += [
T_STRING => T_STRING,
T_COLON => T_COLON,
T_STRING => T_STRING,
T_COLON => T_COLON,
T_NS_SEPARATOR => T_COLON,
T_NULLABLE => T_COLON,

This comment has been minimized.

Copy link
@michalbundyra

michalbundyra Nov 19, 2019

Contributor

@gsherwood is it correct ?
Shouldn't be as I've suggested:

T_NS_SEPARATOR => T_NS_SEPARATOR,
T_NULLABLE => T_NULLABLE,

?

This comment has been minimized.

Copy link
@gsherwood

gsherwood Nov 19, 2019

Author Member

Thanks, you're right. The array value isn't actually used, which is why that error didn't get picked up, but I've committed that change now.

];

$closer = $this->tokens[$x]['parenthesis_closer'];
Expand Down
3 changes: 3 additions & 0 deletions tests/Core/Tokenizer/BackfillFnTokenTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,6 @@ $a = [

/* testYield */
$a = fn($x) => yield 'k' => $x;

/* testNullableNamespace */
$a = fn(?\DateTime $x) : ?\DateTime => $x;
28 changes: 28 additions & 0 deletions tests/Core/Tokenizer/BackfillFnTokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,34 @@ public function testYield()
}//end testYield()


/**
* Test arrow functions that use nullable namespace types.
*
* @covers PHP_CodeSniffer\Tokenizers\PHP::processAdditional
*
* @return void
*/
public function testNullableNamespace()
{
$tokens = self::$phpcsFile->getTokens();

$token = $this->getTargetToken('/* testNullableNamespace */', T_FN);
$this->backfillHelper($token);

$this->assertSame($tokens[$token]['scope_opener'], ($token + 15), 'Scope opener is not the arrow token');
$this->assertSame($tokens[$token]['scope_closer'], ($token + 18), 'Scope closer is not the semicolon token');

$opener = $tokens[$token]['scope_opener'];
$this->assertSame($tokens[$opener]['scope_opener'], ($token + 15), 'Opener scope opener is not the arrow token');
$this->assertSame($tokens[$opener]['scope_closer'], ($token + 18), 'Opener scope closer is not the semicolon token');

$closer = $tokens[$token]['scope_opener'];
$this->assertSame($tokens[$closer]['scope_opener'], ($token + 15), 'Closer scope opener is not the arrow token');
$this->assertSame($tokens[$closer]['scope_closer'], ($token + 18), 'Closer scope closer is not the semicolon token');

}//end testNullableNamespace()


/**
* Test that anonymous class tokens without parenthesis do not get assigned a parenthesis owner.
*
Expand Down

0 comments on commit c58e746

Please sign in to comment.