From a7ece482ec30352d97c5c049aec4825318c263b1 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 12 Mar 2024 11:19:59 -0300 Subject: [PATCH 1/6] Generic/CallTimePassByReference: rename test case file Doing this to be able to create tests with syntax errors on separate files. --- ... => CallTimePassByReferenceUnitTest.1.inc} | 0 .../CallTimePassByReferenceUnitTest.php | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) rename src/Standards/Generic/Tests/Functions/{CallTimePassByReferenceUnitTest.inc => CallTimePassByReferenceUnitTest.1.inc} (100%) diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.inc b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.inc rename to src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php index 4f9198b3d4..e0d2ded5de 100644 --- a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php @@ -26,18 +26,26 @@ final class CallTimePassByReferenceUnitTest extends AbstractSniffUnitTest * The key of the array should represent the line number and the value * should represent the number of errors that should occur on that line. * + * @param string $testFile The name of the test file to process. + * * @return array */ - public function getErrorList() + public function getErrorList($testFile='CallTimePassByReferenceUnitTest.1.inc') { - return [ - 9 => 1, - 12 => 1, - 15 => 1, - 18 => 2, - 23 => 1, - 30 => 1, - ]; + switch ($testFile) { + case 'CallTimePassByReferenceUnitTest.1.inc': + return [ + 9 => 1, + 12 => 1, + 15 => 1, + 18 => 2, + 23 => 1, + 30 => 1, + ]; + + default: + return []; + } }//end getErrorList() From 295b11384c8d8a99962a2bcf98ce2d69e09a8269 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 12 Mar 2024 11:49:01 -0300 Subject: [PATCH 2/6] Generic/CallTimePassByReference: fix non-problematic bug This commit fixes a non-problematic bug when there are only empty tokens after a variable or string token and nothing else. Before this commit, the code would search for the next non-empty token after a variable or string token. It would assume that there would always be such a token and it would check if this token is a open parenthesis token. It is possible that there are no tokens or only empty tokens after the variable or string token when live coding. This assumption would not cause any problems because when checking if `$tokens[$openBracket]['code'] !== T_OPEN_PARENTHESIS`, `$openBracket` would be `false`, and then PHP would check for the first element in the $tokens array. The first token can never be a T_OPEN_PARENTHESIS and so the code would bail early as expected. This commit adds a `$openBracket === false` check to bail early as well to make it clear that `$openBracket` can be false. --- .../Sniffs/Functions/CallTimePassByReferenceSniff.php | 2 +- .../Tests/Functions/CallTimePassByReferenceUnitTest.2.inc | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.2.inc diff --git a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php index d97c0f79bf..f575244345 100644 --- a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php @@ -69,7 +69,7 @@ public function process(File $phpcsFile, $stackPtr) true ); - if ($tokens[$openBracket]['code'] !== T_OPEN_PARENTHESIS) { + if ($openBracket === false || $tokens[$openBracket]['code'] !== T_OPEN_PARENTHESIS) { return; } diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.2.inc b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.2.inc new file mode 100644 index 0000000000..769c5d61ba --- /dev/null +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.2.inc @@ -0,0 +1,7 @@ + Date: Thu, 14 Mar 2024 11:54:16 -0300 Subject: [PATCH 3/6] Generic/CallTimePassByReference: remove unreachable condition Removes a condition that checks if `nested_parenthesis` property of the variable or short syntax array is not set. As far as I can check, this condition can never be true. A few lines above, we are already checking for the presence of opening and closing parentheses and the while loop is limited to T_VARIABLE and T_OPEN_SHORT_ARRAY inside the parentheses. Since it is limited to tokens inside the parentheses, `nested_parenthesis` will always be set. The removed check was added in this commit that added defensive code to protect against syntax errors to a few sniffs: https://github.com/PHPCSStandards/PHP_CodeSniffer/commit/0e42098df29b59b21eac9a009c5941219058366e --- .../Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php index f575244345..0d05fca20d 100644 --- a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php @@ -86,10 +86,6 @@ public function process(File $phpcsFile, $stackPtr) ]; while (($nextSeparator = $phpcsFile->findNext($find, ($nextSeparator + 1), $closeBracket)) !== false) { - if (isset($tokens[$nextSeparator]['nested_parenthesis']) === false) { - continue; - } - if ($tokens[$nextSeparator]['code'] === T_OPEN_SHORT_ARRAY) { $nextSeparator = $tokens[$nextSeparator]['bracket_closer']; continue; From 39b8764378c6633bb58838cd4ba77bd7afb59ee5 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 20 Mar 2024 11:54:25 -0300 Subject: [PATCH 4/6] Generic/CallTimePassByReference: remove unnecessary condition This commit removes the `|| $prevCode === T_CLASS` condition from a if statement. The removed check is not necessary as the next thing the sniff does is to check for an opening parenthesis after `$stackPtr` and bow out if one is not found. Class definitions will never have an opening parentesis after its name. Anonymous classes do have a opening parenthesis after the T_CLASS token. They are currently not handled correctly by this sniff and this will be addressed on another commit. --- .../Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php index 0d05fca20d..73e83288f2 100644 --- a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php @@ -50,12 +50,12 @@ public function process(File $phpcsFile, $stackPtr) $prev = $phpcsFile->findPrevious($findTokens, ($stackPtr - 1), null, true); - // Skip tokens that are the names of functions or classes + // Skip tokens that are the names of functions // within their definitions. For example: function myFunction... // "myFunction" is T_STRING but we should skip because it is not a // function or method *call*. $prevCode = $tokens[$prev]['code']; - if ($prevCode === T_FUNCTION || $prevCode === T_CLASS) { + if ($prevCode === T_FUNCTION) { return; } From 7030f03bbbfc0abbaf80142535d5171ce4a70773 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 14 Mar 2024 14:15:46 -0300 Subject: [PATCH 5/6] Generic/CallTimePassByReference: improve test coverage --- .../Functions/CallTimePassByReferenceUnitTest.1.inc | 12 ++++++++++++ .../Functions/CallTimePassByReferenceUnitTest.3.inc | 7 +++++++ .../Functions/CallTimePassByReferenceUnitTest.php | 3 +++ 3 files changed, 22 insertions(+) create mode 100644 src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.3.inc diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc index 3fa33649eb..6a6db5a3dc 100644 --- a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc @@ -37,3 +37,15 @@ myfunc(MY_CONST&$myvar); efg( true == &$b ); efg( true === &$b ); + +foo($a, bar(&$b)); +foo($a, array(&$b)); + +enum Foo {} +interface Foo {} +trait Foo {} + +$instance = new $var($a); +$instance = new MyClass($a); +$instance = new $var(&$a); +$instance = new MyClass(&$a); diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.3.inc b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.3.inc new file mode 100644 index 0000000000..26f467bd79 --- /dev/null +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.3.inc @@ -0,0 +1,7 @@ + 2, 23 => 1, 30 => 1, + 41 => 1, + 50 => 1, + 51 => 1, ]; default: From b91a18b0106f3e63e3f0b498bc9ce882bcc072f8 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 21 Mar 2024 11:51:35 -0300 Subject: [PATCH 6/6] Generic/CallTimePassByReference: support anonymous classes This commit changes the sniff to flag call-time pass-by-reference arguments when instantiating an anonymous class. --- .../Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php | 1 + .../Tests/Functions/CallTimePassByReferenceUnitTest.1.inc | 3 +++ .../Tests/Functions/CallTimePassByReferenceUnitTest.php | 1 + 3 files changed, 5 insertions(+) diff --git a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php index 73e83288f2..618b32a8c9 100644 --- a/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php @@ -27,6 +27,7 @@ public function register() return [ T_STRING, T_VARIABLE, + T_ANON_CLASS, ]; }//end register() diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc index 6a6db5a3dc..eabef57cdf 100644 --- a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.1.inc @@ -49,3 +49,6 @@ $instance = new $var($a); $instance = new MyClass($a); $instance = new $var(&$a); $instance = new MyClass(&$a); + +$anon = new class($a) {}; +$anon = new class(&$a) {}; diff --git a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php index b4c02c8728..8fdea95ea5 100644 --- a/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.php @@ -44,6 +44,7 @@ public function getErrorList($testFile='CallTimePassByReferenceUnitTest.1.inc') 41 => 1, 50 => 1, 51 => 1, + 54 => 1, ]; default: