From 1c59b291c23c4f43f95356831e1b779aff16e400 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 31 Aug 2020 00:00:51 +0200 Subject: [PATCH 1/4] Tokenizer: fix nullable type tokenization with namespace operator The `namespace` keyword as an operator is perfectly valid to be used as part of a type declaration. This usage was so far not supported in the tokenizer for the determination whether a `?` is a nullable type token or a ternary. Fixed now. --- src/Tokenizers/PHP.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php index 5180a5f0e8..085d74990a 100644 --- a/src/Tokenizers/PHP.php +++ b/src/Tokenizers/PHP.php @@ -1149,6 +1149,7 @@ protected function tokenize($string) if ($tokenType === T_STRING || $tokenType === T_ARRAY + || $tokenType === T_NAMESPACE || $tokenType === T_NS_SEPARATOR ) { $lastRelevantNonEmpty = $tokenType; @@ -1382,6 +1383,7 @@ function return types. We want to keep the parenthesis map clean, T_CALLABLE => T_CALLABLE, T_SELF => T_SELF, T_PARENT => T_PARENT, + T_NAMESPACE => T_NAMESPACE, T_NS_SEPARATOR => T_NS_SEPARATOR, ]; From db43214122992786b97d329c6394bd3d860b1ff2 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 31 Aug 2020 00:56:56 +0200 Subject: [PATCH 2/4] Tokenizer: fix handling of namespace operator in types for arrow functions The `namespace` keyword as an operator is perfectly valid to be used as part of a type declaration. This usage was so far not supported in the tokenizer for the arrow function backfill. Fixed now. --- src/Tokenizers/PHP.php | 1 + tests/Core/Tokenizer/BackfillFnTokenTest.inc | 3 +++ tests/Core/Tokenizer/BackfillFnTokenTest.php | 28 ++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php index 085d74990a..898536d728 100644 --- a/src/Tokenizers/PHP.php +++ b/src/Tokenizers/PHP.php @@ -1898,6 +1898,7 @@ protected function processAdditional() T_STRING => T_STRING, T_ARRAY => T_ARRAY, T_COLON => T_COLON, + T_NAMESPACE => T_NAMESPACE, T_NS_SEPARATOR => T_NS_SEPARATOR, T_NULLABLE => T_NULLABLE, T_CALLABLE => T_CALLABLE, diff --git a/tests/Core/Tokenizer/BackfillFnTokenTest.inc b/tests/Core/Tokenizer/BackfillFnTokenTest.inc index 083fe6979c..46c165a2d2 100644 --- a/tests/Core/Tokenizer/BackfillFnTokenTest.inc +++ b/tests/Core/Tokenizer/BackfillFnTokenTest.inc @@ -63,6 +63,9 @@ $a = fn($x) => yield 'k' => $x; /* testNullableNamespace */ $a = fn(?\DateTime $x) : ?\DateTime => $x; +/* testNamespaceOperatorInTypes */ +$fn = fn(namespace\Foo $a) : ?namespace\Foo => $a; + /* testSelfReturnType */ fn(self $a) : self => $a; diff --git a/tests/Core/Tokenizer/BackfillFnTokenTest.php b/tests/Core/Tokenizer/BackfillFnTokenTest.php index 55d378333f..9ef8dad442 100644 --- a/tests/Core/Tokenizer/BackfillFnTokenTest.php +++ b/tests/Core/Tokenizer/BackfillFnTokenTest.php @@ -465,6 +465,34 @@ public function testNullableNamespace() }//end testNullableNamespace() + /** + * Test arrow functions that use the namespace operator in the return type. + * + * @covers PHP_CodeSniffer\Tokenizers\PHP::processAdditional + * + * @return void + */ + public function testNamespaceOperatorInTypes() + { + $tokens = self::$phpcsFile->getTokens(); + + $token = $this->getTargetToken('/* testNamespaceOperatorInTypes */', T_FN); + $this->backfillHelper($token); + + $this->assertSame($tokens[$token]['scope_opener'], ($token + 16), 'Scope opener is not the arrow token'); + $this->assertSame($tokens[$token]['scope_closer'], ($token + 19), 'Scope closer is not the semicolon token'); + + $opener = $tokens[$token]['scope_opener']; + $this->assertSame($tokens[$opener]['scope_opener'], ($token + 16), 'Opener scope opener is not the arrow token'); + $this->assertSame($tokens[$opener]['scope_closer'], ($token + 19), 'Opener scope closer is not the semicolon token'); + + $closer = $tokens[$token]['scope_closer']; + $this->assertSame($tokens[$closer]['scope_opener'], ($token + 16), 'Closer scope opener is not the arrow token'); + $this->assertSame($tokens[$closer]['scope_closer'], ($token + 19), 'Closer scope closer is not the semicolon token'); + + }//end testNamespaceOperatorInTypes() + + /** * Test arrow functions that use self/parent/callable/array/static return types. * From f2a5abfea738baaeb271a83d3adabd9f09e6d637 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 30 Aug 2020 23:55:08 +0200 Subject: [PATCH 3/4] File::get[Method|Member][Properties|Parameters](): add support for namespace relative type declarations The `namespace` keyword as an operator is perfectly valid to be used as part of a type declaration. This usage was so far not supported in the `File::getMethodParameters()`, `File::getMethodProperties()` and the `File::getMemberProperties()` methods. Fixes now. Including adding a unit test for each. --- src/Files/File.php | 3 +++ tests/Core/File/GetMemberPropertiesTest.inc | 5 +++++ tests/Core/File/GetMemberPropertiesTest.php | 10 +++++++++ tests/Core/File/GetMethodParametersTest.inc | 3 +++ tests/Core/File/GetMethodParametersTest.php | 22 ++++++++++++++++++++ tests/Core/File/GetMethodPropertiesTest.inc | 3 +++ tests/Core/File/GetMethodPropertiesTest.php | 23 +++++++++++++++++++++ 7 files changed, 69 insertions(+) diff --git a/src/Files/File.php b/src/Files/File.php index 9a34bd0f43..1b82ceb1b8 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -1441,6 +1441,7 @@ public function getMethodParameters($stackPtr) $typeHintEndToken = $i; } break; + case T_NAMESPACE: case T_NS_SEPARATOR: // Part of a type hint or default value. if ($defaultStart === null) { @@ -1630,6 +1631,7 @@ public function getMethodProperties($stackPtr) T_SELF => T_SELF, T_PARENT => T_PARENT, T_STATIC => T_STATIC, + T_NAMESPACE => T_NAMESPACE, T_NS_SEPARATOR => T_NS_SEPARATOR, ]; @@ -1813,6 +1815,7 @@ public function getMemberProperties($stackPtr) T_CALLABLE => T_CALLABLE, T_SELF => T_SELF, T_PARENT => T_PARENT, + T_NAMESPACE => T_NAMESPACE, T_NS_SEPARATOR => T_NS_SEPARATOR, ]; diff --git a/tests/Core/File/GetMemberPropertiesTest.inc b/tests/Core/File/GetMemberPropertiesTest.inc index 8c87c64035..0a511e588d 100644 --- a/tests/Core/File/GetMemberPropertiesTest.inc +++ b/tests/Core/File/GetMemberPropertiesTest.inc @@ -188,3 +188,8 @@ class PHP8Mixed { // Intentional fatal error - nullability is not allowed with mixed, but that's not the concern of the method. private ?mixed $nullableMixed; } + +class NSOperatorInType { + /* testNamespaceOperatorTypeHint */ + public ?namespace\Name $prop; +} diff --git a/tests/Core/File/GetMemberPropertiesTest.php b/tests/Core/File/GetMemberPropertiesTest.php index 3e8d164dcd..dfee43e203 100644 --- a/tests/Core/File/GetMemberPropertiesTest.php +++ b/tests/Core/File/GetMemberPropertiesTest.php @@ -479,6 +479,16 @@ public function dataGetMemberProperties() 'nullable_type' => true, ], ], + [ + '/* testNamespaceOperatorTypeHint */', + [ + 'scope' => 'public', + 'scope_specified' => true, + 'is_static' => false, + 'type' => '?namespace\Name', + 'nullable_type' => true, + ], + ], ]; }//end dataGetMemberProperties() diff --git a/tests/Core/File/GetMethodParametersTest.inc b/tests/Core/File/GetMethodParametersTest.inc index e4d5bfca74..4ffd44221f 100644 --- a/tests/Core/File/GetMethodParametersTest.inc +++ b/tests/Core/File/GetMethodParametersTest.inc @@ -38,3 +38,6 @@ function mixedTypeHint(mixed &...$var1) {} /* testPHP8MixedTypeHintNullable */ // Intentional fatal error - nullability is not allowed with mixed, but that's not the concern of the method. function mixedTypeHintNullable(?Mixed $var1) {} + +/* testNamespaceOperatorTypeHint */ +function namespaceOperatorTypeHint(?namespace\Name $var1) {} diff --git a/tests/Core/File/GetMethodParametersTest.php b/tests/Core/File/GetMethodParametersTest.php index f5d43c1a63..92f51959e6 100644 --- a/tests/Core/File/GetMethodParametersTest.php +++ b/tests/Core/File/GetMethodParametersTest.php @@ -318,6 +318,28 @@ public function testPHP8MixedTypeHintNullable() }//end testPHP8MixedTypeHintNullable() + /** + * Verify recognition of type declarations using the namespace operator. + * + * @return void + */ + public function testNamespaceOperatorTypeHint() + { + $expected = []; + $expected[0] = [ + 'name' => '$var1', + 'content' => '?namespace\Name $var1', + 'pass_by_reference' => false, + 'variable_length' => false, + 'type_hint' => '?namespace\Name', + 'nullable_type' => true, + ]; + + $this->getMethodParametersTestHelper('/* '.__FUNCTION__.' */', $expected); + + }//end testNamespaceOperatorTypeHint() + + /** * Test helper. * diff --git a/tests/Core/File/GetMethodPropertiesTest.inc b/tests/Core/File/GetMethodPropertiesTest.inc index c268ece3a9..82c032efa0 100644 --- a/tests/Core/File/GetMethodPropertiesTest.inc +++ b/tests/Core/File/GetMethodPropertiesTest.inc @@ -80,3 +80,6 @@ function mixedTypeHint() :mixed {} /* testPHP8MixedTypeHintNullable */ // Intentional fatal error - nullability is not allowed with mixed, but that's not the concern of the method. function mixedTypeHintNullable(): ?mixed {} + +/* testNamespaceOperatorTypeHint */ +function namespaceOperatorTypeHint() : ?namespace\Name {} diff --git a/tests/Core/File/GetMethodPropertiesTest.php b/tests/Core/File/GetMethodPropertiesTest.php index 36d57e7301..ee11fc2b9f 100644 --- a/tests/Core/File/GetMethodPropertiesTest.php +++ b/tests/Core/File/GetMethodPropertiesTest.php @@ -452,6 +452,29 @@ public function testPHP8MixedTypeHintNullable() }//end testPHP8MixedTypeHintNullable() + /** + * Test a function with return type using the namespace operator. + * + * @return void + */ + public function testNamespaceOperatorTypeHint() + { + $expected = [ + 'scope' => 'public', + 'scope_specified' => false, + 'return_type' => '?namespace\Name', + 'nullable_return_type' => true, + 'is_abstract' => false, + 'is_final' => false, + 'is_static' => false, + 'has_body' => true, + ]; + + $this->getMethodPropertiesTestHelper('/* '.__FUNCTION__.' */', $expected); + + }//end testNamespaceOperatorTypeHint() + + /** * Test helper. * From 16d2cd1c9ebcf9488ce185558bbd3def31b3f148 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 31 Aug 2020 03:53:46 +0200 Subject: [PATCH 4/4] Tokenizer::recurseScopeMap(): fix scope setting for namespace operators The namespace keyword as a scope opener can never be within another scope, so we can safely ignore it when encountered as it will always be the namespace keyword used as an operator in that case. Includes dedicated unit tests. --- package.xml | 6 ++ src/Tokenizers/Tokenizer.php | 7 ++ .../ScopeSettingWithNamespaceOperatorTest.inc | 19 ++++ .../ScopeSettingWithNamespaceOperatorTest.php | 98 +++++++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.inc create mode 100644 tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.php diff --git a/package.xml b/package.xml index e88cfd2b11..962d1193d5 100644 --- a/package.xml +++ b/package.xml @@ -114,6 +114,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> + + @@ -1986,6 +1988,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> + + @@ -2047,6 +2051,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> + + diff --git a/src/Tokenizers/Tokenizer.php b/src/Tokenizers/Tokenizer.php index 24f11d2505..e0bf22fb01 100644 --- a/src/Tokenizers/Tokenizer.php +++ b/src/Tokenizers/Tokenizer.php @@ -1111,6 +1111,13 @@ private function recurseScopeMap($stackPtr, $depth=1, &$ignore=0) 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 ($tokenType === T_FUNCTION && $this->tokens[$stackPtr]['code'] !== T_FUNCTION ) { diff --git a/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.inc b/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.inc new file mode 100644 index 0000000000..e2d61bb664 --- /dev/null +++ b/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.inc @@ -0,0 +1,19 @@ + new namespace\Baz; diff --git a/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.php b/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.php new file mode 100644 index 0000000000..23cbd9877c --- /dev/null +++ b/tests/Core/Tokenizer/ScopeSettingWithNamespaceOperatorTest.php @@ -0,0 +1,98 @@ + + * @copyright 2020 Squiz Pty Ltd (ABN 77 084 670 600) + * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Tokenizer; + +use PHP_CodeSniffer\Tests\Core\AbstractMethodUnitTest; + +class ScopeSettingWithNamespaceOperatorTest extends AbstractMethodUnitTest +{ + + + /** + * Test that the scope opener/closers are set correctly when the namespace keyword is encountered as an operator. + * + * @param string $testMarker The comment which prefaces the target tokens in the test file. + * @param int|string[] $tokenTypes The token type to search for. + * @param int|string[] $open Optional. The token type for the scope opener. + * @param int|string[] $close Optional. The token type for the scope closer. + * + * @dataProvider dataScopeSetting + * @covers PHP_CodeSniffer\Tokenizers\Tokenizer::recurseScopeMap + * + * @return void + */ + public function testScopeSetting($testMarker, $tokenTypes, $open=T_OPEN_CURLY_BRACKET, $close=T_CLOSE_CURLY_BRACKET) + { + $tokens = self::$phpcsFile->getTokens(); + + $target = $this->getTargetToken($testMarker, $tokenTypes); + $opener = $this->getTargetToken($testMarker, $open); + $closer = $this->getTargetToken($testMarker, $close); + + $this->assertArrayHasKey('scope_opener', $tokens[$target], 'Scope opener missing'); + $this->assertArrayHasKey('scope_closer', $tokens[$target], 'Scope closer missing'); + $this->assertSame($opener, $tokens[$target]['scope_opener'], 'Scope opener not same'); + $this->assertSame($closer, $tokens[$target]['scope_closer'], 'Scope closer not same'); + + $this->assertArrayHasKey('scope_opener', $tokens[$opener], 'Scope opener missing for open curly'); + $this->assertArrayHasKey('scope_closer', $tokens[$opener], 'Scope closer missing for open curly'); + $this->assertSame($opener, $tokens[$opener]['scope_opener'], 'Scope opener not same for open curly'); + $this->assertSame($closer, $tokens[$opener]['scope_closer'], 'Scope closer not same for open curly'); + + $this->assertArrayHasKey('scope_opener', $tokens[$closer], 'Scope opener missing for close curly'); + $this->assertArrayHasKey('scope_closer', $tokens[$closer], 'Scope closer missing for close curly'); + $this->assertSame($opener, $tokens[$closer]['scope_opener'], 'Scope opener not same for close curly'); + $this->assertSame($closer, $tokens[$closer]['scope_closer'], 'Scope closer not same for close curly'); + + }//end testScopeSetting() + + + /** + * Data provider. + * + * @see testScopeSetting() + * + * @return array + */ + public function dataScopeSetting() + { + return [ + [ + '/* testClassExtends */', + [T_CLASS], + ], + [ + '/* testClassImplements */', + [T_ANON_CLASS], + ], + [ + '/* testInterfaceExtends */', + [T_INTERFACE], + ], + [ + '/* testFunctionReturnType */', + [T_FUNCTION], + ], + [ + '/* testClosureReturnType */', + [T_CLOSURE], + ], + [ + '/* testArrowFunctionReturnType */', + [T_FN], + [T_FN_ARROW], + [T_SEMICOLON], + ], + ]; + + }//end dataScopeSetting() + + +}//end class