Scrutinizer in 4.2

[wonderboymusic]

Continued efforts to improve static analysis of the codebase. Most commits will be minor changes that either remove dead code, improve comprehensibility of existing code, or clarify types specified by @param docs.

[wonderboymusic]

In 30978:

Improve some wp-admin/includes/class-wp-filesystem-*.php docs for @param.

See #30799.

[wonderboymusic]

In 30979:

Improve some docs for @param. Remove an unneeded $wpdb global import.

See #30799.

[wonderboymusic]

In 30980:

In wp_import_handle_upload():

$file was essentially getting declared/overwritten 3 times. In lieu of this, return an array containing the error immediately instead of doing a short-circuit array key assignment on error. Rename the local variable to $upload and use its properties instead of creating 3 new local vars, one of which stomped the array.

See #30799.

[wonderboymusic]

In 30981:

In image_size_input_fields():

• Declare $out as an empty array - this is not a strict PHP requirement, but is a good practice before loops
• Most of this function was tabbed twice, instead of once

See #30799.

[wonderboymusic]

In 30982:

For clarity, initialize some arrays that previously were only assigned via short circuit in loops.

See #30799.

[wonderboymusic]

In 30983:

Correct the @param docs for arguments that are truthy/falsey.

See #30799.

[wonderboymusic]

In 30984:

Comment List Tables:

• Declare $extra_items property
• Since ->get_column_info() overrides its parent's method, it doesn't need to set _column_headers, which is an undeclared property that is only used for caching the lookup after the first run in the parent class, which isn't called in the child class.
• $_args in WP_List_Table has to be protected for WP_Post_Comments_List_Table to legally access it. $_args is awkward as is and should probably be only available via a getter.

See #30799.

[wonderboymusic]

In 31077:

Pinking Shears.

See #30799.

[wonderboymusic]

In 31078:

Access Modifiers:

• In WP_Plugin_Install_List_Table, use public instead of var
• In WP_User, ->data is accessed directly on an instance if the constructor receives it: make it public
• In WP_Locale, every property is exported to a global and is already public via var, half of the properties are accessed directly already, make them all public
• In WP_Rewrite, several properties are accessed publicly in functions via the $wp_rewrite global, make those props public.
• In WP_Rewrite, the property ->comment_feed_structure was misspelled as ->comments_feed_structure

See #30799.

[wonderboymusic]

In 31079:

Perl-style comments should not be used

See #30799.

[wonderboymusic]

In 31081:

WP_Query->parse_tax_query() - for BC, this method is not marked as protected. See [28987]. It needs an access modifier, it shall have public. The comment remains.

See #30799.

[wonderboymusic]

In 31083:

In Customizer classes:

• public final function methods should be final public function - confusing Hack and aligns with PSR2
• Some methods were missing access modifiers

See #30799.

[wonderboymusic]

In 31085:

There is no need to use the final modifier inside a final class. Everything in it is final by default.

See #30799.

[wonderboymusic]

In 31086:

PHP keywords and constants "true", "false", "null" should be in lower case - there was one lingering capitalized false in _http_build_query().

See #30799.

[wonderboymusic]

In 31087:

In wp-includes/taxonomy.php:

The keyword elseif should be used instead of else if so that all control keywords look like single words.

See #30799.

[wonderboymusic]

In 31090:

The keyword elseif should be used instead of else if so that all control keywords look like single words.

This was a mess, is now standardized across the codebase, except for a few 3rd-party libs.

See #30799.

[wonderboymusic]

In 31091:

In wp-includes/plugin.php, collapse 3 else-newline-tab-if statements.

See #30799.

[wonderboymusic]

In 31092:

In wp_xmlrpc_server, remove dead code.

See #30799.

[wonderboymusic]

In 31099:

Use && instead of and in the 3 places where and was used.

See #30799.

[wonderboymusic]

In 31100:

Jump statements should not be followed by other statements (there were 5 lingering).

See #30799.

[wonderboymusic]

In 31101:

Overriding methods should do more than simply call the same method in the super class.

See #30799.

[wonderboymusic]

​https://scrutinizer-ci.com/g/staylor/WordPress/inspections

[wonderboymusic]

In 31116:

