From 21afc774468cfdfcaf493c5adba79477ffcdaa9d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Dec 2023 22:33:44 +0100 Subject: [PATCH 1/2] Tests/IsReferenceTest: use named data sets With non-named data sets, when a test fails, PHPUnit will display the number of the test which failed. With tests which have a _lot_ of data sets, this makes it _interesting_ (and time-consuming) to debug those, as one now has to figure out which of the data sets in the data provider corresponds to that number. Using named data sets makes debugging failing tests more straight forward as PHPUnit will display the data set name instead of the number. Using named data sets also documents what exactly each data set is testing. Aside from adding the data set name, this commit also adds the parameter name for each item in the data set, this time in an effort to make it more straight forward to update and add tests as it will be more obvious what each key in the data set signifies. Includes making the data type in the docblock more specific. --- tests/Core/File/IsReferenceTest.php | 296 ++++++++++++++-------------- 1 file changed, 148 insertions(+), 148 deletions(-) diff --git a/tests/Core/File/IsReferenceTest.php b/tests/Core/File/IsReferenceTest.php index 5372bf163c..3dd5ff8501 100644 --- a/tests/Core/File/IsReferenceTest.php +++ b/tests/Core/File/IsReferenceTest.php @@ -44,206 +44,206 @@ public function testIsReference($identifier, $expected) * * @see testIsReference() * - * @return array + * @return array> */ public function dataIsReference() { return [ - [ - '/* testBitwiseAndA */', - false, + 'bitwise and: param in function call' => [ + 'testMarker' => '/* testBitwiseAndA */', + 'expected' => false, ], - [ - '/* testBitwiseAndB */', - false, + 'bitwise and: in unkeyed short array, first value' => [ + 'testMarker' => '/* testBitwiseAndB */', + 'expected' => false, ], - [ - '/* testBitwiseAndC */', - false, + 'bitwise and: in unkeyed short array, last value' => [ + 'testMarker' => '/* testBitwiseAndC */', + 'expected' => false, ], - [ - '/* testBitwiseAndD */', - false, + 'bitwise and: in unkeyed long array, last value' => [ + 'testMarker' => '/* testBitwiseAndD */', + 'expected' => false, ], - [ - '/* testBitwiseAndE */', - false, + 'bitwise and: in keyed short array, last value' => [ + 'testMarker' => '/* testBitwiseAndE */', + 'expected' => false, ], - [ - '/* testBitwiseAndF */', - false, + 'bitwise and: in keyed long array, last value' => [ + 'testMarker' => '/* testBitwiseAndF */', + 'expected' => false, ], - [ - '/* testBitwiseAndG */', - false, + 'bitwise and: in assignment' => [ + 'testMarker' => '/* testBitwiseAndG */', + 'expected' => false, ], - [ - '/* testBitwiseAndH */', - false, + 'bitwise and: in param default value in function declaration' => [ + 'testMarker' => '/* testBitwiseAndH */', + 'expected' => false, ], - [ - '/* testBitwiseAndI */', - false, + 'bitwise and: in param default value in closure declaration' => [ + 'testMarker' => '/* testBitwiseAndI */', + 'expected' => false, ], - [ - '/* testFunctionReturnByReference */', - true, + 'reference: function declared to return by reference' => [ + 'testMarker' => '/* testFunctionReturnByReference */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceA */', - true, + 'reference: only param in function declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceA */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceB */', - true, + 'reference: last param in function declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceB */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceC */', - true, + 'reference: only param in closure declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceC */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceD */', - true, + 'reference: last param in closure declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceD */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceE */', - true, + 'reference: typed param in function declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceE */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceF */', - true, + 'reference: typed param in closure declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceF */', + 'expected' => true, ], - [ - '/* testFunctionPassByReferenceG */', - true, + 'reference: variadic param in function declaration, pass by reference' => [ + 'testMarker' => '/* testFunctionPassByReferenceG */', + 'expected' => true, ], - [ - '/* testForeachValueByReference */', - true, + 'reference: foreach value' => [ + 'testMarker' => '/* testForeachValueByReference */', + 'expected' => true, ], - [ - '/* testForeachKeyByReference */', - true, + 'reference: foreach key' => [ + 'testMarker' => '/* testForeachKeyByReference */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceA */', - true, + 'reference: keyed short array, first value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceA */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceB */', - true, + 'reference: keyed short array, last value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceB */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceC */', - true, + 'reference: unkeyed short array, only value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceC */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceD */', - true, + 'reference: unkeyed short array, last value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceD */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceE */', - true, + 'reference: keyed long array, first value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceE */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceF */', - true, + 'reference: keyed long array, last value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceF */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceG */', - true, + 'reference: unkeyed long array, only value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceG */', + 'expected' => true, ], - [ - '/* testArrayValueByReferenceH */', - true, + 'reference: unkeyed long array, last value, value by reference' => [ + 'testMarker' => '/* testArrayValueByReferenceH */', + 'expected' => true, ], - [ - '/* testAssignByReferenceA */', - true, + 'reference: variable, assign by reference' => [ + 'testMarker' => '/* testAssignByReferenceA */', + 'expected' => true, ], - [ - '/* testAssignByReferenceB */', - true, + 'reference: variable, assign by reference, spacing variation' => [ + 'testMarker' => '/* testAssignByReferenceB */', + 'expected' => true, ], - [ - '/* testAssignByReferenceC */', - true, + 'reference: variable, assign by reference, concat assign' => [ + 'testMarker' => '/* testAssignByReferenceC */', + 'expected' => true, ], - [ - '/* testAssignByReferenceD */', - true, + 'reference: property, assign by reference' => [ + 'testMarker' => '/* testAssignByReferenceD */', + 'expected' => true, ], - [ - '/* testAssignByReferenceE */', - true, + 'reference: function return value, assign by reference' => [ + 'testMarker' => '/* testAssignByReferenceE */', + 'expected' => true, ], - [ - '/* testPassByReferenceA */', - true, + 'reference: first param in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceA */', + 'expected' => true, ], - [ - '/* testPassByReferenceB */', - true, + 'reference: last param in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceB */', + 'expected' => true, ], - [ - '/* testPassByReferenceC */', - true, + 'reference: property in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceC */', + 'expected' => true, ], - [ - '/* testPassByReferenceD */', - true, + 'reference: hierarchical self property in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceD */', + 'expected' => true, ], - [ - '/* testPassByReferenceE */', - true, + 'reference: hierarchical parent property in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceE */', + 'expected' => true, ], - [ - '/* testPassByReferenceF */', - true, + 'reference: hierarchical static property in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceF */', + 'expected' => true, ], - [ - '/* testPassByReferenceG */', - true, + 'reference: static property in function call, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceG */', + 'expected' => true, ], - [ - '/* testPassByReferenceH */', - true, + 'reference: static property in function call, first with FQN, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceH */', + 'expected' => true, ], - [ - '/* testPassByReferenceI */', - true, + 'reference: static property in function call, last with FQN, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceI */', + 'expected' => true, ], - [ - '/* testPassByReferenceJ */', - true, + 'reference: static property in function call, last with namespace relative name, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceJ */', + 'expected' => true, ], - [ - '/* testNewByReferenceA */', - true, + 'reference: new by reference' => [ + 'testMarker' => '/* testNewByReferenceA */', + 'expected' => true, ], - [ - '/* testNewByReferenceB */', - true, + 'reference: new by reference as function call param' => [ + 'testMarker' => '/* testNewByReferenceB */', + 'expected' => true, ], - [ - '/* testUseByReference */', - true, + 'reference: closure use by reference' => [ + 'testMarker' => '/* testUseByReference */', + 'expected' => true, ], - [ - '/* testArrowFunctionReturnByReference */', - true, + 'reference: arrow fn declared to return by reference' => [ + 'testMarker' => '/* testArrowFunctionReturnByReference */', + 'expected' => true, ], - [ - '/* testArrowFunctionPassByReferenceA */', - true, + 'reference: typed param in arrow fn declaration, pass by reference' => [ + 'testMarker' => '/* testArrowFunctionPassByReferenceA */', + 'expected' => true, ], - [ - '/* testArrowFunctionPassByReferenceB */', - true, + 'reference: variadic param in arrow fn declaration, pass by reference' => [ + 'testMarker' => '/* testArrowFunctionPassByReferenceB */', + 'expected' => true, ], - [ - '/* testClosureReturnByReference */', - true, + 'reference: closure declared to return by reference' => [ + 'testMarker' => '/* testClosureReturnByReference */', + 'expected' => true, ], ]; From 3622657175f647fd5e01d2bab7c15f3b68e2a7c6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Dec 2023 22:54:16 +0100 Subject: [PATCH 2/2] Tests/IsReferenceTest: add extra tests This adds some extra tests which were already in use in PHPCSUtils. This brings test coverage for this method up to 100%. It also cleans up the test case file a little by removing some code which isn't actually used in the tests (namespace declaration). --- tests/Core/File/IsReferenceTest.inc | 62 +++++++++++++++- tests/Core/File/IsReferenceTest.php | 105 ++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/tests/Core/File/IsReferenceTest.inc b/tests/Core/File/IsReferenceTest.inc index f71e2639d8..93c7acc677 100644 --- a/tests/Core/File/IsReferenceTest.inc +++ b/tests/Core/File/IsReferenceTest.inc @@ -1,6 +1,8 @@ getValue(); /* testAssignByReferenceE */ $collection = &collector(); +/* testAssignByReferenceF */ +$collection ??= &collector(); + +/* testShortListAssignByReferenceNoKeyA */ +[ + &$a, + /* testShortListAssignByReferenceNoKeyB */ + &$b, + /* testNestedShortListAssignByReferenceNoKey */ + [$c, &$d] +] = $array; + +/* testLongListAssignByReferenceNoKeyA */ +list($a, &$b, list(/* testLongListAssignByReferenceNoKeyB */ &$c, /* testLongListAssignByReferenceNoKeyC */ &$d)) = $array; + +[ + /* testNestedShortListAssignByReferenceWithKeyA */ + 'a' => [&$a, $b], + /* testNestedShortListAssignByReferenceWithKeyB */ + 'b' => [$c, &$d] +] = $array; + + +/* testLongListAssignByReferenceWithKeyA */ +list(get_key()[1] => &$e) = [1, 2, 3]; + /* testPassByReferenceA */ functionCall(&$something, $somethingElse); @@ -128,6 +156,9 @@ functionCall($something, &\SomeNS\SomeClass::$somethingElse); /* testPassByReferenceJ */ functionCall($something, &namespace\SomeClass::$somethingElse); +/* testPassByReferencePartiallyQualifiedName */ +functionCall($something, &Sub\Level\SomeClass::$somethingElse); + /* testNewByReferenceA */ $foobar2 = &new Foobar(); @@ -137,9 +168,27 @@ functionCall( $something , &new Foobar() ); /* testUseByReference */ $closure = function() use (&$var){}; +/* testUseByReferenceWithCommentFirstParam */ +$closure = function() use /*comment*/ (&$this->value){}; + +/* testUseByReferenceWithCommentSecondParam */ +$closure = function() use /*comment*/ ($varA, &$varB){}; + /* testArrowFunctionReturnByReference */ fn&($x) => $x; +$closure = function ( + /* testBitwiseAndExactParameterA */ + $a = MY_CONSTANT & parent::OTHER_CONSTANT, + /* testPassByReferenceExactParameterB */ + &$b, + /* testPassByReferenceExactParameterC */ + &...$c, + /* testBitwiseAndExactParameterD */ + $d = E_NOTICE & E_STRICT, +) {}; + +// Issue PHPCS#3049. /* testArrowFunctionPassByReferenceA */ $fn = fn(array &$one) => 1; @@ -148,3 +197,14 @@ $fn = fn($param, &...$moreParams) => 1; /* testClosureReturnByReference */ $closure = function &($param) use ($value) {}; + +/* testBitwiseAndArrowFunctionInDefault */ +$fn = fn( $one = E_NOTICE & E_STRICT) => 1; + +/* testTokenizerIssue1284PHPCSlt280A */ +if ($foo) {} +[&$a, /* testTokenizerIssue1284PHPCSlt280B */ &$b] = $c; + +/* testTokenizerIssue1284PHPCSlt280C */ +if ($foo) {} +[&$a, $b]; diff --git a/tests/Core/File/IsReferenceTest.php b/tests/Core/File/IsReferenceTest.php index 3dd5ff8501..929779d43b 100644 --- a/tests/Core/File/IsReferenceTest.php +++ b/tests/Core/File/IsReferenceTest.php @@ -20,6 +20,19 @@ class IsReferenceTest extends AbstractMethodUnitTest { + /** + * Test that false is returned when a non-"bitwise and" token is passed. + * + * @return void + */ + public function testNotBitwiseAndToken() + { + $target = $this->getTargetToken('/* testBitwiseAndA */', T_STRING); + $this->assertFalse(self::$phpcsFile->isReference($target)); + + }//end testNotBitwiseAndToken() + + /** * Test correctly identifying whether a "bitwise and" token is a reference or not. * @@ -49,6 +62,14 @@ public function testIsReference($identifier, $expected) public function dataIsReference() { return [ + 'issue-1971-list-first-in-file' => [ + 'testMarker' => '/* testTokenizerIssue1971PHPCSlt330gt271A */', + 'expected' => true, + ], + 'issue-1971-list-first-in-file-nested' => [ + 'testMarker' => '/* testTokenizerIssue1971PHPCSlt330gt271B */', + 'expected' => true, + ], 'bitwise and: param in function call' => [ 'testMarker' => '/* testBitwiseAndA */', 'expected' => false, @@ -177,6 +198,46 @@ public function dataIsReference() 'testMarker' => '/* testAssignByReferenceE */', 'expected' => true, ], + 'reference: function return value, assign by reference, null coalesce assign' => [ + 'testMarker' => '/* testAssignByReferenceF */', + 'expected' => true, + ], + 'reference: unkeyed short list, first var, assign by reference' => [ + 'testMarker' => '/* testShortListAssignByReferenceNoKeyA */', + 'expected' => true, + ], + 'reference: unkeyed short list, second var, assign by reference' => [ + 'testMarker' => '/* testShortListAssignByReferenceNoKeyB */', + 'expected' => true, + ], + 'reference: unkeyed short list, nested var, assign by reference' => [ + 'testMarker' => '/* testNestedShortListAssignByReferenceNoKey */', + 'expected' => true, + ], + 'reference: unkeyed long list, second var, assign by reference' => [ + 'testMarker' => '/* testLongListAssignByReferenceNoKeyA */', + 'expected' => true, + ], + 'reference: unkeyed long list, first nested var, assign by reference' => [ + 'testMarker' => '/* testLongListAssignByReferenceNoKeyB */', + 'expected' => true, + ], + 'reference: unkeyed long list, last nested var, assign by reference' => [ + 'testMarker' => '/* testLongListAssignByReferenceNoKeyC */', + 'expected' => true, + ], + 'reference: keyed short list, first nested var, assign by reference' => [ + 'testMarker' => '/* testNestedShortListAssignByReferenceWithKeyA */', + 'expected' => true, + ], + 'reference: keyed short list, last nested var, assign by reference' => [ + 'testMarker' => '/* testNestedShortListAssignByReferenceWithKeyB */', + 'expected' => true, + ], + 'reference: keyed long list, only var, assign by reference' => [ + 'testMarker' => '/* testLongListAssignByReferenceWithKeyA */', + 'expected' => true, + ], 'reference: first param in function call, pass by reference' => [ 'testMarker' => '/* testPassByReferenceA */', 'expected' => true, @@ -217,6 +278,10 @@ public function dataIsReference() 'testMarker' => '/* testPassByReferenceJ */', 'expected' => true, ], + 'reference: static property in function call, last with PQN, pass by reference' => [ + 'testMarker' => '/* testPassByReferencePartiallyQualifiedName */', + 'expected' => true, + ], 'reference: new by reference' => [ 'testMarker' => '/* testNewByReferenceA */', 'expected' => true, @@ -229,10 +294,34 @@ public function dataIsReference() 'testMarker' => '/* testUseByReference */', 'expected' => true, ], + 'reference: closure use by reference, first param, with comment' => [ + 'testMarker' => '/* testUseByReferenceWithCommentFirstParam */', + 'expected' => true, + ], + 'reference: closure use by reference, last param, with comment' => [ + 'testMarker' => '/* testUseByReferenceWithCommentSecondParam */', + 'expected' => true, + ], 'reference: arrow fn declared to return by reference' => [ 'testMarker' => '/* testArrowFunctionReturnByReference */', 'expected' => true, ], + 'bitwise and: first param default value in closure declaration' => [ + 'testMarker' => '/* testBitwiseAndExactParameterA */', + 'expected' => false, + ], + 'reference: param in closure declaration, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceExactParameterB */', + 'expected' => true, + ], + 'reference: variadic param in closure declaration, pass by reference' => [ + 'testMarker' => '/* testPassByReferenceExactParameterC */', + 'expected' => true, + ], + 'bitwise and: last param default value in closure declaration' => [ + 'testMarker' => '/* testBitwiseAndExactParameterD */', + 'expected' => false, + ], 'reference: typed param in arrow fn declaration, pass by reference' => [ 'testMarker' => '/* testArrowFunctionPassByReferenceA */', 'expected' => true, @@ -245,6 +334,22 @@ public function dataIsReference() 'testMarker' => '/* testClosureReturnByReference */', 'expected' => true, ], + 'bitwise and: param default value in arrow fn declaration' => [ + 'testMarker' => '/* testBitwiseAndArrowFunctionInDefault */', + 'expected' => false, + ], + 'issue-1284-short-list-directly-after-close-curly-control-structure' => [ + 'testMarker' => '/* testTokenizerIssue1284PHPCSlt280A */', + 'expected' => true, + ], + 'issue-1284-short-list-directly-after-close-curly-control-structure-second-item' => [ + 'testMarker' => '/* testTokenizerIssue1284PHPCSlt280B */', + 'expected' => true, + ], + 'issue-1284-short-array-directly-after-close-curly-control-structure' => [ + 'testMarker' => '/* testTokenizerIssue1284PHPCSlt280C */', + 'expected' => true, + ], ]; }//end dataIsReference()