diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php index 11348e49de..f2c021e900 100644 --- a/src/Tokenizers/PHP.php +++ b/src/Tokenizers/PHP.php @@ -1446,15 +1446,11 @@ protected function tokenize($string) && strpos($token[1], '#[') === 0 ) { $subTokens = $this->parsePhpAttribute($tokens, $stackPtr); - if ($subTokens !== null) { - array_splice($tokens, $stackPtr, 1, $subTokens); - $numTokens = count($tokens); + array_splice($tokens, $stackPtr, 1, $subTokens); + $numTokens = count($tokens); - $tokenIsArray = true; - $token = $tokens[$stackPtr]; - } else { - $token[0] = T_ATTRIBUTE; - } + $tokenIsArray = true; + $token = $tokens[$stackPtr]; } if ($tokenIsArray === true @@ -4105,11 +4101,10 @@ private function findCloser(array &$tokens, $start, $openerTokens, $closerChar) * @param array $tokens The original array of tokens (as returned by token_get_all). * @param int $stackPtr The current position in token array. * - * @return array|null The array of parsed attribute tokens + * @return array The array of parsed attribute tokens */ private function parsePhpAttribute(array &$tokens, $stackPtr) { - $token = $tokens[$stackPtr]; $commentBody = substr($token[1], 2); @@ -4121,11 +4116,7 @@ private function parsePhpAttribute(array &$tokens, $stackPtr) && strpos($subToken[1], '#[') === 0 ) { $reparsed = $this->parsePhpAttribute($subTokens, $i); - if ($reparsed !== null) { - array_splice($subTokens, $i, 1, $reparsed); - } else { - $subToken[0] = T_ATTRIBUTE; - } + array_splice($subTokens, $i, 1, $reparsed); } } @@ -4133,6 +4124,16 @@ private function parsePhpAttribute(array &$tokens, $stackPtr) // Go looking for the close bracket. $bracketCloser = $this->findCloser($subTokens, 1, '[', ']'); + + /* + * No closer bracket found, this might be a multi-line attribute, + * but it could also be an unfinished attribute (parse error). + * + * If it is a multi-line attribute, we need to grab a larger part of the code. + * If it is a parse error, we need to stick with only handling the line + * containing the attribute opener. + */ + if (PHP_VERSION_ID < 80000 && $bracketCloser === null) { foreach (array_slice($tokens, ($stackPtr + 1)) as $token) { if (is_array($token) === true) { @@ -4142,20 +4143,17 @@ private function parsePhpAttribute(array &$tokens, $stackPtr) } } - $subTokens = @token_get_all('findCloser($subTokens, 1, '[', ']'); + $bracketCloser = $this->findCloser($newSubTokens, 1, '[', ']'); if ($bracketCloser !== null) { - array_splice($tokens, ($stackPtr + 1), count($tokens), array_slice($subTokens, ($bracketCloser + 1))); - $subTokens = array_slice($subTokens, 0, ($bracketCloser + 1)); + // We found the closer, overwrite the original $subTokens array. + array_splice($tokens, ($stackPtr + 1), count($tokens), array_slice($newSubTokens, ($bracketCloser + 1))); + $subTokens = array_slice($newSubTokens, 0, ($bracketCloser + 1)); } } - if ($bracketCloser === null) { - return null; - } - return $subTokens; }//end parsePhpAttribute() diff --git a/tests/Core/Tokenizers/PHP/AttributesParseError1Test.inc b/tests/Core/Tokenizers/PHP/AttributesParseError1Test.inc new file mode 100644 index 0000000000..3c57b67a6d --- /dev/null +++ b/tests/Core/Tokenizers/PHP/AttributesParseError1Test.inc @@ -0,0 +1,8 @@ + + * @author Juliette Reinders Folmer + * @copyright 2019-2023 Squiz Pty Ltd (ABN 77 084 670 600) + * @copyright 2023 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/HEAD/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Tokenizers\PHP; + +use PHP_CodeSniffer\Tests\Core\Tokenizers\AbstractTokenizerTestCase; + +final class AttributesParseError1Test extends AbstractTokenizerTestCase +{ + + + /** + * Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly + * and that tokens "within" the attribute are not removed. + * + * @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize + * @covers PHP_CodeSniffer\Tokenizers\PHP::findCloser + * @covers PHP_CodeSniffer\Tokenizers\PHP::parsePhpAttribute + * + * @return void + */ + public function testInvalidAttribute() + { + $tokens = $this->phpcsFile->getTokens(); + + $attribute = $this->getTargetToken('/* testInvalidAttribute */', T_ATTRIBUTE); + + $this->assertArrayHasKey('attribute_closer', $tokens[$attribute]); + $this->assertNull($tokens[$attribute]['attribute_closer']); + + $expectedTokenCodes = [ + 'T_ATTRIBUTE', + 'T_STRING', + 'T_WHITESPACE', + 'T_FUNCTION', + ]; + $length = count($expectedTokenCodes); + + $map = array_map( + function ($token) { + if ($token['code'] === T_ATTRIBUTE) { + $this->assertArrayHasKey('attribute_closer', $token); + $this->assertNull($token['attribute_closer']); + } else { + $this->assertArrayNotHasKey('attribute_closer', $token); + } + + return $token['type']; + }, + array_slice($tokens, $attribute, $length) + ); + + $this->assertSame($expectedTokenCodes, $map); + + }//end testInvalidAttribute() + + +}//end class diff --git a/tests/Core/Tokenizers/PHP/AttributesParseError2Test.inc b/tests/Core/Tokenizers/PHP/AttributesParseError2Test.inc new file mode 100644 index 0000000000..a6468d3d57 --- /dev/null +++ b/tests/Core/Tokenizers/PHP/AttributesParseError2Test.inc @@ -0,0 +1,8 @@ +phpcsFile->getTokens(); + + $attribute = $this->getTargetToken('/* testLiveCoding */', T_ATTRIBUTE); + + $this->assertArrayHasKey('attribute_closer', $tokens[$attribute]); + $this->assertNull($tokens[$attribute]['attribute_closer']); + + $expectedTokenCodes = [ + 'T_ATTRIBUTE', + 'T_STRING', + 'T_OPEN_PARENTHESIS', + 'T_LNUMBER', + 'T_CLOSE_PARENTHESIS', + 'T_WHITESPACE', + 'T_FUNCTION', + ]; + + $length = count($expectedTokenCodes); + + $map = array_map( + function ($token) { + if ($token['code'] === T_ATTRIBUTE) { + $this->assertArrayHasKey('attribute_closer', $token); + $this->assertNull($token['attribute_closer']); + } else { + $this->assertArrayNotHasKey('attribute_closer', $token); + } + + return $token['type']; + }, + array_slice($tokens, $attribute, $length) + ); + + $this->assertSame($expectedTokenCodes, $map); + + }//end testInvalidAttribute() + + +}//end class diff --git a/tests/Core/Tokenizers/PHP/AttributesParseError3Test.inc b/tests/Core/Tokenizers/PHP/AttributesParseError3Test.inc new file mode 100644 index 0000000000..1b65a0f85c --- /dev/null +++ b/tests/Core/Tokenizers/PHP/AttributesParseError3Test.inc @@ -0,0 +1,10 @@ +phpcsFile->getTokens(); + + $attribute = $this->getTargetToken('/* testLiveCoding */', T_ATTRIBUTE); + + $this->assertArrayHasKey('attribute_closer', $tokens[$attribute]); + $this->assertNull($tokens[$attribute]['attribute_closer']); + + $expectedTokenCodes = [ + 'T_ATTRIBUTE', + 'T_STRING', + 'T_OPEN_PARENTHESIS', + 'T_LNUMBER', + 'T_CLOSE_PARENTHESIS', + 'T_COMMA', + 'T_WHITESPACE', + 'T_STRING', + 'T_OPEN_PARENTHESIS', + 'T_WHITESPACE', + 'T_WHITESPACE', + 'T_PUBLIC', + 'T_WHITESPACE', + 'T_FINAL', + 'T_WHITESPACE', + 'T_FUNCTION', + ]; + + $length = count($expectedTokenCodes); + + $map = array_map( + function ($token) { + if ($token['code'] === T_ATTRIBUTE) { + $this->assertArrayHasKey('attribute_closer', $token); + $this->assertNull($token['attribute_closer']); + } else { + $this->assertArrayNotHasKey('attribute_closer', $token); + } + + return $token['type']; + }, + array_slice($tokens, $attribute, $length) + ); + + $this->assertSame($expectedTokenCodes, $map); + + }//end testInvalidAttribute() + + +}//end class diff --git a/tests/Core/Tokenizers/PHP/AttributesParseError4Test.inc b/tests/Core/Tokenizers/PHP/AttributesParseError4Test.inc new file mode 100644 index 0000000000..8a5c3f17d5 --- /dev/null +++ b/tests/Core/Tokenizers/PHP/AttributesParseError4Test.inc @@ -0,0 +1,8 @@ +phpcsFile->getTokens(); + + $attribute = $this->getTargetToken('/* testLiveCoding */', T_ATTRIBUTE); + + $expectedTokenCodesAttribute1 = [ + 'T_ATTRIBUTE', + 'T_STRING', + 'T_ATTRIBUTE_END', + 'T_WHITESPACE', + ]; + + $lengthAttribute1 = count($expectedTokenCodesAttribute1); + + $map = array_map( + function ($token) use ($attribute, $lengthAttribute1) { + if ($token['code'] !== T_WHITESPACE) { + $this->assertArrayHasKey('attribute_closer', $token); + $this->assertSame(($attribute + 2), $token['attribute_closer']); + } + + return $token['type']; + }, + array_slice($tokens, $attribute, $lengthAttribute1) + ); + + $this->assertSame($expectedTokenCodesAttribute1, $map); + + $expectedTokenCodesAttribute2 = [ + 'T_ATTRIBUTE', + 'T_STRING', + 'T_WHITESPACE', + 'T_FUNCTION', + ]; + + $lengthAttribute2 = count($expectedTokenCodesAttribute2); + + $map = array_map( + function ($token) { + if ($token['code'] === T_ATTRIBUTE) { + $this->assertArrayHasKey('attribute_closer', $token); + $this->assertNull($token['attribute_closer']); + } else { + $this->assertArrayNotHasKey('attribute_closer', $token); + } + + return $token['type']; + }, + array_slice($tokens, ($attribute + $lengthAttribute1), $lengthAttribute2) + ); + + $this->assertSame($expectedTokenCodesAttribute2, $map); + + }//end testInvalidAttribute() + + +}//end class diff --git a/tests/Core/Tokenizers/PHP/AttributesTest.inc b/tests/Core/Tokenizers/PHP/AttributesTest.inc index 9d56c09527..c44c026c29 100644 --- a/tests/Core/Tokenizers/PHP/AttributesTest.inc +++ b/tests/Core/Tokenizers/PHP/AttributesTest.inc @@ -58,7 +58,7 @@ function attribute_multiline_with_comment_test() {} function single_attribute_on_parameter_test(#[ParamAttribute] int $param) {} /* testMultipleAttributesOnParameter */ -function multiple_attributes_on_parameter_test(#[ParamAttribute, AttributeWithParams(/* another comment */ 'foo')] int $param) {} +function multiple_attributes_on_parameter_test(#[ParamAttribute, AttributeWithParams(/* another comment */ 'foo')] array $param) {} /* testFqcnAttribute */ #[Boo\QualifiedName, \Foo\FullyQualifiedName('foo')] @@ -73,7 +73,7 @@ function multiline_attributes_on_parameter_test(#[ AttributeWithParams( 'foo' ) - ] int $param) {} + ] null $param) {} /* testAttributeContainingTextLookingLikeCloseTag */ #[DeprecationReason('reason: ')] @@ -84,7 +84,3 @@ function attribute_containing_text_looking_like_close_tag() {} 'reason: ' )] function attribute_containing_mulitline_text_looking_like_close_tag() {} - -/* testInvalidAttribute */ -#[ThisIsNotAnAttribute -function invalid_attribute_test() {} diff --git a/tests/Core/Tokenizers/PHP/AttributesTest.php b/tests/Core/Tokenizers/PHP/AttributesTest.php index 09071829f9..ec42fe6961 100644 --- a/tests/Core/Tokenizers/PHP/AttributesTest.php +++ b/tests/Core/Tokenizers/PHP/AttributesTest.php @@ -9,6 +9,7 @@ namespace PHP_CodeSniffer\Tests\Core\Tokenizers\PHP; +use PHP_CodeSniffer\Util\Tokens; use PHP_CodeSniffer\Tests\Core\Tokenizers\AbstractTokenizerTestCase; final class AttributesTest extends AbstractTokenizerTestCase @@ -18,8 +19,9 @@ final class AttributesTest extends AbstractTokenizerTestCase /** * Test that attributes are parsed correctly. * - * @param string $testMarker The comment which prefaces the target token in the test file. - * @param array $tokenCodes The codes of tokens inside the attributes. + * @param string $testMarker The comment which prefaces the target token in the test file. + * @param array $tokenCodes The codes of tokens inside the attributes. + * @param int|string $expectedFirstAfter The expected token code for the first token after the attribute. * * @dataProvider dataAttribute * @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize @@ -28,7 +30,7 @@ final class AttributesTest extends AbstractTokenizerTestCase * * @return void */ - public function testAttribute($testMarker, $tokenCodes) + public function testAttribute($testMarker, $tokenCodes, $expectedFirstAfter) { $tokens = $this->phpcsFile->getTokens(); @@ -58,6 +60,9 @@ function ($token) use ($attribute, $length) { $this->assertSame($tokenCodes, $map); + $actualFirstAfter = $this->phpcsFile->findNext(Tokens::$emptyTokens, ($closer + 1), null, true); + $this->assertSame($expectedFirstAfter, $tokens[$actualFirstAfter]['code']); + }//end testAttribute() @@ -72,14 +77,15 @@ public static function dataAttribute() { return [ 'class attribute' => [ - 'testMarker' => '/* testAttribute */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttribute */', + 'tokenCodes' => [ T_STRING, ], + 'expectedFirstAfter' => T_CLASS, ], 'class attribute with param' => [ - 'testMarker' => '/* testAttributeWithParams */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeWithParams */', + 'tokenCodes' => [ T_STRING, T_OPEN_PARENTHESIS, T_STRING, @@ -87,10 +93,11 @@ public static function dataAttribute() T_STRING, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_CLASS, ], 'class attribute with named param' => [ - 'testMarker' => '/* testAttributeWithNamedParam */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeWithNamedParam */', + 'tokenCodes' => [ T_STRING, T_OPEN_PARENTHESIS, T_PARAM_NAME, @@ -101,16 +108,18 @@ public static function dataAttribute() T_STRING, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_CLASS, ], 'function attribute' => [ - 'testMarker' => '/* testAttributeOnFunction */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeOnFunction */', + 'tokenCodes' => [ T_STRING, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute with params' => [ - 'testMarker' => '/* testAttributeOnFunctionWithParams */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeOnFunctionWithParams */', + 'tokenCodes' => [ T_STRING, T_OPEN_PARENTHESIS, T_CONSTANT_ENCAPSED_STRING, @@ -128,10 +137,11 @@ public static function dataAttribute() T_CLOSE_SHORT_ARRAY, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute with arrow function as param' => [ - 'testMarker' => '/* testAttributeWithShortClosureParameter */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeWithShortClosureParameter */', + 'tokenCodes' => [ T_STRING, T_OPEN_PARENTHESIS, T_STATIC, @@ -149,10 +159,11 @@ public static function dataAttribute() T_VARIABLE, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute; multiple comma separated classes' => [ - 'testMarker' => '/* testAttributeGrouping */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeGrouping */', + 'tokenCodes' => [ T_STRING, T_COMMA, T_WHITESPACE, @@ -179,10 +190,11 @@ public static function dataAttribute() T_CLOSE_SHORT_ARRAY, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute; multiple comma separated classes, one per line' => [ - 'testMarker' => '/* testAttributeMultiline */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeMultiline */', + 'tokenCodes' => [ T_WHITESPACE, T_WHITESPACE, T_STRING, @@ -214,10 +226,11 @@ public static function dataAttribute() T_CLOSE_PARENTHESIS, T_WHITESPACE, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute; multiple comma separated classes, one per line, with comments' => [ - 'testMarker' => '/* testAttributeMultilineWithComment */', - 'tokenCodes' => [ + 'testMarker' => '/* testAttributeMultilineWithComment */', + 'tokenCodes' => [ T_WHITESPACE, T_WHITESPACE, T_STRING, @@ -252,10 +265,11 @@ public static function dataAttribute() T_CLOSE_PARENTHESIS, T_WHITESPACE, ], + 'expectedFirstAfter' => T_FUNCTION, ], 'function attribute; using partially qualified and fully qualified class names' => [ - 'testMarker' => '/* testFqcnAttribute */', - 'tokenCodes' => [ + 'testMarker' => '/* testFqcnAttribute */', + 'tokenCodes' => [ T_STRING, T_NS_SEPARATOR, T_STRING, @@ -269,6 +283,7 @@ public static function dataAttribute() T_CONSTANT_ENCAPSED_STRING, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_FUNCTION, ], ]; @@ -325,9 +340,11 @@ public function testAttributeAndLineComment() /** * Test that attributes on function declaration parameters are parsed correctly. * - * @param string $testMarker The comment which prefaces the target token in the test file. - * @param int $position The token position (starting from T_FUNCTION) of T_ATTRIBUTE token. - * @param array $tokenCodes The codes of tokens inside the attributes. + * @param string $testMarker The comment which prefaces the target token in the test file. + * @param int $position The token position (starting from T_FUNCTION) of T_ATTRIBUTE token. + * @param array $tokenCodes The codes of tokens inside the attributes. + * @param int|string $expectedFirstAfter The expected token code for the first token after the attribute. + * @param string $expectedFirstAfterContent The expected token code for the first token after the attribute. * * @dataProvider dataAttributeOnParameters * @@ -337,7 +354,7 @@ public function testAttributeAndLineComment() * * @return void */ - public function testAttributeOnParameters($testMarker, $position, array $tokenCodes) + public function testAttributeOnParameters($testMarker, $position, array $tokenCodes, $expectedFirstAfter, $expectedFirstAfterContent) { $tokens = $this->phpcsFile->getTokens(); @@ -354,8 +371,8 @@ public function testAttributeOnParameters($testMarker, $position, array $tokenCo $closer = $tokens[$attribute]['attribute_closer']; $this->assertSame(T_WHITESPACE, $tokens[($closer + 1)]['code']); - $this->assertSame(T_STRING, $tokens[($closer + 2)]['code']); - $this->assertSame('int', $tokens[($closer + 2)]['content']); + $this->assertSame($expectedFirstAfter, $tokens[($closer + 2)]['code']); + $this->assertSame($expectedFirstAfterContent, $tokens[($closer + 2)]['content']); $this->assertSame(T_VARIABLE, $tokens[($closer + 4)]['code']); $this->assertSame('$param', $tokens[($closer + 4)]['content']); @@ -386,16 +403,18 @@ public static function dataAttributeOnParameters() { return [ 'parameter attribute; single, inline' => [ - 'testMarker' => '/* testSingleAttributeOnParameter */', - 'position' => 4, - 'tokenCodes' => [ + 'testMarker' => '/* testSingleAttributeOnParameter */', + 'position' => 4, + 'tokenCodes' => [ T_STRING, ], + 'expectedFirstAfter' => T_STRING, + 'expectedFirstAfterContent' => 'int', ], 'parameter attribute; multiple comma separated, inline' => [ - 'testMarker' => '/* testMultipleAttributesOnParameter */', - 'position' => 4, - 'tokenCodes' => [ + 'testMarker' => '/* testMultipleAttributesOnParameter */', + 'position' => 4, + 'tokenCodes' => [ T_STRING, T_COMMA, T_WHITESPACE, @@ -406,11 +425,13 @@ public static function dataAttributeOnParameters() T_CONSTANT_ENCAPSED_STRING, T_CLOSE_PARENTHESIS, ], + 'expectedFirstAfter' => T_STRING, + 'expectedFirstAfterContent' => 'array', ], 'parameter attribute; single, multiline' => [ - 'testMarker' => '/* testMultilineAttributesOnParameter */', - 'position' => 4, - 'tokenCodes' => [ + 'testMarker' => '/* testMultilineAttributesOnParameter */', + 'position' => 4, + 'tokenCodes' => [ T_WHITESPACE, T_WHITESPACE, T_STRING, @@ -424,6 +445,8 @@ public static function dataAttributeOnParameters() T_WHITESPACE, T_WHITESPACE, ], + 'expectedFirstAfter' => T_NULL, + 'expectedFirstAfterContent' => 'null', ], ]; @@ -581,27 +604,6 @@ public static function dataAttributeOnTextLookingLikeCloseTag() }//end dataAttributeOnTextLookingLikeCloseTag() - /** - * Test that invalid attribute (or comment starting with #[ and without ]) are parsed correctly. - * - * @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize - * @covers PHP_CodeSniffer\Tokenizers\PHP::findCloser - * @covers PHP_CodeSniffer\Tokenizers\PHP::parsePhpAttribute - * - * @return void - */ - public function testInvalidAttribute() - { - $tokens = $this->phpcsFile->getTokens(); - - $attribute = $this->getTargetToken('/* testInvalidAttribute */', T_ATTRIBUTE); - - $this->assertArrayHasKey('attribute_closer', $tokens[$attribute]); - $this->assertNull($tokens[$attribute]['attribute_closer']); - - }//end testInvalidAttribute() - - /** * Test that nested attributes are parsed correctly. *