Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Recognize that get_defined_vars() takes unused parameters #91

Closed
sybrew opened this issue Jun 22, 2019 · 2 comments · Fixed by #92
Closed

Recognize that get_defined_vars() takes unused parameters #91

sybrew opened this issue Jun 22, 2019 · 2 comments · Fixed by #92

Comments

@sybrew
Copy link

sybrew commented Jun 22, 2019

Code speaks a thousand words.

function some_function( $var_1 ) { // VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

    $var_2 = get_something(); // VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

    do_something( get_defined_vars() ); // this should negate above warnings.
}
@sirbrillig
Copy link
Owner

Wow, I somehow never knew about get_defined_vars() before. Thanks for brining this up!

So you're suggesting that if get_defined_vars() is called in a scope, it should disable all unused variable warnings within that scope. That does make sense in the sense that all variables in that scope are being read just by calling that function.

🤔 I'm trying to think of any risks with doing that. Do you have any real-world examples of that function being used that I could reference?

@sybrew
Copy link
Author

sybrew commented Jun 24, 2019

Cheers 😄

Yea, Automattic doesn't seem too fond of that function. I don't know why 🤷‍♂; I only found one (convoluted) instance in Jetpack.

And yes, that's exactly what I'm suggesting!

Aside from the example provided above, I use it when passing variadic parameters to my file inclusion handler*.

Here's a reference: https://github.com/sybrew/the-seo-framework/blob/9638de251d856937b135a57dd5d2d663e81a39b8/inc/classes/profile.class.php#L79-L102

The placement of the error is a bit odd in that reference, but that's a common issue with phpcs.
image

* file inclusions from an object method will also pass $this, and make the object accessible in even the private scope, but that's a story for another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants