From 7fda8cfbc262714d9cc51c1fda9f163f316005c2 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 30 Dec 2020 16:29:25 -0500 Subject: [PATCH 1/5] Add test for foreach with pass-by-reference --- .../fixtures/FunctionWithReferenceFixture.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php index c59b4dae..ea9aa5f1 100644 --- a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php @@ -73,3 +73,10 @@ function function_with_array_walk($userNameParts) { $value = ucfirst($value); }); } + +function function_with_foreach_with_reference($derivatives, $base_plugin_definition) { + foreach ($derivatives as &$entry) { + $entry .= $base_plugin_definition; + } + return $derivatives; +} From f0a72cec6cb09d6bd8516ae3e8bdebe4c9fc0ce9 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 30 Dec 2020 16:31:53 -0500 Subject: [PATCH 2/5] Add counterexample of unused reference variable in foreach --- Tests/VariableAnalysisSniff/VariableAnalysisTest.php | 3 +++ .../fixtures/FunctionWithReferenceFixture.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php index 5344362b..b461ee46 100644 --- a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php +++ b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php @@ -286,6 +286,7 @@ public function testFunctionWithReferenceWarnings() { 59, 60, 64, + 81, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -312,6 +313,7 @@ public function testFunctionWithReferenceWarningsAllowsCustomFunctions() { 40, 46, 64, + 81, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -339,6 +341,7 @@ public function testFunctionWithReferenceWarningsAllowsWordPressFunctionsIfSet() 46, 59, 60, + 81, ]; $this->assertEquals($expectedWarnings, $lines); } diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php index ea9aa5f1..65e9803c 100644 --- a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php @@ -78,5 +78,8 @@ function function_with_foreach_with_reference($derivatives, $base_plugin_definit foreach ($derivatives as &$entry) { $entry .= $base_plugin_definition; } + foreach ($derivatives as &$unused) { // unused variable + $base_plugin_definition .= '1'; + } return $derivatives; } From 17a60a268d6e978f5c0e8085a30981cbe5c83fa5 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 30 Dec 2020 17:07:37 -0500 Subject: [PATCH 3/5] Add isDynamicReference to VariableInfo --- VariableAnalysis/Lib/VariableInfo.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/VariableAnalysis/Lib/VariableInfo.php b/VariableAnalysis/Lib/VariableInfo.php index 028626cc..b0593e52 100644 --- a/VariableAnalysis/Lib/VariableInfo.php +++ b/VariableAnalysis/Lib/VariableInfo.php @@ -30,6 +30,13 @@ class VariableInfo { */ public $referencedVariableScope; + /** + * True if the variable is a reference but one created at runtime + * + * @var bool + */ + public $isDynamicReference = false; + /** * Stack pointer of first declaration * From ffea5c6d69a709203c1b1d7f4b0d910e8c602f87 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 30 Dec 2020 17:08:21 -0500 Subject: [PATCH 4/5] Set isDynamicReference for by-reference foreach loop vars --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 6aa0bfc6..355d900c 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -418,6 +418,7 @@ protected function markVariableAssignment($varName, $stackPtr, $currScope) { return; } $varInfo->firstInitialized = $stackPtr; + Helpers::debug('markVariableAssignment: marked as initialized', $varName); } /** @@ -1164,6 +1165,13 @@ protected function processVariableAsForeachLoopVar(File $phpcsFile, $stackPtr, $ $varInfo->isForeachLoopAssociativeValue = true; } + // Are we pass-by-reference? + $referencePtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true); + if (($referencePtr !== false) && ($tokens[$referencePtr]['code'] === T_BITWISE_AND)) { + Helpers::debug("processVariableAsForeachLoopVar: found foreach loop variable assigned by reference"); + $varInfo->isDynamicReference = true; + } + return true; } From f640b503eff7a68358ebf8e891aa3afbd776db97 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Wed, 30 Dec 2020 17:08:47 -0500 Subject: [PATCH 5/5] Treat writing to a by-reference foreach loopvar as a read --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 355d900c..6ab2deb0 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -945,6 +945,14 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN Helpers::debug('processVariableAsAssignment: marking as assignment in scope', $currScope); $this->markVariableAssignment($varName, $stackPtr, $currScope); + + // If the left-hand-side of the assignment (the variable we are examining) + // is itself a reference, then that counts as a read as well as a write. + $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); + if ($varInfo->isDynamicReference) { + Helpers::debug('processVariableAsAssignment: also marking as a use because variable is a reference'); + $this->markVariableRead($varName, $stackPtr, $currScope); + } } /**