PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

QA: review stray variables after loops

Open jrfnl opened this issue 8 months ago • 0 comments

Inspired by the conversation in this PR review thread.

PHPCS uses for/foreach/while loops a lot and quite often the variables created for those loops are not destroyed after the loop has finished.

This can cause confusion is the variable name is re-used for something else later on in the same code flow. It can also cause problems if a variable name is re-used and conditionally set later on in the same code flow as the wrong variable may end up being used.

To prevent this it would be good to review all code for these code patterns and to add unset()s where appropriate.

Examples:

for ($i = $stackPtr; $i < $phpcsFile->numTokens; $i++) {
    // Do something...
}
+unset($i);
foreach ($patterns as $pattern => $type) {
    // Do something...
}
+unset($pattern, $type);
while (($ptr = $phpcsFile->findNext(Tokens::$ooScopeTokens, ($ptr + 1)) !== false) {
    // Do something...
}
+unset($ptr);

Notes:

  • If a loop is at the end of a function, no unset is needed as the variable will be destroyed once the function ends.
  • If a variable as-it-is-at-the-end-of-the-loop is deliberately re-used later on, the code does not need an update, though it might be helpful at times to add a comment documenting this.

Planning and how to contribute

This task doesn't need to be done in one go. Anyone who wants to contribute to this task can claim a single file, or a group of files to review by leaving a comment in this thread.

We'll keep track of what's (being) done by listing what's been claimed/reviewed below.

Claimed

Nothing yet

Done

Nothing yet

Excluded from this task

  • Code which doesn't have dedicated tests.
  • Files in the src/Tokenizer directory.
  • Sniffs which are specific to JS/CSS (as those will be removed in PHPCS 4.0 anyway).

jrfnl avatar Jun 05 '24 15:06 jrfnl