Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

QA: review stray variables after loops #521

Open
jrfnl opened this issue Jun 5, 2024 · 0 comments
Open

QA: review stray variables after loops #521

jrfnl opened this issue Jun 5, 2024 · 0 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Jun 5, 2024

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).
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant