Docs: Undocumented @global instances

[ravipatel]

@global wpdb not define on line no 230.

display_setup_form() on define but not call outside of the function.

[ravipatel]

codeing standard undefine varible.

[davidbaumwald]

@ravipatel Can you refresh these patches against the src? The patch does not include the file path and is more difficult to apply. For instance, install.php should be src/wp-admin/install.php.

Here's a Handbook page about creating the patch from src/ ​https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/

[ravipatel]

Added with full path for undefined varibale

[ravipatel]

Added file patch another file - undefined variable

[ravipatel]

Combile all chnages + $post_id missing to define on comment file.

[ravipatel]

$comment & $action not defined edit-form-comment.php

[davidbaumwald]

@ravipatel Looking at these patches more in depth, it looks like most of these added global vars are not necessary, as they are already defined earlier in execution. For example, this patch is not necessary. edit-form-comment.php is an included template, and both $action and $comment are defined in the parent file src/wp-admin/comment.php. Same goes for the $typenow and $wpdb in src/wp-admin/edit.php.

I'm adding the close keyword because I think this ticket is not needed. I'll leave it open for a couple of days to see if anyone think s this does deserve to be a standalone ticket. Please ensure that these variables are indeed undefined at runtime, if they aren't I suggest adding to this ticket instead.

[ravipatel]

@davidbaumwald
OK, but applied some changes which is right for a declaration for variables as per PHP code doc format.

[hellofromTonya]

The changes in the install.php file are from Version 4.2 rather than 5.6 (trunk), ie per Ticket #30799. Changing the Version to 4.2.

[SergeyBiryukov]

Hi there, thanks for the patches!

I have checked the suggested changes and I agree with comment:4 that they don't seem to be necessary. These globals are not declared or documented in these particular places because they are all declared and documented elsewhere.

For example, [45233] / #46602 removed the $wpdb declaration from wp-admin/install.php, because the variable is already available there, it was previously declared in wp-load.php. As noted in comment:2:ticket:46602:

Per the ​documentation standards, the @global tag is meant to list PHP globals used within functions or methods.

Looking at other files in wp-admin, it seems like we don't generally document $wpdb in code that itself is in the global namespace. The standards don't say anything about it either.

The same goes for other variables in the patch too, they may not be explicitly declared in these specific instances but they are defined. If a static analysis tool flags them as undefined, that would be a false positive.

On second thought, if the goal here is to improve developer experience, I guess there might be some benefit in declaring these globals explicitly. I think they should be declared closer to where they are used, so that the context is immediately clear, and there is no need to redefine them. See 51439.diff​, moving to 6.0 for consideration.

As for adding the @var keyword in wp-admin/credits.php and wp-admin/includes/ajax-actions.php: WordPress generally documents class properties or constants like that, but not local variables, so I don't think this is needed.

As for declaring the $wp_version global in wp-admin/includes/class-core-upgrader.php: the current code specifically imports it as a local variable (we do that in a few other places as well), so I think there is no need to declare it as a global. Doing that might have unintended side effects, like changing the global value instead of the local one.

[costdev]

Per the discussion in the bug scrub, I'm moving this ticket to the 6.1 milestone for now. If a committer finds that this patch is ready for commit, feel free to pull it back into the 6.0 milestone for inclusion in the release.

[SergeyBiryukov]

In 53450:

Docs: Explicitly declare some globals for clarity.

This aims to improve developer experience by making it clear that these variables are defined elsewhere.

Props ravipatel, davidbaumwald, hellofromTonya, costdev, SergeyBiryukov.
Fixes #51439.