Rename is_admin() and related functions for clarity

[SergeyBiryukov]

Background: #16490, #19898, #41090.

Following the introduction of is_login_screen() in [53884], I would like to propose reconsidering the is_*_admin() functions family:

• is_admin()
  • Determines whether the current request is for an administrative interface page.
• is_blog_admin()
  • Whether the current request is for a site's administrative interface, e.g. /wp-admin/.
• is_network_admin()
  • Whether the current request is for the network administrative interface, e.g. /wp-admin/network/.
• is_user_admin()
  • Whether the current request is for a user admin screen, e.g. /wp-admin/user/.

For someone new to WordPress, these names can be quite confusing, especially the last one. When using these functions, one always needs to remember or read the documentation to find out that they don't actually check if the current user is a site administrator.

To complicate things further, there is one more similarly named function that does exactly the latter:

• is_super_admin()
  • Determines whether user is a site admin.

With the above in mind, I think these names would better describe the functionality and match is_login_screen():

• is_admin() → is_admin_screen()
• is_blog_admin() → is_blog_admin_screen() or is_site_admin_screen()
• is_network_admin() → is_network_admin_screen()
• is_user_admin() → is_user_admin_screen()

The problem with deprecating the current names is that they are used in a huge number of ​plugins and ​themes, but I still believe changing them would have a beneficial effect on the project and is feasible in the long run:

1. As a first step, we could add these aliases and adjust the documentation to recommend using the newer names and note that the older names are going to be deprecated in the near future.
2. After a few major releases, we could add deprecation notices to the older functions. By that time, plugins and themes should be able to safely switch to the newer names even if they support multiple WordPress versions.

[tobifjellner]

We could even consider post-fitting these aliases a couple of versions back next time we roll out some security updates.

[jrf]

Just wondering: as there are four "screen" functions and one "user" function and the impact of renaming these functions on userland code is large, what is the reason to favour renaming the (four) "screen" functions instead of renaming the (one and much less used) "user" function ?

is_super_admin() could become is_super_admin_user()

Note: I'm in favour of adding the aliases either way as it allows for more descriptive code, but I'm not sure the original "screen" functions should be deprecated.

[SergeyBiryukov]

Replying to jrf:

Just wondering: as there are four "screen" functions and one "user" function and the impact of renaming these functions on userland code is large, what is the reason to favour renaming the (four) "screen" functions instead of renaming the (one and much less used) "user" function ?

is_super_admin() could become is_super_admin_user()

Thanks! It did not occur to me, but I think it makes perfect sense to add that alias too. Commit incoming :)

[SergeyBiryukov]

In 54259:

Bootstrap/Load: Introduce is_*_admin_screen() aliases for is_*_admin() function family.

Following the introduction of is_login_screen() in [53884], it is time to reconsider the is_*_admin() functions:

• is_admin(): Determines whether the current request is for an administrative interface page.
• is_blog_admin(): Whether the current request is for a site's administrative interface, e.g. /wp-admin/.
• is_network_admin(): Whether the current request is for the network administrative interface, e.g. /wp-admin/network/.
• is_user_admin(): Whether the current request is for a user admin screen, e.g. /wp-admin/user/.

For someone new to WordPress, these names can be quite confusing, especially the last one. When using these functions, one always needs to remember that they don't actually check if the current user is a site administrator.

To complicate things further, there is one more similarly named function that does exactly the latter:

• is_super_admin(): Determines whether user is a site admin.

With the above in mind, this commit introduces aliases that better match the functionality and allow for more descriptive code:

• is_admin() → is_admin_screen()
• is_blog_admin() → is_site_admin_screen()
• is_network_admin() → is_network_admin_screen()
• is_user_admin() → is_user_admin_screen()

Additionally, is_super_admin_user() is introduced as an alias for is_super_admin():

• is_super_admin() → is_super_admin_user()

Plugins and themes are encouraged to start using the newer function names to make code self-descriptive and bring more clarity. The older names are not deprecated at this time, though it may be up for discussion in the future.

Follow-up to [2481], [6412], [12393], [12732], [15558], [15481], [15746], [53884].

Props jrf, tobifjellner, SergeyBiryukov.
See #56400.

[davidbaumwald]

Tagging this for inclusion in the Misc Dev Note for 6.1.

[azaozz]

Hmmm, I'm a bit unsure if these are good changes/renames/aliases. Adding _screen doesn't really make them clearer imho. Even the new is_login_screen() is not any clearer by having screen in there. Seems is_login() or even is_login_request() would be more accurate :)

is_admin() Determines whether the current request is for an administrative interface page.

I agree, generally is_admin() means that the request is for an "administration screen" (as opposed to a front-end screen) but the more important meaning there is that the request was made by an authenticated user. It is unclear if "an administrative interface page" will be loaded as a result of that request. For example: AJAX requests.

Same for the other is_*_screen functions. They may or may not load an admin screen, but the important part is that the requests were made by appropriately authenticated users, imho. Perhaps instead of _screen it should be _request? Seems a bit better and more "truthful", maybe. An alternative would be to add _user to all of these but..... Not sure it would make it clearer. For these reasons thinking that the new aliases aren't better than the original names and may be more confusing in some cases, perhaps?