In Custom_Background and Custom_Header:

• In ->init(), don't check current_user_can() since add_theme_page() will return false immediately if the cap check fails.
• Bail if add_theme_page() returns false
• wp_check_filetype_and_ext() doesn't need a 3rd param, it already defaults to null. Passing false would fail a strict check.

See #30799.

[wonderboymusic]

In 31120:

Use PHP_SAPI constant instead of php_sapi_name() in iis7_supports_permalinks(), wp_fix_server_vars(), and wp_redirect().

See #30799.

[wonderboymusic]

In 31121:

Admin globals:

• Declare $wp_importers as a global in admin.php
• Declare $post_type, $post_type_object, and $post as globals where applicable

See #30799.

[wonderboymusic]

In 31122:

WP_Upgrader will take any "skin" that is passed to it, and set ->skin via composition. The default type of ->skin is WP_Upgrader_Skin, which doesn't have methods declared for ->bulk_header() and ->bulk_footer(). Add noop methods to WP_Upgrader_Skin.

See #30799.

[wonderboymusic]

In 31123:

In wp-admin/includes/revision.php, $post->modified is a coding error. It should be $post->post_modified.

See #30799.

[wonderboymusic]

In 31124:

Declare $wp_version, $required_php_version, and $required_mysql_version as globals in install and upgrade admin files.

See #30799.

[wonderboymusic]

In 31125:

Adding a @return annotation to constructors is generally not recommended as a constructor does not have a meaningful return value - anything that is returned is discarded.

See #30799.

[wonderboymusic]

In 31126:

Adding a @return annotation to constructors is generally not recommended as a constructor does not have a meaningful return value. Constructors do not have meaningful return values, anything that is returned from here is discarded.

See #30799.

[wonderboymusic]

In 31127:

Fix some @param docs that have chars too close them.
Add @property annotations to WP_User and WP_Post.
Remove erroneous @params from image editor class methods.
Officially add the property $_column_headers to WP_List_Table.

See #30799.

[wonderboymusic]

In 31128:

Declare $action as a global in wp-admin/comment.php.

See #30799.

[wonderboymusic]

In 31129:

Since get_theme_mod() returns false on failure, $background_image_thumb can be set to it and checked instead of calling get_background_image() 3 times in Custom_Background->admin_page().

See #30799.

[wonderboymusic]

In 31130:

get_header_image() can return false. In Custom_Image_Header->step_1(), check the value before setting the background-image portion of the style attribute. Setting the the URL to empty string will cause the current request to be set as the source of the background image.

See #30799.

[wonderboymusic]

In 31131:

In edit-form-advanced.php:

• get_permalink( $post_ID ) can return false, set it to a variable and check it
• Using the variable allows us to replace 11 separate calls to get_permalink( $post_ID ) in the file
• These notices were triggered by the potential for false to be passed to esc_url()

See #30799.

[SergeyBiryukov]

Replying to wonderboymusic:

In 31120:

Use PHP_SAPI constant instead of php_sapi_name() in iis7_supports_permalinks(), wp_fix_server_vars(), and wp_redirect().

Just to clarify, what's wrong with php_sapi_name()?

[wonderboymusic]

In 31208:

WP_Filesystem_Base should declare $errors and $options as fields for use by subclasses.

See #30799.

[wonderboymusic]

In 31209:

In WP_Filesystem_Base subclasses that set $wp_base, remove this dead code.

Unused since [8009] - "Make WP_Filesystem work with new directory constants"

There is currently no declared field and no @property annotation.

See #30799.

[wonderboymusic]

In 31210:

Add 2 noop methods to WP_List_Table: ->column_default() and ->column_cb().

WP_List_Table is essentially an abstract class. Some of its methods throw die() warnings if they aren't overridden in a child class.

These noop methods wouldn't be abstract, because they are not required in subclasses. However, WP_List_Table can call these methods in its own method, ->single_row_columns(), whether a subclass defined them or not.

See #30799.

[wonderboymusic]

In 31211:

[31210] broke Supportflow on dotorg, which declares these methods as protected. Switch to protected for the noop methods. The subclasses can make them more visible using public.

See #30799.

[wonderboymusic]

In 31212:

@param cleanup:

