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

switch_to_blog() - flag for missing subsequent restore_current_blog() #651

Open
rebeccahum opened this issue Apr 13, 2021 · 3 comments
Open

Comments

@rebeccahum
Copy link
Contributor

rebeccahum commented Apr 13, 2021

Describe the solution you'd like

From #649 (comment), it would be worth flagging that restore_current_blog() should be called subsequently (if not already).

What code should be reported as a violation?

function switch_blog() {
	$id = get_current_blog_id();

	if ( $id === 2 ) {
		return;
	}

	switch_to_blog( 2 );
}

What code should not be reported as a violation?

function switch_blog() {
	$id = get_current_blog_id();

	if ( $id === 2 ) {
		return;
	}

	switch_to_blog( 2 );
	restore_current_blog();
}
@jrfnl
Copy link
Collaborator

jrfnl commented Apr 15, 2021

Note: while I think it would be good to add a check for this, it should be a warning and the checking for restore_current_blog() should be limited to the scope in which the call to switch_to_blog() is found.

Any such sniff will always have the potential for false positives as a plugin can have a wrapper PluginClass::restore_stuff() function declared in another file and we won't be able to trace the function calls through the code base/across files.

All the same, it is best practice to always call restore_current_blog() after a switch_to_blog() within the same function, so the fact that the sniff has the risk of false positives should not be a deterrent for creating it as the potential for bugs when the call is not made far outweighs the annoyance of false positives from a sniff.

@rebeccahum
Copy link
Contributor Author

#649 (comment) You mentioned that you can see this being a good sniff for WPCS — is this something you'd like me to open an issue there for, @jrfnl?

@jrfnl
Copy link
Collaborator

jrfnl commented Apr 15, 2021

@rebeccahum I wouldn't be surprised if there is an issue open for it already, but yeah, go ahead, though search in the repo first for existing issues.

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

No branches or pull requests

2 participants