The more important part here (imho) seems to be to add better descriptions in the docblocks for the is_* functions. That would probably be way more helpful that the aliases as the docblocks are displayed in most IDEs.

[johnbillion]

I agree with @azaozz that these new functions don't appear to add any clarity and the _screen suffix isn't always appropriate (eg in Ajax context). They also should not have been added to the 6.1 milestone this close to beta.

I recommend that these new functions are reverted for 6.1 and we'll revisit them in 6.2.

[SergeyBiryukov]

Replying to azaozz:

Hmmm, I'm a bit unsure if these are good changes/renames/aliases. Adding _screen doesn't really make them clearer imho. Even the new is_login_screen() is not any clearer by having screen in there. Seems is_login() or even is_login_request() would be more accurate :)
...
Same for the other is_*_screen functions. They may or may not load an admin screen, but the important part is that the requests were made by appropriately authenticated users, imho. Perhaps instead of _screen it should be _request? Seems a bit better and more "truthful", maybe.

Thanks for flagging this! With these considerations, I agree _request makes more sense.

is_login() was discussed in comment:6:ticket:19898 and ruled out as too confusing, similar to the existing functions.

56400.diff​ renames the new functions to is_*_request(), along with is_login_screen() from #19898.

The more important part here (imho) seems to be to add better descriptions in the docblocks for the is_* functions. That would probably be way more helpful that the aliases as the docblocks are displayed in most IDEs.

This change is aimed at those who are new to WordPress and perhaps new to web development as well (and may not use an IDE), rather than experienced software engineers, so while I'm all for better descriptions, I don't think they would solve the issue of confusing function names.

[manfcarlo]

I agree that the word "screen" is not appropriate to describe all requests, but the use of the admin_request pattern does not solve the original problem. If someone wrongly thinks "admin" refers to Super Admin then they may still interpret admin_request as being a request by a Super Admin.

[azaozz]

Replying to SergeyBiryukov:

Thanks for flagging this! With these considerations, I agree _request makes more sense.

Actually thinking more about it, this will not bring any benefits, just confusion. There are probably thousands of references to is_admin and the similar in books, posts, plugins, everywhere. Breaking these references is like breaking links: very bad UX :)

Another reason is that the is_admin is "open ended", i.e. it means all of these:

• is-admin-user (authenticated user is making the request)
• is-admin-request
• is-admin-page that may be loaded
• can-use-functions-that-are-only-available-in-admin-context
• is-not-front-end request/page

Changing it to is_admin_request doesn't make it better and will not convey all these meanings. The same is true for is_login. I don't agree with Nacin's comment there that is may be confusing. It just needs better explanation in the docblock.

Also see https://core.trac.wordpress.org/ticket/55883#comment:21 about general sentiment about renaming functions and variables that have existed for many many years. There may be some that would be way better, but generally think it brings a lot of confusion (especially for new contributors) that will not go away in the future as it breaks all existing references.

In these terms I'd like to propose that this is reverted and closed.

[jrf]

@azaozz I agree that causing confusion isn't good and that existing code should not be impacted by this change, but I also agree with @SergeyBiryukov that that confusion already exists and that people often don't fully realize what those functions do and use them incorrectly.

Updating the docs IMO is not a solution, as, if people would read the docs, they wouldn't use the wrong function in the first place, but it still happens.

As I said before in this discussion, I don't think the original functions should be deprecated, but I do think having some aliases which allow for more descriptive code/are more self-explanatory would not necessarily be a bad thing.

You mention the low adoption of the new name in https://core.trac.wordpress.org/ticket/55883#comment:21.
I don't think these changes are comparable as:

1. apply_shortcode() IMO is actually less descriptive than do_shortcode().
2. The new function only being mentioned in one dev-note still makes it relatively unknown.
3. As the "old" function wasn't deprecated, there is no incentive for developers to update existing code.

An honest comparison of the uptake would need to compare the use of the "old" function at the time the rename was introduced vs now, to see the number of new uses which have been introduced between then and now and how many of those use the "old" function name vs the "new" function name. As the "old" numbers aren't mentioned anywhere in that thread, such a comparison is not possible, so no value can be attached to the current usage numbers.

So, what about this ?

• Revert most of the changes in this ticket, except for the adding of the alias is_super_admin_user().
• Document in this ticket the number of usages found in plugins and themes at this time.
• Publish a dev-note about the change.
• Update existing documentation which contains example code for plugins/themes which uses the is_super_admin() to use the new function name.
• Revisit this ticket after 5/6 releases and do an honest comparison of the numbers based on the plugin/theme usage then compared to now (without presuming people will update existing code).

[SergeyBiryukov]

Thanks for the comments!

Replying to azaozz:

Changing it to is_admin_request doesn't make it better and will not convey all these meanings. The same is true for is_login. I don't agree with Nacin's comment there that is may be confusing. It just needs better explanation in the docblock.

I'm happy to revert the aliases for now to allow for more discussion, or even close as wontfix, but I would like to finalize the naming of is_login()/is_login_screen()/is_login_request(), as there was a reasonable demand for that function in #19898, specifically implementing a reliable way to detect the login screen.

