From 3c25994278b9f27c3406df0bdbf412e4fc526be4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 20 Apr 2022 03:30:48 +0200 Subject: [PATCH 1/2] BCFile::findStartOfStatement(): add tests for upstream bugfix for switch handling When inside a `switch` `case`/`default` `break`/`continue`/`return`/`exit`/`throw` statement, the start of the statement was not correctly determined. This adds unit tests safeguarding the upstream change. Refs: * squizlabs/php_codesniffer 3192 * squizlabs/PHP_CodeSniffer 3186 --- .../BCFile/FindStartOfStatementTest.inc | 30 +++++ .../BCFile/FindStartOfStatementTest.php | 106 ++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc b/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc index 366047b6..6e2a6a6c 100644 --- a/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc +++ b/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc @@ -10,3 +10,33 @@ $value = [ Text::make('Title') ]), ]; + +switch ($foo) { + /* testCaseStatement */ + case 1: + /* testInsideCaseStatement */ + $var = doSomething(); + /* testInsideCaseBreakStatement */ + break 2; + + case 2: + /* testInsideCaseContinueStatement */ + continue 2; + + case 3: + /* testInsideCaseReturnStatement */ + return false; + + case 4: + /* testInsideCaseExitStatement */ + exit(1); + + case 5: + /* testInsideCaseThrowStatement */ + throw new Exception(); + + /* testDefaultStatement */ + default: + /* testInsideDefaultContinueStatement */ + continue $var; +} diff --git a/Tests/BackCompat/BCFile/FindStartOfStatementTest.php b/Tests/BackCompat/BCFile/FindStartOfStatementTest.php index 966d5ec5..55086c09 100644 --- a/Tests/BackCompat/BCFile/FindStartOfStatementTest.php +++ b/Tests/BackCompat/BCFile/FindStartOfStatementTest.php @@ -40,4 +40,110 @@ public function testObjectCallPrecededByArrowFunctionAsFunctionCallParameterInAr $this->assertSame($expected, $found); } + + /** + * Test finding the start of a statement inside a switch control structure case/default statement. + * + * @link https://github.com/squizlabs/php_codesniffer/issues/3192 + * @link https://github.com/squizlabs/PHP_CodeSniffer/pull/3186/commits/18a0e54735bb9b3850fec266e5f4c50dacf618ea + * + * @dataProvider dataFindStartInsideSwitchCaseDefaultStatements + * + * @param string $testMarker The comment which prefaces the target token in the test file. + * @param array|string|int $targets The token to search for after the test marker. + * @param string|int $expectedTarget Token code of the expected start of statement stack pointer. + * + * @return void + */ + public function testFindStartInsideSwitchCaseDefaultStatements($testMarker, $targets, $expectedTarget) + { + $testToken = $this->getTargetToken($testMarker, $targets); + $expected = $this->getTargetToken($testMarker, $expectedTarget); + + $found = BCFile::findStartOfStatement(self::$phpcsFile, $testToken); + + $this->assertSame($expected, $found); + } + + /** + * Data provider. + * + * @return array + */ + public static function dataFindStartInsideSwitchCaseDefaultStatements() + { + return [ + 'Case keyword should be start of case statement - case itself' => [ + 'testMarker' => '/* testCaseStatement */', + 'targets' => \T_CASE, + 'expectedTarget' => \T_CASE, + ], + 'Case keyword should be start of case statement - number (what\'s being compared)' => [ + 'testMarker' => '/* testCaseStatement */', + 'targets' => \T_LNUMBER, + 'expectedTarget' => \T_CASE, + ], + 'Variable should be start of arbitrary assignment statement - variable itself' => [ + 'testMarker' => '/* testInsideCaseStatement */', + 'targets' => \T_VARIABLE, + 'expectedTarget' => \T_VARIABLE, + ], + 'Variable should be start of arbitrary assignment statement - equal sign' => [ + 'testMarker' => '/* testInsideCaseStatement */', + 'targets' => \T_EQUAL, + 'expectedTarget' => \T_VARIABLE, + ], + 'Variable should be start of arbitrary assignment statement - function call' => [ + 'testMarker' => '/* testInsideCaseStatement */', + 'targets' => \T_STRING, + 'expectedTarget' => \T_VARIABLE, + ], + 'Break should be start for contents of the break statement - contents' => [ + 'testMarker' => '/* testInsideCaseBreakStatement */', + 'targets' => \T_LNUMBER, + 'expectedTarget' => \T_BREAK, + ], + 'Continue should be start for contents of the continue statement - contents' => [ + 'testMarker' => '/* testInsideCaseContinueStatement */', + 'targets' => \T_LNUMBER, + 'expectedTarget' => \T_CONTINUE, + ], + 'Return should be start for contents of the return statement - contents' => [ + 'testMarker' => '/* testInsideCaseReturnStatement */', + 'targets' => \T_FALSE, + 'expectedTarget' => \T_RETURN, + ], + 'Exit should be start for contents of the exit statement - close parenthesis' => [ + // Note: not sure if this is actually correct - should this be the open parenthesis ? + 'testMarker' => '/* testInsideCaseExitStatement */', + 'targets' => \T_CLOSE_PARENTHESIS, + 'expectedTarget' => \T_EXIT, + ], + 'Throw should be start for contents of the throw statement - new keyword' => [ + 'testMarker' => '/* testInsideCaseThrowStatement */', + 'targets' => \T_NEW, + 'expectedTarget' => \T_THROW, + ], + 'Throw should be start for contents of the throw statement - exception name' => [ + 'testMarker' => '/* testInsideCaseThrowStatement */', + 'targets' => \T_STRING, + 'expectedTarget' => \T_THROW, + ], + 'Throw should be start for contents of the throw statement - close parenthesis' => [ + 'testMarker' => '/* testInsideCaseThrowStatement */', + 'targets' => \T_CLOSE_PARENTHESIS, + 'expectedTarget' => \T_THROW, + ], + 'Default keyword should be start of default statement - default itself' => [ + 'testMarker' => '/* testDefaultStatement */', + 'targets' => \T_DEFAULT, + 'expectedTarget' => \T_DEFAULT, + ], + 'Return should be start for contents of the return statement (inside default) - variable' => [ + 'testMarker' => '/* testInsideDefaultContinueStatement */', + 'targets' => \T_VARIABLE, + 'expectedTarget' => \T_CONTINUE, + ], + ]; + } } From 81c6e9e67e60607125b4fdd34f4533139b1a6c37 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 5 Oct 2022 17:51:08 +0200 Subject: [PATCH 2/2] BCFile::findStartOfStatement(): sync in new tests from upstream Refs: * https://github.com/squizlabs/PHP_CodeSniffer/commit/ef80e53de0e3c78f1a5599310d2b9803061ae9bd * https://github.com/squizlabs/PHP_CodeSniffer/commit/97f78982711bdf122200dc37be6f33dfbd4aa4dd * https://github.com/squizlabs/PHP_CodeSniffer/commit/ae4f33bc1ba3c779681275a506c44f8dff378c28 * https://github.com/squizlabs/PHP_CodeSniffer/commit/faff2f08399546faa306f6903b3df3920a462eae * https://github.com/squizlabs/PHP_CodeSniffer/commit/bffe7a54cab1d36f8a1a8499ba847fd551fbdc01 * https://github.com/squizlabs/PHP_CodeSniffer/commit/3540a5d0cdebc2eac09950dd01fcac8c0deeaaff Co-authored-by: Greg Sherwood --- .../BCFile/FindStartOfStatementTest.inc | 131 ++++++ .../BCFile/FindStartOfStatementTest.php | 435 ++++++++++++++++++ 2 files changed, 566 insertions(+) diff --git a/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc b/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc index 6e2a6a6c..7061c473 100644 --- a/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc +++ b/Tests/BackCompat/BCFile/FindStartOfStatementTest.inc @@ -1,5 +1,136 @@ $foo + $bar, 'b' => true]; + +/* testUseGroup */ +use Vendor\Package\{ClassA as A, ClassB, ClassC as C}; + +$a = [ + /* testArrowFunctionArrayValue */ + 'a' => fn() => return 1, + 'b' => fn() => return 1, +]; + +/* testStaticArrowFunction */ +static fn ($a) => $a; + +/* testArrowFunctionReturnValue */ +fn(): array => [a($a, $b)]; + +/* testArrowFunctionAsArgument */ +$foo = foo( + fn() => bar() +); + +/* testArrowFunctionWithArrayAsArgument */ +$foo = foo( + fn() => [$row[0], $row[3]] +); + +$match = match ($a) { + /* testMatchCase */ + 1 => 'foo', + /* testMatchDefault */ + default => 'bar' +}; + +$match = match ($a) { + /* testMatchMultipleCase */ + 1, 2, => $a * $b, + /* testMatchDefaultComma */ + default, => 'something' +}; + +match ($pressedKey) { + /* testMatchFunctionCall */ + Key::RETURN_ => save($value, $user) +}; + +$result = match (true) { + /* testMatchFunctionCallArm */ + str_contains($text, 'Welcome') || str_contains($text, 'Hello') => 'en', + str_contains($text, 'Bienvenue') || str_contains($text, 'Bonjour') => 'fr', + default => 'pl' +}; + +/* testMatchClosure */ +$result = match ($key) { + 1 => function($a, $b) {}, + 2 => function($b, $c) {}, +}; + +/* testMatchArray */ +$result = match ($key) { + 1 => [1,2,3], + 2 => [1 => one($a, $b), 2 => two($b, $c)], + 3 => [], +}; + +/* testNestedMatch */ +$result = match ($key) { + 1 => match ($key) { + 1 => 'one', + 2 => 'two', + }, + 2 => match ($key) { + 1 => 'two', + 2 => 'one', + }, +}; + +return 0; + +/* testOpenTag */ +?> +

Test

+', foo(), ''; + +/* testOpenTagWithEcho */ +?> +

Test

+', foo(), ''; + +?> $song->url()) diff --git a/Tests/BackCompat/BCFile/FindStartOfStatementTest.php b/Tests/BackCompat/BCFile/FindStartOfStatementTest.php index 55086c09..6d3f4b7f 100644 --- a/Tests/BackCompat/BCFile/FindStartOfStatementTest.php +++ b/Tests/BackCompat/BCFile/FindStartOfStatementTest.php @@ -23,6 +23,441 @@ class FindStartOfStatementTest extends UtilityMethodTestCase { + /** + * Test a simple assignment. + * + * @return void + */ + public function testSimpleAssignment() + { + $start = $this->getTargetToken('/* testSimpleAssignment */', T_SEMICOLON); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 5), $found); + } + + /** + * Test a function call. + * + * @return void + */ + public function testFunctionCall() + { + $start = $this->getTargetToken('/* testFunctionCall */', T_CLOSE_PARENTHESIS); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 6), $found); + } + + /** + * Test a function call. + * + * @return void + */ + public function testFunctionCallArgument() + { + $start = $this->getTargetToken('/* testFunctionCallArgument */', T_VARIABLE, '$b'); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame($start, $found); + } + + /** + * Test a direct call to a control structure. + * + * @return void + */ + public function testControlStructure() + { + $start = $this->getTargetToken('/* testControlStructure */', T_CLOSE_CURLY_BRACKET); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 6), $found); + } + + /** + * Test the assignment of a closure. + * + * @return void + */ + public function testClosureAssignment() + { + $start = $this->getTargetToken('/* testClosureAssignment */', T_CLOSE_CURLY_BRACKET); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 12), $found); + } + + /** + * Test using a heredoc in a function argument. + * + * @return void + */ + public function testHeredocFunctionArg() + { + // Find the start of the function. + $start = $this->getTargetToken('/* testHeredocFunctionArg */', T_SEMICOLON); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 10), $found); + + // Find the start of the heredoc. + $start -= 4; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 4), $found); + + // Find the start of the last arg. + $start += 2; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame($start, $found); + } + + /** + * Test parts of a switch statement. + * + * @return void + */ + public function testSwitch() + { + // Find the start of the switch. + $start = $this->getTargetToken('/* testSwitch */', T_CLOSE_CURLY_BRACKET); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 47), $found); + + // Find the start of default case. + $start -= 5; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 6), $found); + + // Find the start of the second case. + $start -= 12; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 5), $found); + + // Find the start of the first case. + $start -= 13; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 8), $found); + + // Test inside the first case. + --$start; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 1), $found); + } + + /** + * Test statements that are array values. + * + * @return void + */ + public function testStatementAsArrayValue() + { + // Test short array syntax. + $start = $this->getTargetToken('/* testStatementAsArrayValue */', T_STRING, 'Datetime'); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 2), $found); + + // Test long array syntax. + $start += 12; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 2), $found); + + // Test same statement outside of array. + ++$start; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 9), $found); + + // Test with an array index. + $start += 17; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 5), $found); + } + + /** + * Test a use group. + * + * @return void + */ + public function testUseGroup() + { + $start = $this->getTargetToken('/* testUseGroup */', T_SEMICOLON); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 23), $found); + } + + /** + * Test arrow function as array value. + * + * @return void + */ + public function testArrowFunctionArrayValue() + { + $start = $this->getTargetToken('/* testArrowFunctionArrayValue */', T_COMMA); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 9), $found); + } + + /** + * Test static arrow function. + * + * @return void + */ + public function testStaticArrowFunction() + { + $start = $this->getTargetToken('/* testStaticArrowFunction */', T_SEMICOLON); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 11), $found); + } + + /** + * Test arrow function with return value. + * + * @return void + */ + public function testArrowFunctionReturnValue() + { + $start = $this->getTargetToken('/* testArrowFunctionReturnValue */', T_SEMICOLON); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 18), $found); + } + + /** + * Test arrow function used as a function argument. + * + * @return void + */ + public function testArrowFunctionAsArgument() + { + $start = $this->getTargetToken('/* testArrowFunctionAsArgument */', T_FN); + $start += 8; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 8), $found); + } + + /** + * Test arrow function with arrays used as a function argument. + * + * @return void + */ + public function testArrowFunctionWithArrayAsArgument() + { + $start = $this->getTargetToken('/* testArrowFunctionWithArrayAsArgument */', T_FN); + $start += 17; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 17), $found); + } + + /** + * Test simple match expression case. + * + * @return void + */ + public function testMatchCase() + { + $start = $this->getTargetToken('/* testMatchCase */', T_COMMA); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 1), $found); + } + + /** + * Test simple match expression default case. + * + * @return void + */ + public function testMatchDefault() + { + $start = $this->getTargetToken('/* testMatchDefault */', T_CONSTANT_ENCAPSED_STRING, "'bar'"); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame($start, $found); + } + + /** + * Test multiple comma-separated match expression case values. + * + * @return void + */ + public function testMatchMultipleCase() + { + $start = $this->getTargetToken('/* testMatchMultipleCase */', T_MATCH_ARROW); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 6), $found); + + $start += 6; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 4), $found); + } + + /** + * Test match expression default case with trailing comma. + * + * @return void + */ + public function testMatchDefaultComma() + { + $start = $this->getTargetToken('/* testMatchDefaultComma */', T_MATCH_ARROW); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 3), $found); + + $start += 2; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame($start, $found); + } + + /** + * Test match expression with function call. + * + * @return void + */ + public function testMatchFunctionCall() + { + $start = $this->getTargetToken('/* testMatchFunctionCall */', T_CLOSE_PARENTHESIS); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 6), $found); + } + + /** + * Test match expression with function call in the arm. + * + * @return void + */ + public function testMatchFunctionCallArm() + { + // Check the first case. + $start = $this->getTargetToken('/* testMatchFunctionCallArm */', T_MATCH_ARROW); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 18), $found); + + // Check the second case. + $start += 24; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 18), $found); + } + + /** + * Test match expression with closure. + * + * @return void + */ + public function testMatchClosure() + { + $start = $this->getTargetToken('/* testMatchClosure */', T_LNUMBER); + $start += 14; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 10), $found); + + $start += 17; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 10), $found); + } + + /** + * Test match expression with array declaration. + * + * @return void + */ + public function testMatchArray() + { + // Start of first case statement. + $start = $this->getTargetToken('/* testMatchArray */', T_LNUMBER); + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + $this->assertSame($start, $found); + + // Comma after first statement. + $start += 11; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + $this->assertSame(($start - 7), $found); + + // Start of second case statement. + $start += 3; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + $this->assertSame($start, $found); + + // Comma after first statement. + $start += 30; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + $this->assertSame(($start - 26), $found); + } + + /** + * Test nested match expressions. + * + * @return void + */ + public function testNestedMatch() + { + $start = $this->getTargetToken('/* testNestedMatch */', T_LNUMBER); + $start += 30; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 26), $found); + + $start -= 4; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 1), $found); + + $start -= 3; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 2), $found); + } + + /** + * Test full PHP open tag. + * + * @return void + */ + public function testOpenTag() + { + $start = $this->getTargetToken('/* testOpenTag */', T_OPEN_TAG); + $start += 2; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 1), $found); + } + + /** + * Test PHP open tag with echo. + * + * @return void + */ + public function testOpenTagWithEcho() + { + $start = $this->getTargetToken('/* testOpenTagWithEcho */', T_OPEN_TAG_WITH_ECHO); + $start += 3; + $found = BCFile::findStartOfStatement(self::$phpcsFile, $start); + + $this->assertSame(($start - 1), $found); + } + /** * Test object call on result of static function call with arrow function as parameter and wrapped within an array. *