assuming it was set in the WP loop that loads plugins. (We’ll be reporting that as a defect to them as well, of course 🙂
How could assuming a WordPress variable is set by WordPress when loading plugins possibly be a defect?
[FYI to anyone following along – this is not relevant to the issue reported here.]
Hi Jason,
Setting aside the imprudence of using Global variables at all…
1) $plugin is not a documented WordPress global variable.
2) Assuming a global variable exists and is set to a particular value is bad practice in any case.
3) Doing so when you are writing code that will run with other, unknown third-party modules that all have access to the global namespace is asking for trouble.
In the particular case of this other plugin, they are checking for three global vars to set an include path for their own plugin files. Aside from it being folly to rely on global variables, they have about 12 lines of conditional code that can be simply replaced with dirname(__FILE__)
Cheers,
David
1) $plugin is not a documented WordPress global variable.
Actually it is.
Assuming a global variable exists and is set to a particular value is bad practice in any case.
The Codex is full of examples of Global variables, $pagenow and $wpdb for instance.In the case of WordPress, global variables exist as part of the API. We have to work with what we’re given.
Specifically, line 13 of wpsendgrid.php is setting a global variable $plugin:
$plugin = plugin_basename( __FILE__ );
100% this is wrong on SendGrid’s part, because $plugin is implicitly a reserved WordPress global variable.
But to say the other plugin shouldn’t expect $plugin to be there is nonsense.
I appreciate and enjoyed your point of view though.
Cheers,
Jason 🙂
Update.
I found this $plugin codex example:
static $plugin;
if (!isset($plugin))
$plugin = plugin_basename(__FILE__);
I think this is probably where the SendGrid author got the idea from. Except s/he omitted the static keyword causing the collision with global.
Also, I take back what I said about $plugin, I misunderstood what you said about WP setting it in a loop. Turns out WordPress does not set this variable at all, so it’s not part of the API or the codex. It’s just a badly used global variable, you are right.
Hi Jason,
The codex example you cited is defining a static variable inside a function block, hence its scope is local to that function (not conflicting with the Global variable at all). as an aside, the static keyword does not affect scope—It just causes the variable to hang around in memory so that it’s value is retained and re-used every time that function is executed. In the Sendgrid plugin, the code in question is being executed in the Global namespace.
WordPress does set a global $plugin variable (iteratively, for each plugin loaded in a normal WP installation), and tears it down after running through that foreach loop. They do not however claim it is ‘wordpress-specific’:
https://codex.wordpress.org/Global_Variables
https://codex.wordpress.org/WPMU_Global_Variables
The other plugin I referred to is assuming no Global $plugin variable would ever be set unless it had been set in that loop, and is executing code assuming the value is a string, representing the system path to where its code resides. The second assumption will be true when it is loaded as a regular plugin, but when loaded in a MU environment as a network-wide plugin, its code is not being executed in that loop, but in a loop which sets an (again, global) variable named $network_plugin instead. Since the Sendgrid plugin was executed previously in that same loop, it’s definition of the Global $plugin variable is still defined.
As far as ‘working with what we’re given’: Sadly, yes, when developing code for WordPress, we must deal with it’s, err… quirks. Even so, someone had enough sense to specify “Accessing other globals besides the ones listed below is not recommended.” on the global variables reference page linked above. Doesn’t change the fact that making assumptions about Global variables is bad practice—The WP core team will be the first to tell you there are plenty of bad practices present in the code they maintain.
Finally, no offense, but I’m going to refrain from discussing this further here. This thread was intended as bug report for the Sendgrid team to address, and this conversation is off-topic.
Cheers,
David
We have added the suggested change to the last version of the plugin. Thanks for your feedback!