Should we rename it to is_login()?

[azaozz]

Replying to jrf:

but I also agree with @SergeyBiryukov that that confusion already exists

Yes, at first look I also thought the aliases were a good idea and would improve things. But then started thinking what the impact will be for existing code and references to the old names, for existing contributors (that know the old names but not the new) and for new contributors that are likely to find many references to the old names but then be told to use the new names. More chances for confusion.

Updating the docs IMO is not a solution

I don't agree here. The (inline) comments and the docblocks are the best source of knowledge about the code. Even better when combined with reading through it. This is the case not only for WordPress but for any open source project. They are super useful imho, and should be expanded and enhanced as much as possible with every WP release. For example I started contributing to WP in 2006. 16 years later I still refer to the inline docs many times every day, even for code I've written (can't remember everything).

So if the existing, and especially the new contributors are not checking the docblocks they are DOING-IT-WRONG (yes, in capital letters) :)

You mention the low adoption of the new name in https://core.trac.wordpress.org/ticket/55883#comment:21.

Yes. Looking there again I'm starting to think that the alias name is actually worse than the original. That function actually replaces the shortcode string(s) with string(s) provided by the plugin/theme. In English you do not say to "apply a replacement", the proper way to say is to "do a replacement". But lets keep the discussion there.

So, what about this ?

• Revert most of the changes in this ticket, except for the adding of the alias is_super_admin_user().

Yes, adding the is_super_admin_user() alias seems like a good change. On the other hand I don't think is_super_admin() is that confusing as logically only a user can be "superadmin" (but perhaps that comes with experience). Considering all the broken references (in code and on the net), and the possible confusion they will be causing for years, I'm starting to think it would probably be better to keep the old name and update the inline docs.

Looking at usage in plugins and themes (as you suggested), is_super_admin seems to be used a total of 15,200 times (​https://wpdirectory.net/search/01GDNZ8HP5Q5G89FEX84KR3N5M and ​https://wpdirectory.net/search/01GDNZ9CRH01N8D3FWMWZHRM2K). Doing a quick search on Google returns about 8,000 results. That is a lot of references.

So I'm currently "undecided" on whether the is_super_admin_user() alias is a good idea or not. Would really appreciate another opinion :)

[azaozz]

Replying to SergeyBiryukov:

I'm happy to revert the aliases for now to allow for more discussion, or even close as wontfix

Great, thanks!

I would like to finalize the naming of is_login()/is_login_screen()/is_login_request(),
...
Should we rename it to is_login()?

My preference there is for is_login() only because it matches is_admin(). The is_login_request() is a pretty good choice too, imho. It seems a little more descriptive and doesn't seem to have any possibility of confusion. I trust your judgement, go with whatever you think is best! :)

In any case thinking that a better/expanded description in the docblock there would be great.

[SergeyBiryukov]

In 54332:

Bootstrap/Load: Revert the is_*_admin_screen() aliases for is_*_admin() function family.

After some further discussion, it is apparent that the added clarity was subjective, and the _screen suffix may not always be appropriate, e.g. in Ajax context. To address any confusion with the existing names, the documentation for these functions can be updated instead.

Additionally, the is_super_admin_user() alias for is_super_admin() is reverted as well, which may be reconsidered in the future.

Follow-up to [54259].

Props azaozz, jrf, johnbillion, manfcarlo, Clorith.
See #56400.

[SergeyBiryukov]

Replying to azaozz:

I would like to finalize the naming of is_login()/is_login_screen()/is_login_request(),
...
Should we rename it to is_login()?

My preference there is for is_login() only because it matches is_admin(). The is_login_request() is a pretty good choice too, imho. It seems a little more descriptive and doesn't seem to have any possibility of confusion. I trust your judgement, go with whatever you think is best! :)

In any case thinking that a better/expanded description in the docblock there would be great.

Thanks! Let's follow up on that in #19898.

Closing this ticket, for now at least. The is_super_admin_user() alias can be explored separately.

[azaozz]

Replying to SergeyBiryukov:

Closing this ticket, for now at least. The is_super_admin_user() alias can be explored separately.

Sounds good, thanks! I'll (try to) update the docblocks before 6.1 is released.

[SergeyBiryukov]

In 54447:

Login and Registration: Rename is_login_screen() function to is_login().

As the function can be used in a variety of contexts, the _screen suffix may not always be appropriate.

This commit aims to reduce confusion by renaming the newly added is_login_screen() function to is_login(), which better aligns with is_admin() and the related is_*_admin() function family.

While it does not save a lot of lines of code, this function aims to save developers some time that would otherwise be spent investigating the most reliable way to determine whether the current request is for the login screen.

Implementation details:

• By checking wp_login_url(), the function accounts for custom login locations set via the login_url filter.
• By checking $_SERVER['SCRIPT_NAME'] directly, instead of did_action( 'login_form_login' ) or $pagenow global, the function can work as early as possible, for example in a must-use plugin.

Follow-up to [53884].

Props azaozz.
Fixes #19898. See #56400.