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

Unexpected failure in foreach statement #72

Closed
danielbachhuber opened this issue Feb 12, 2019 · 4 comments
Closed

Unexpected failure in foreach statement #72

danielbachhuber opened this issue Feb 12, 2019 · 4 comments

Comments

@danielbachhuber
Copy link

foreach ( $recipe_json as $k => $v ) {
	if ( false !== strpos( $k, '_rendered' ) ) {
		unset( $recipe_json[ $k ] );
	}
}

In this statement, $v is flagged as unused. I guess this is the same as #65?

@sirbrillig
Copy link
Owner

It should be the same, yes. If you set allowUnusedForeachVariables to true, then it will not be reported. But this does continue that discussion, specifically

we can consider if we want that option to be on by default

What do you think? It seems like there's an obvious benefit, but the reason I haven't enabled it by default already is that I haven't taken the time to examine the possible downsides. It definitely hides unused variables and I wonder if that's something that could be problematic for some code.

@danielbachhuber
Copy link
Author

It'd make sense to me to enable allowUnusedForeachVariables by default because PHP doesn't offer any alternatives.

Similarly, I also had to enable allowUnusedCaughtExceptions.

@sirbrillig
Copy link
Owner

PHP doesn't offer any alternatives

I mean, there's foreach ( array_keys( $recipe_json ) as $key ), but I see your point. For large arrays that's another time through.

Let me work up a PR and we'll see what that looks like.

Similarly, I also had to enable allowUnusedCaughtExceptions.

That's a good point and not one I've seen mentioned a lot, but it could probably help more people to have that on be default as well.

@danielbachhuber
Copy link
Author

PHP doesn't offer any alternatives

I mean, there's foreach ( array_keys( $recipe_json ) as $key ), but I see your point. For large arrays that's another time through.

Ah, good point. Even so, I think this is one of those border areas (i.e. there's a reasonable explanation for an unused variable).

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

No branches or pull requests

2 participants