• get_metadata() will return literally anything, needs to be mixed
• wp() and WP_Query::__construct() no longer just take a query string
• Clarify a few others

See #30799.

[wonderboymusic]

In 31213:

By initializing this array before a loop, Scrutinizer reports 0 (zero) "Coding Style" errors.

There are plenty of other kinds of errors, but this label will be cleared out.

See #30799.

[iandunn]

re: r31211, I've ​updated SupportFlow to use public for column_default(), so you can switch back to public if you prefer that and aren't worried about other plugins.

[wonderboymusic]

In 31215:

Calling ->chown() and ->chgrp() in WP_Filesystem_FTPext produces no side-effects. Remove the calls. ->chgrp() is defined in the super class, and does nothing, can be removed from the child class.

See #30799.

[wonderboymusic]

In 31216:

Calling ->chown() and ->chgrp() in WP_Filesystem_ftpsockets produces no side-effects. Remove the calls. ->chgrp() is defined in the super class, and does nothing, can be removed from the child class.

See #30799.

[wonderboymusic]

In 31217:

Remove unused local vars in delete_plugins(), delete_theme(), WP_Date_Query->validate_date_values(), global_terms(), and WP_Text_Diff_Renderer_Table->_changed().

This will clear out the "Unused Code" label in the next Scrutinizer report.

See #30799.

[wonderboymusic]

In 31219:

Fix some erroneous @param annotations.

See #30799.

[wonderboymusic]

In 31220:

Fix some internal types that are passed to functions to avoid changing the acceptable types passed as arguments to those functions:

• In WP_Importer->is_user_over_quota(), the default value for the first argument for upload_is_user_over_quota() is true. Don't bother passing 1.
• When calling submit_button() with no $name, pass empty string instead of false.
• The default value for the 2nd argument to get_edit_post_link() is 'display'. Because PHP is PHP, passing true is the same as passing 'display' or nothing. Don't bother passing true.
• In WP_User_Meta_Session_Tokens::drop_sessions(), pass 0 instead of false to delete_metadata() as the value for $object_id, which expects an int.

See #30799.

[wonderboymusic]

In 31221:

wp_set_object_terms() takes $taxonomy as a string. Update @param.
See #30799.

[DrewAPicture]

Related: #31308

[SergeyBiryukov]

Related: #31390

[wonderboymusic]

In 31554:

Don't call the size function count() as part of a test condition in loops. Compute the size beforehand, and not on each iteration.

Scrutinizer added a Performance label: these are the only violations.

See #30799.

[wonderboymusic]

In 31555:

Cleanup the @property annotations for WP_User.

See #30799.

[wonderboymusic]

In 31672:

Use $this-> instead of parent:: when calling tablenav() in WP_Theme_Install_List_Table.

See #30799.

[wonderboymusic]

In 31678:

Avoid function calls on each iteration of a for loop.

See #30799.

[wonderboymusic]

In 31679:

Empty return statements are unnecessary at the end of functions.

See #30799.

[wonderboymusic]

In 31681:

There are a few functions that have the ability to return false instead of a string, so the return value should be checked before being passed to functions that expect string.

These are trivial, but they clear out some Scrutinizer issues.

See #30799.

[wonderboymusic]

In 31682:

Declare multisite as a field for WP_Object_Cache.

See #30799.

[wonderboymusic]

In 31683:

self should be used for accessing local static members.

See #30799.

[wonderboymusic]

In 31684:

Remove the PHP4 constructor from WP_Widget.

See #30799.

[nacin]

Replying to wonderboymusic:

In 31684:

Remove the PHP4 constructor from WP_Widget.

See #30799.

This is guaranteed to break plugins.

[wonderboymusic]

will reverse

[wonderboymusic]

In 31685:

[31684] has BC issues, revert it.

Props nacin.
See #30799.

[DrewAPicture]

Closing as fixed for 4.2 with @wonderboymusic's blessing. Good improvements all.

[dd32]

In 39687:

Upgrade: Fix the installation of TwentySeventeen upon upgrade from an early version.

This reverts part of [31124] which incorrectly caused $old_wp_version to equal the version of WordPress being upgraded to due to global variable access changes.

See #38551, #30799.
Fixes #39138 for trunk.