diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php index 2f0bd8bb..c59b4dae 100644 --- a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php @@ -63,3 +63,13 @@ function function_with_ignored_reference_call() { function function_with_wordpress_reference_calls() { wp_parse_str('foo=bar', $vars); } + +function function_with_array_walk($userNameParts) { + array_walk($userNameParts, function (string &$value): void { + if (strlen($value) <= 3) { + return; + } + + $value = ucfirst($value); + }); +} diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 5bde2274..6f4a87d6 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -1015,4 +1015,52 @@ public static function isTokenInsideAssignmentRHS(File $phpcsFile, $stackPtr) { } return false; } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return bool + */ + public static function isTokenInsideAssignmentLHS(File $phpcsFile, $stackPtr) { + // Is the next non-whitespace an assignment? + $assignPtr = self::getNextAssignPointer($phpcsFile, $stackPtr); + if (! is_int($assignPtr)) { + return false; + } + + // Is this a variable variable? If so, it's not an assignment to the current variable. + if (self::isTokenVariableVariable($phpcsFile, $stackPtr)) { + self::debug('found variable variable'); + return false; + } + return true; + } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return bool + */ + public static function isTokenVariableVariable(File $phpcsFile, $stackPtr) { + $tokens = $phpcsFile->getTokens(); + + $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true); + if ($prev === false) { + return false; + } + if ($tokens[$prev]['code'] === T_DOLLAR) { + return true; + } + if ($tokens[$prev]['code'] !== T_OPEN_CURLY_BRACKET) { + return false; + } + + $prevPrev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true); + if ($prevPrev !== false && $tokens[$prevPrev]['code'] === T_DOLLAR) { + return true; + } + return false; + } } diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 4be183da..6aa0bfc6 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -368,33 +368,36 @@ protected function getVariableInfo($varName, $currScope) { * @return VariableInfo */ protected function getOrCreateVariableInfo($varName, $currScope) { + Helpers::debug("getOrCreateVariableInfo: starting for '{$varName}'"); $scopeInfo = $this->getOrCreateScopeInfo($currScope); - if (!isset($scopeInfo->variables[$varName])) { - Helpers::debug("creating a new variable for '{$varName}' in scope", $scopeInfo); - $scopeInfo->variables[$varName] = new VariableInfo($varName); - $validUnusedVariableNames = (empty($this->validUnusedVariableNames)) - ? [] - : Helpers::splitStringToArray('/\s+/', trim($this->validUnusedVariableNames)); - $validUndefinedVariableNames = (empty($this->validUndefinedVariableNames)) - ? [] - : Helpers::splitStringToArray('/\s+/', trim($this->validUndefinedVariableNames)); - if (in_array($varName, $validUnusedVariableNames)) { - $scopeInfo->variables[$varName]->ignoreUnused = true; - } - if (isset($this->ignoreUnusedRegexp) && preg_match($this->ignoreUnusedRegexp, $varName) === 1) { - $scopeInfo->variables[$varName]->ignoreUnused = true; - } - if ($scopeInfo->scopeStartIndex === 0 && $this->allowUndefinedVariablesInFileScope) { - $scopeInfo->variables[$varName]->ignoreUndefined = true; - } - if (in_array($varName, $validUndefinedVariableNames)) { - $scopeInfo->variables[$varName]->ignoreUndefined = true; - } - if (isset($this->validUndefinedVariableRegexp) && preg_match($this->validUndefinedVariableRegexp, $varName) === 1) { - $scopeInfo->variables[$varName]->ignoreUndefined = true; - } - } - Helpers::debug("scope for '{$varName}' is now", $scopeInfo); + if (isset($scopeInfo->variables[$varName])) { + Helpers::debug("getOrCreateVariableInfo: found scope for '{$varName}'", $scopeInfo); + return $scopeInfo->variables[$varName]; + } + Helpers::debug("getOrCreateVariableInfo: creating a new variable for '{$varName}' in scope", $scopeInfo); + $scopeInfo->variables[$varName] = new VariableInfo($varName); + $validUnusedVariableNames = (empty($this->validUnusedVariableNames)) + ? [] + : Helpers::splitStringToArray('/\s+/', trim($this->validUnusedVariableNames)); + $validUndefinedVariableNames = (empty($this->validUndefinedVariableNames)) + ? [] + : Helpers::splitStringToArray('/\s+/', trim($this->validUndefinedVariableNames)); + if (in_array($varName, $validUnusedVariableNames)) { + $scopeInfo->variables[$varName]->ignoreUnused = true; + } + if (isset($this->ignoreUnusedRegexp) && preg_match($this->ignoreUnusedRegexp, $varName) === 1) { + $scopeInfo->variables[$varName]->ignoreUnused = true; + } + if ($scopeInfo->scopeStartIndex === 0 && $this->allowUndefinedVariablesInFileScope) { + $scopeInfo->variables[$varName]->ignoreUndefined = true; + } + if (in_array($varName, $validUndefinedVariableNames)) { + $scopeInfo->variables[$varName]->ignoreUndefined = true; + } + if (isset($this->validUndefinedVariableRegexp) && preg_match($this->validUndefinedVariableRegexp, $varName) === 1) { + $scopeInfo->variables[$varName]->ignoreUndefined = true; + } + Helpers::debug("getOrCreateVariableInfo: scope for '{$varName}' is now", $scopeInfo); return $scopeInfo->variables[$varName]; } @@ -406,10 +409,12 @@ protected function getOrCreateVariableInfo($varName, $currScope) { * @return void */ protected function markVariableAssignment($varName, $stackPtr, $currScope) { + Helpers::debug('markVariableAssignment: starting for', $varName); $this->markVariableAssignmentWithoutInitialization($varName, $stackPtr, $currScope); + Helpers::debug('markVariableAssignment: marked as assigned without initialization', $varName); $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); if (isset($varInfo->firstInitialized) && ($varInfo->firstInitialized <= $stackPtr)) { - Helpers::debug('markVariableAssignment variable is already initialized', $varName); + Helpers::debug('markVariableAssignment: variable is already initialized', $varName); return; } $varInfo->firstInitialized = $stackPtr; @@ -427,7 +432,11 @@ protected function markVariableAssignmentWithoutInitialization($varName, $stackP // Is the variable referencing another variable? If so, mark that variable used also. if ($varInfo->referencedVariableScope !== null && $varInfo->referencedVariableScope !== $currScope) { - $this->markVariableAssignment($varInfo->name, $stackPtr, $varInfo->referencedVariableScope); + // Don't do this if the referenced variable does not exist; eg: if it's going to be bound at runtime like in array_walk + if ($this->getVariableInfo($varInfo->name, $varInfo->referencedVariableScope)) { + Helpers::debug('markVariableAssignmentWithoutInitialization: marking referenced variable as assigned also', $varName); + $this->markVariableAssignment($varInfo->name, $stackPtr, $varInfo->referencedVariableScope); + } } if (!isset($varInfo->scopeType)) { @@ -620,12 +629,14 @@ protected function processVariableAsFunctionDefinitionArgument(File $phpcsFile, // 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("processVariableAsFunctionDefinitionArgument found pass-by-reference to scope", $outerScope); $varInfo = $this->getOrCreateVariableInfo($varName, $functionPtr); $varInfo->referencedVariableScope = $outerScope; } // Are we optional with a default? if (Helpers::getNextAssignPointer($phpcsFile, $stackPtr) !== null) { + Helpers::debug("processVariableAsFunctionDefinitionArgument optional with default"); $this->markVariableAssignment($varName, $stackPtr, $functionPtr); } } @@ -896,19 +907,13 @@ protected function processVariableAsStaticOutsideClass(File $phpcsFile, $stackPt * @param string $varName * @param int $currScope * - * @return bool + * @return void */ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varName, $currScope) { - // Is the next non-whitespace an assignment? + Helpers::debug("processVariableAsAssignment: starting for '${varName}'"); $assignPtr = Helpers::getNextAssignPointer($phpcsFile, $stackPtr); if (! is_int($assignPtr)) { - return false; - } - - // Is this a variable variable? If so, it's not an assignment to the current variable. - if ($this->processVariableAsVariableVariable($phpcsFile, $stackPtr)) { - Helpers::debug('found variable variable'); - return false; + return; } // If the right-hand-side of the assignment to this variable is a reference @@ -920,12 +925,12 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN $tokens = $phpcsFile->getTokens(); $referencePtr = $phpcsFile->findNext(Tokens::$emptyTokens, $assignPtr + 1, null, true, null, true); if (is_int($referencePtr) && $tokens[$referencePtr]['code'] === T_BITWISE_AND) { + Helpers::debug('processVariableAsAssignment: found reference variable'); $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); // If the variable was already declared, but was not yet read, it is // unused because we're about to change the binding. $scopeInfo = $this->getOrCreateScopeInfo($currScope); $this->processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo); - Helpers::debug('found reference variable'); // The referenced variable may have a different name, but we don't // actually need to mark it as used in this case because the act of this // assignment will mark it used on the next token. @@ -934,39 +939,11 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN // An assignment to a reference is a binding and should not count as // initialization since it doesn't change any values. $this->markVariableAssignmentWithoutInitialization($varName, $stackPtr, $currScope); - return true; + return; } + Helpers::debug('processVariableAsAssignment: marking as assignment in scope', $currScope); $this->markVariableAssignment($varName, $stackPtr, $currScope); - - return true; - } - - /** - * @param File $phpcsFile - * @param int $stackPtr - * - * @return bool - */ - protected function processVariableAsVariableVariable(File $phpcsFile, $stackPtr) { - $tokens = $phpcsFile->getTokens(); - - $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true); - if ($prev === false) { - return false; - } - if ($tokens[$prev]['code'] === T_DOLLAR) { - return true; - } - if ($tokens[$prev]['code'] !== T_OPEN_CURLY_BRACKET) { - return false; - } - - $prevPrev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true); - if ($prevPrev !== false && $tokens[$prevPrev]['code'] === T_DOLLAR) { - return true; - } - return false; } /** @@ -1394,13 +1371,14 @@ protected function processVariable(File $phpcsFile, $stackPtr) { } // Is the next non-whitespace an assignment? - if ($this->processVariableAsAssignment($phpcsFile, $stackPtr, $varName, $currScope)) { + if (Helpers::isTokenInsideAssignmentLHS($phpcsFile, $stackPtr)) { + Helpers::debug('found assignment'); + $this->processVariableAsAssignment($phpcsFile, $stackPtr, $varName, $currScope); if (Helpers::isTokenInsideAssignmentRHS($phpcsFile, $stackPtr) || Helpers::isTokenInsideFunctionCall($phpcsFile, $stackPtr)) { Helpers::debug("found assignment that's also inside an expression"); $this->markVariableRead($varName, $stackPtr, $currScope); return; } - Helpers::debug('found assignment'); return; }