Skip to content

Commit 2e3a010

Browse files
jrfnljrfnl
committed
PHP Tokenizer: improve arrow function backfill consistency
This commit implements the proposal as outlined in 2859#issuecomment-583695917. The `fn` keyword will now only be tokenized as `T_FN` if it really is an arrow function, in which case it will also have the additional indexes set in the token array. In all other cases - including when the keyword is used in a function declaration for a named function or method -, it will be tokenized as `T_STRING`. Includes numerous additional unit tests. Includes removing the changes made to the `File::getDeclarationName()` function and the `AbstractPatternSniff` in commit 37dda44 as those are no longer necessary. Fixes #2859
1 parent fbf67ef commit 2e3a010

File tree

5 files changed

+121
-40
lines changed

5 files changed

+121
-40
lines changed

‎src/Files/File.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,9 +1254,7 @@ public function getDeclarationName($stackPtr)
12541254

12551255
$content = null;
12561256
for ($i = $stackPtr; $i < $this->numTokens; $i++) {
1257-
if ($this->tokens[$i]['code'] === T_STRING
1258-
|| $this->tokens[$i]['code'] === T_FN
1259-
) {
1257+
if ($this->tokens[$i]['code'] === T_STRING) {
12601258
$content = $this->tokens[$i]['content'];
12611259
break;
12621260
}

‎src/Sniffs/AbstractPatternSniff.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,9 +614,7 @@ protected function processPattern($patternInfo, File $phpcsFile, $stackPtr)
614614
$stackPtr = ($tokens[$next][$pattern[$i]['to']] + 1);
615615
}//end if
616616
} else if ($pattern[$i]['type'] === 'string') {
617-
if ($tokens[$stackPtr]['code'] !== T_STRING
618-
&& $tokens[$stackPtr]['code'] !== T_FN
619-
) {
617+
if ($tokens[$stackPtr]['code'] !== T_STRING) {
620618
$hasError = true;
621619
}
622620

‎src/Tokenizers/PHP.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1894,7 +1894,7 @@ protected function processAdditional()
18941894
$this->tokens[$i]['scope_closer'] = $scopeCloser;
18951895
$this->tokens[$i]['parenthesis_owner'] = $i;
18961896
$this->tokens[$i]['parenthesis_opener'] = $x;
1897-
$this->tokens[$i]['parenthesis_closer'] = $this->tokens[$x]['parenthesis_closer'];
1897+
$this->tokens[$i]['parenthesis_closer'] = $closer;
18981898

18991899
$this->tokens[$arrow]['code'] = T_FN_ARROW;
19001900
$this->tokens[$arrow]['type'] = 'T_FN_ARROW';
@@ -1918,6 +1918,12 @@ protected function processAdditional()
19181918
}//end if
19191919
}//end if
19201920
}//end if
1921+
1922+
// If after all that, the extra tokens are not set, this is not an arrow function.
1923+
if (isset($this->tokens[$i]['scope_closer']) === false) {
1924+
$this->tokens[$i]['code'] = T_STRING;
1925+
$this->tokens[$i]['type'] = 'T_STRING';
1926+
}
19211927
} else if ($this->tokens[$i]['code'] === T_OPEN_SQUARE_BRACKET) {
19221928
if (isset($this->tokens[$i]['bracket_closer']) === false) {
19231929
continue;

‎tests/Core/Tokenizer/BackfillFnTokenTest.inc‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,49 @@ fn(array $a) : array => $a;
7373
/* testTernary */
7474
$fn = fn($a) => $a ? /* testTernaryThen */ fn() : string => 'a' : /* testTernaryElse */ fn() : string => 'b';
7575

76+
/* testConstantDeclaration */
77+
const FN = 'a';
78+
79+
class Foo {
80+
/* testStaticMethodName */
81+
public static function fn($param) {
82+
/* testNestedInMethod */
83+
$fn = fn($c) => $callable($factory($c), $c);
84+
}
85+
86+
public function foo() {
87+
/* testPropertyAssignment */
88+
$this->fn = 'a';
89+
}
90+
}
91+
92+
$anon = new class() {
93+
/* testAnonClassMethodName */
94+
protected function fn($param) {
95+
}
96+
}
97+
98+
/* testNonArrowStaticMethodCall */
99+
$a = Foo::fn($param);
100+
101+
/* testNonArrowStaticMethodCallWithChaining */
102+
$a = Foo::fn($param)->another();
103+
104+
/* testNonArrowStaticConstant */
105+
$a = MyClass::FN;
106+
107+
/* testNonArrowStaticConstantDeref */
108+
$a = MyClass::FN[$a];
109+
110+
/* testNonArrowObjectMethodCall */
111+
$a = $obj->fn($param);
112+
113+
/* testNonArrowNamespacedFunctionCall */
114+
$a = MyNS\Sub\fn($param);
115+
116+
/* testNonArrowNamespaceOperatorFunctionCall */
117+
$a = namespace\fn($param);
118+
76119
/* testLiveCoding */
77120
// Intentional parse error. This has to be the last test in the file.
78121
$fn = fn

‎tests/Core/Tokenizer/BackfillFnTokenTest.php‎

Lines changed: 69 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -101,28 +101,6 @@ public function testComments()
101101
}//end testComments()
102102

103103

104-
/**
105-
* Test a function called fn.
106-
*
107-
* @covers PHP_CodeSniffer\Tokenizers\PHP::processAdditional
108-
*
109-
* @return void
110-
*/
111-
public function testFunctionName()
112-
{
113-
$tokens = self::$phpcsFile->getTokens();
114-
115-
$token = $this->getTargetToken('/* testFunctionName */', T_FN);
116-
$this->assertFalse(array_key_exists('scope_condition', $tokens[$token]), 'Scope condition is set');
117-
$this->assertFalse(array_key_exists('scope_opener', $tokens[$token]), 'Scope opener is set');
118-
$this->assertFalse(array_key_exists('scope_closer', $tokens[$token]), 'Scope closer is set');
119-
$this->assertFalse(array_key_exists('parenthesis_owner', $tokens[$token]), 'Parenthesis owner is set');
120-
$this->assertFalse(array_key_exists('parenthesis_opener', $tokens[$token]), 'Parenthesis opener is set');
121-
$this->assertFalse(array_key_exists('parenthesis_closer', $tokens[$token]), 'Parenthesis closer is set');
122-
123-
}//end testFunctionName()
124-
125-
126104
/**
127105
* Test nested arrow functions.
128106
*
@@ -553,27 +531,85 @@ public function testTernary()
553531

554532

555533
/**
556-
* Test that the backfill presumes T_FN during live coding, but doesn't set the additional index keys.
534+
* Test arrow function nested within a method declaration.
557535
*
558536
* @covers PHP_CodeSniffer\Tokenizers\PHP::processAdditional
559537
*
560538
* @return void
561539
*/
562-
public function testLiveCoding()
540+
public function testNestedInMethod()
541+
{
542+
$tokens = self::$phpcsFile->getTokens();
543+
544+
$token = $this->getTargetToken('/* testNestedInMethod */', T_FN);
545+
$this->backfillHelper($token);
546+
547+
$this->assertSame($tokens[$token]['scope_opener'], ($token + 5), 'Scope opener is not the arrow token');
548+
$this->assertSame($tokens[$token]['scope_closer'], ($token + 17), 'Scope closer is not the semicolon token');
549+
550+
$opener = $tokens[$token]['scope_opener'];
551+
$this->assertSame($tokens[$opener]['scope_opener'], ($token + 5), 'Opener scope opener is not the arrow token');
552+
$this->assertSame($tokens[$opener]['scope_closer'], ($token + 17), 'Opener scope closer is not the semicolon token');
553+
554+
$closer = $tokens[$token]['scope_opener'];
555+
$this->assertSame($tokens[$closer]['scope_opener'], ($token + 5), 'Closer scope opener is not the arrow token');
556+
$this->assertSame($tokens[$closer]['scope_closer'], ($token + 17), 'Closer scope closer is not the semicolon token');
557+
558+
}//end testNestedInMethod()
559+
560+
561+
/**
562+
* Verify that "fn" keywords which are not arrow functions get tokenized as T_STRING and don't
563+
* have the extra token array indexes.
564+
*
565+
* @param string $testMarker The comment prefacing the target token.
566+
*
567+
* @dataProvider dataNotAnArrowFunction
568+
* @covers PHP_CodeSniffer\Tokenizers\PHP::processAdditional
569+
*
570+
* @return void
571+
*/
572+
public function testNotAnArrowFunction($testMarker)
563573
{
564574
$tokens = self::$phpcsFile->getTokens();
565575

566-
$token = $this->getTargetToken('/* testLiveCoding */', [T_STRING, T_FN]);
567-
$this->assertSame($tokens[$token]['code'], T_FN, 'Token not tokenized as T_FN');
576+
$token = $this->getTargetToken('/* testFunctionName */', [T_STRING, T_FN], 'fn');
577+
$tokenArray = $tokens[$token];
578+
579+
$this->assertSame('T_STRING', $tokenArray['type'], 'Token tokenized as '.$tokenArray['type'].', not T_STRING');
580+
581+
$this->assertArrayNotHasKey('scope_condition', $tokenArray, 'Scope condition is set');
582+
$this->assertArrayNotHasKey('scope_opener', $tokenArray, 'Scope opener is set');
583+
$this->assertArrayNotHasKey('scope_closer', $tokenArray, 'Scope closer is set');
584+
$this->assertArrayNotHasKey('parenthesis_owner', $tokenArray, 'Parenthesis owner is set');
585+
$this->assertArrayNotHasKey('parenthesis_opener', $tokenArray, 'Parenthesis opener is set');
586+
$this->assertArrayNotHasKey('parenthesis_closer', $tokenArray, 'Parenthesis closer is set');
568587

569-
$this->assertArrayNotHasKey('scope_condition', $tokens[$token], 'Scope condition is set');
570-
$this->assertArrayNotHasKey('scope_opener', $tokens[$token], 'Scope opener is set');
571-
$this->assertArrayNotHasKey('scope_closer', $tokens[$token], 'Scope closer is set');
572-
$this->assertArrayNotHasKey('parenthesis_owner', $tokens[$token], 'Parenthesis owner is set');
573-
$this->assertArrayNotHasKey('parenthesis_opener', $tokens[$token], 'Parenthesis opener is set');
574-
$this->assertArrayNotHasKey('parenthesis_closer', $tokens[$token], 'Parenthesis closer is set');
588+
}//end testNotAnArrowFunction()
589+
590+
591+
/**
592+
* Data provider.
593+
*
594+
* @see testNotAnArrowFunction()
595+
*
596+
* @return array
597+
*/
598+
public function dataNotAnArrowFunction()
599+
{
600+
return [
601+
['/* testFunctionName */'],
602+
['/* testStaticMethodName */'],
603+
['/* testAnonClassMethodName */'],
604+
['/* testNonArrowStaticMethodCall */'],
605+
['/* testNonArrowStaticMethodCallWithChaining */'],
606+
['/* testNonArrowObjectMethodCall */'],
607+
['/* testNonArrowNamespacedFunctionCall */'],
608+
['/* testNonArrowNamespaceOperatorFunctionCall */'],
609+
['/* testLiveCoding */'],
610+
];
575611

576-
}//end testLiveCoding()
612+
}//end dataNotAnArrowFunction()
577613

578614

579615
/**

0 commit comments

Comments
 (0)