Issues found using a static analysis tool

[rlerdorf]

These all look like valid, but minor, issues:

--------------------------------
File       : wp-includes/class-json.php:495
Reason     : UnknownFunction
Snippet    : class_name($var)
Line       : : new Services_JSON_Error(class_name($var).

--------------------------------
File       : wp-includes/SimplePie/Locator.php:94
Reason     : RequiredAfterOptionalParam
Snippet    : $type = SIMPLEPIE_LOCATOR_ALL
Line       : public function find($type = SIMPLEPIE_LOCATOR_ALL, &$working)

--------------------------------
File       : wp-includes/ID3/module.tag.id3v2.php:433
Reason     : TooFewArgument
Snippet    : substr($footer[5])
Line       : $id3_flags = ord(substr($footer{5}));

--------------------------------
File       : wp-includes/ID3/module.tag.id3v2.php:1586
Reason     : StatementHasNoEffect
Snippet    : $frame_ownerid == '';
Line       : $frame_ownerid == '';

--------------------------------
File       : wp-includes/ID3/module.audio.mp3.php:37
Reason     : TooManyArgument
Snippet    : $this->getOnlyMPEGaudioInfoBruteForce($this->getid3->fp, $info)
Line       : $this->getOnlyMPEGaudioInfoBruteForce($this->getid3->fp, $info);

--------------------------------
File       : wp-includes/SimplePie/Misc.php:127
Reason     : TooManyArgument
Snippet    : SimplePie_Misc::entities_decode(end($attribs[$j]), 'UTF-8')
Line       : $return[$i]['attribs'][strtolower($attribs[$j][1])]['data'] = SimplePie_Misc::entities_decode(end($attribs[$j]), 
'UTF-8');

Resolved:

--------------------------------
File       : wp-includes/class-wp-walker.php:118
Reason     : RequiredAfterOptionalParam
Snippet    : $depth = 0
Line       : function display_element( $element, &$children_elements, $max_depth, $depth=0, $args, &$output ) {

--------------------------------
File       : wp-includes/comment-template.php:1298
Reason     : RequiredAfterOptionalParam
Snippet    : $depth = 0
Line       : function display_element( $element, &$children_elements, $max_depth, $depth=0, $args, &$output ) {

--------------------------------
File       : wp-includes/deprecated.php:802
Reason     : RequiredAfterOptionalParam
Snippet    : $echo = false
Line       : function get_author_link($echo = false, $author_id, $author_nicename = '') {

--------------------------------
File       : wp-includes/deprecated.php:1709
Reason     : TooManyArgument
Snippet    : get_the_content($more_link_text, $stripteaser, $more_file)
Line       : $content = get_the_content($more_link_text, $stripteaser, $more_file);

--------------------------------
File       : wp-signup.php:493
Reason     : RequiredAfterOptionalParam
Snippet    : $user_name = ''
Line       : function confirm_blog_signup($domain, $path, $blog_title, $user_name = '', $user_email = '', $meta) {

--------------------------------
File       : wp-signup.php:493
Reason     : RequiredAfterOptionalParam
Snippet    : $user_email = ''
Line       : function confirm_blog_signup($domain, $path, $blog_title, $user_name = '', $user_email = '', $meta) {

--------------------------------
File       : wp-includes/widgets.php:76
Reason     : RequiredAfterOptionalParam
Snippet    : $id_base = false
Line       : function WP_Widget( $id_base = false, $name, $widget_options = array(), $control_options = array() ) {

--------------------------------
File       : wp-includes/widgets.php:93
Reason     : RequiredAfterOptionalParam
Snippet    : $id_base = false
Line       : function __construct( $id_base = false, $name, $widget_options = array(), $control_options = array() ) {

--------------------------------
File       : wp-includes/post.php:4789
Reason     : RequiredAfterOptionalParam
Snippet    : $deprecated = ''
Line       : function _future_post_hook( $deprecated = '', $post ) {

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:173
Reason     : RequiredAfterOptionalParam
Snippet    : $start = 0
Line       : function _rows( $taxonomy, $terms, &$children, $start = 0, $per_page = 20, &$count, $parent = 0, $level = 0 ) {

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:173
Reason     : RequiredAfterOptionalParam
Snippet    : $per_page = 20
Line       : function _rows( $taxonomy, $terms, &$children, $start = 0, $per_page = 20, &$count, $parent = 0, $level = 0 ) {

--------------------------------
File       : wp-includes/rewrite.php:92
Reason     : TooManyArgument
Snippet    : remove_action($hook, $hook, 10, 1)
Line       : remove_action($hook, $hook, 10, 1);

--------------------------------
File       : wp-admin/includes/user.php:350
Reason     : TooManyArgument
Snippet    : delete_user_setting('default_password_nag', $user_ID)
Line       : delete_user_setting('default_password_nag', $user_ID);

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:159
Reason     : TooManyArgument
Snippet    : $this->single_row($term, 0, $taxonomy)
Line       : $out .= $this->single_row( $term, 0, $taxonomy );

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:202
Reason     : TooManyArgument
Snippet    : $this->single_row($my_parent, $level - $num_parents, $taxonomy)
Line       : $output .=  "\t" . $this->single_row( $my_parent, $level - $num_parents, $taxonomy );

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:208
Reason     : TooManyArgument
Snippet    : $this->single_row($term, $level, $taxonomy)
Line       : $output .= "\t" . $this->single_row( $term, $level, $taxonomy );

--------------------------------
File       : wp-includes/media.php:2453
Reason     : UnknownFunction
Snippet    : sprint($link_fmt, $image)
Line       : $image = sprint( $link_fmt, $image );

--------------------------------
File       : wp-includes/media.php:1040
Reason     : TooManyArgument
Snippet    : wp_mediaelement_fallback($fileurl, $width, $height)
Line       : $html .= wp_mediaelement_fallback( $fileurl, $width, $height );

--------------------------------
File       : wp-includes/Text/Diff/Engine/xdiff.php:30
Reason     : UnknownFunction
Snippet    : xdiff_string_diff($from_string, $to_string, count($to_lines))
Line       : $diff = xdiff_string_diff($from_string, $to_string, count($to_lines));

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:159
Reason     : UseVoidReturn
Snippet    : $this->single_row($term, 0, $taxonomy)
Line       : $out .= $this->single_row( $term, 0, $taxonomy );

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:202
Reason     : UseVoidReturn
Snippet    : $this->single_row($my_parent, $level - $num_parents, $taxonomy)
Line       : $output .=  "\t" . $this->single_row( $my_parent, $level - $num_parents, $taxonomy );

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:208
Reason     : UseVoidReturn
Snippet    : $this->single_row($term, $level, $taxonomy)
Line       : $output .= "\t" . $this->single_row( $term, $level, $taxonomy );

--------------------------------
File       : wp-admin/includes/class-wp-terms-list-table.php:228
Reason     : UseVoidReturn
Snippet    : $this->single_row_columns($tag)
Line       : echo $this->single_row_columns( $tag );

--------------------------------
File       : wp-includes/wp-db.php:648
Reason     : TooManyArgument
Snippet    : $this->has_cap('collation', $dbh)
Line       : if ( $this->has_cap( 'collation', $dbh ) && !empty( $charset ) ) {

--------------------------------
File       : wp-includes/wp-db.php:649
Reason     : TooManyArgument
Snippet    : $this->has_cap('set_charset', $dbh)
Line       : if ( function_exists( 'mysql_set_charset' ) && $this->has_cap( 'set_charset', $dbh ) ) {

--------------------------------
File       : wp-admin/network/site-settings.php:63
Reason     : TooManyArgument
Snippet    : update_option($key, $val, false)
Line       : update_option( $key, $val, false ); // no need to refresh blog details yet

--------------------------------
File       : wp-admin/network/site-settings.php:126
Reason     : TooManyArgument
Snippet    : esc_html(maybe_unserialize($option->option_value), 'single')
Line       : $option->option_value = esc_html( maybe_unserialize( $option->option_value ), 'single' );

--------------------------------
File       : wp-admin/includes/class-wp-comments-list-table.php:318
Reason     : UseVoidReturn
Snippet    : $this->single_row_columns($comment)
Line       : echo $this->single_row_columns( $comment );

--------------------------------
File       : wp-admin/includes/class-wp-list-table.php:829
Reason     : UseVoidReturn
Snippet    : $this->single_row_columns($item)
Line       : echo $this->single_row_columns( $item );

--------------------------------
File       : wp-admin/includes/class-wp-upgrader.php:1132
Reason     : UseVoidReturn
Snippet    : screen_icon()
Line       : echo screen_icon();

--------------------------------
File       : wp-admin/includes/class-wp-posts-list-table.php:386
Reason     : UseVoidReturn
Snippet    : $this->single_row($page, $level)
Line       : echo "\t" . $this->single_row( $page, $level );

--------------------------------
File       : wp-admin/includes/class-wp-posts-list-table.php:401
Reason     : UseVoidReturn
Snippet    : $this->single_row($op, 0)
Line       : echo "\t" . $this->single_row( $op, 0 );

--------------------------------
File       : wp-admin/includes/class-wp-posts-list-table.php:447
Reason     : UseVoidReturn
Snippet    : $this->single_row($my_parent, $level - $num_parents)
Line       : echo "\t" . $this->single_row( $my_parent, $level - $num_parents );

--------------------------------
File       : wp-admin/includes/class-wp-posts-list-table.php:453
Reason     : UseVoidReturn
Snippet    : $this->single_row($page, $level)
Line       : echo "\t" . $this->single_row( $page, $level );

[rlerdorf]

Actually, ignore the xdiff_string_diff() and class_name() ones. Those slipped through my filters. But the others are valid.

[SergeyBiryukov]

In 24118:

Fix typo in get_the_post_format_image(). props rlerdorf. see #23964. see #24210.

[SergeyBiryukov]

24210.patch​ fixes the usage of WP_List_Table::single_row() in echo context.

[SergeyBiryukov]

24210.2.patch​ handles all the UseVoidReturn instances.

WP_Users_List_Table::single_row() didn't require the change, as it actually returns the output, apparently for wp_ajax_add_user(), although that function doesn't appear to be used anywhere.

WP_Terms_List_Table::_rows(), on the other hand, does not need to return the output and currently mixes string concatenation with direct echoing. The patch resolves that.

[SergeyBiryukov]

File       : wp-admin/includes/user.php:350
Reason     : TooManyArgument
Snippet    : delete_user_setting('default_password_nag', $user_ID)
Line       : delete_user_setting('default_password_nag', $user_ID);

Introduced in [14608]. Unlike get_user_option(), delete_user_setting() does not accept a second parameter, so it deletes the setting for current user.

[SergeyBiryukov]

File       : wp-includes/wp-db.php:648
Reason     : TooManyArgument
Snippet    : $this->has_cap('collation', $dbh)
Line       : if ( $this->has_cap( 'collation', $dbh ) && !empty( $charset ) ) {

Introduced in [15537]. wpdb::has_cap() only accepts one parameter.

[ocean90]

Replying to SergeyBiryukov:

Introduced in [14608]. Unlike get_user_option(), delete_user_setting() does not accept a second parameter, so it deletes the setting for current user.

All *_user_setting() functions doesn't accept an $user_id as a parameter since they referring to the current user cookie.

[SergeyBiryukov]

24210.wpdb.patch​ handles the changes in wp-db.php.

[SergeyBiryukov]

getID3() issues should be reported upstream:
​http://www.getid3.org/phpBB3/viewforum.php?f=4

[SergeyBiryukov]

File       : wp-signup.php:493
Reason     : RequiredAfterOptionalParam
Snippet    : $user_name = ''
Line       : function confirm_blog_signup($domain, $path, $blog_title, $user_name = '', $user_email = '', $meta) {

confirm_blog_signup() accepts $meta parameter, but doesn't use it anywhere.

Introduced as show_create_confirm_form() in ​mu:543, renamed to confirm_blog_signup() in ​mu:557. $user_name and $user_email became optional in ​mu:1321.

Merged with WP in [12603].

[SergeyBiryukov]

File       : wp-admin/network/site-settings.php:126
Reason     : TooManyArgument
Snippet    : esc_html(maybe_unserialize($option->option_value), 'single')
Line       : $option->option_value = esc_html( maybe_unserialize( $option->option_value > ), 'single' );

Introduced in [13106]. wp_specialchars(..., 'single') was replaced with esc_html(), which only accepts one parameter.

[SergeyBiryukov]

File       : wp-admin/network/site-settings.php:63
Reason     : TooManyArgument
Snippet    : update_option($key, $val, false)
Line       : update_option( $key, $val, false ); // no need to refresh blog details yet

Introduced in ​mu:1811. update_blog_option() was replaced with update_option(), which only accepts two parameters. The fourth parameter of update_blog_option() was deprecated in [16673].

[SergeyBiryukov]

In 24120:

Remove extraneous function parameters in the network admin. props rlerdorf. see #24210.

[SergeyBiryukov]

In 24121:

Remove extraneous function parameters in the wpdb class. props rlerdorf. see #24210.

[SergeyBiryukov]

In 24122:

Remove extraneous function parameters in wp_video_shortcode(). props rlerdorf. see #24210.

[SergeyBiryukov]

In 24123:

Remove redundant echo calls from list tables. Don't mix string concatenation with direct output. see #24210.

[SergeyBiryukov]

Replying to rlerdorf:

Actually, ignore the xdiff_string_diff() and class_name() ones. Those slipped through my filters. But the others are valid.

The class_name() one seems valid too.

Introduced in [23376]. Looks like get_class() should be used instead.

[rlerdorf]

I guess so, yes. I'll do another run tomorrow and filter a bit less. I may have filtered out other valid issues.

[dd32]

Replying to SergeyBiryukov:

In 24121:

Remove extraneous function parameters in the wpdb class. props rlerdorf. see #24210.

FWIW, This is a hold over from HyperDB I believe, as the has_cap has to be called on the individual table/database connection being tested: ​http://plugins.trac.wordpress.org/browser/hyperdb/trunk/db.php#L858

[SergeyBiryukov]

Replying to dd32:

FWIW, This is a hold over from HyperDB I believe, as the has_cap has to be called on the individual table/database connection being tested

Thanks for the clarification, makes sense.

HyperDB passes the handle to db_version(). wpdb, however, only supports a single connection, so it didn't accept the extra parameter in has_cap().

[nacin]

In 24125:

delete_user_setting() and remove_action() were getting called with too many args. props rlerdorf. see #24210.

[nacin]

In 24126:

Required arguments can't follow optional arguments.

Make required arguments optional in confirm_blog_signup().

Mark arguments as required in _future_post_hook(), the walker method display_element(), get_author_link() (deprecated), and the WP_Widget constructor.

props rlerdorf.
see #24210.

[nacin]

In 24127:

Terms list table:

• Don't call single_row() with an undeclared and unused $taxonomy argument.
• Don't define optional parameters before required parameters in the _rows() method. Make them required.
• Move empty( $terms ) check above other operations. This function was improperly returning an else case until [24123].

props rlerdorf.
see #24210.

[nacin]

In 24128:

Remove an extra argument passed to get_the_content() in the deprecated the_content_rss().

props rlerdorf.
see #24210.

[nacin]

Thanks, Rasmus, for testing your tool on WordPress trunk.

The only ones left by my count are from ID3, Services_JSON, and SimplePie.

SimplePie upstream: ​https://github.com/simplepie/simplepie/issues/293 (open) and ​https://github.com/simplepie/simplepie/pull/291 (merged)

Services_JSON upstream: ​http://pear.php.net/bugs/bug.php?id=19920

The lack of recent activity upstream for ID3 (not to mention the use of a phpbb forum as a bug tracker) scares the hell out of me: ​http://www.getid3.org/phpBB3/viewforum.php?f=4

[rlerdorf]

Is there a branch that has these fixes? I ran the static analyzer again against master and it still sees a lot of issues. The current list is here:

​https://gist.github.com/rlerdorf/5530518

Looks like there are some new ones on the list too. Probably because I have been massaging the filters.

[nacin]

Replying to rlerdorf:

Is there a branch that has these fixes? I ran the static analyzer again against master and it still sees a lot of issues. The current list is here:

​https://gist.github.com/rlerdorf/5530518

Looks like there are some new ones on the list too. Probably because I have been massaging the filters.

All of those are new, except a few pending upstream ones. Many of them have the reason UseUndeclaredVariable, which was unmentioned in the original report. The others all look like they are the result of filter improvements.

[nacin]

In 24186:

Remove the body of the deprecated links_popup_script() as currently it issues a fatal error. see #24210.

[nacin]

In 24187:

Remove manual printing of userSettings as utils.js receives this as inline script data (since 3.5). see #24210.

[nacin]

In 24188:

remove_filter() only accepts three arguments: filter, callback, and priority. An accepted args parameter is only used for adds.

props rlerdorf.
see #24210.

[nacin]

In 24189:

Fix usage of undeclared variables.

• the_weekday_date() needs the global $currentday
• ms_site_check() needs the global $current_site
• media list table does not need to check for $total_orphans
• upgrader has no $feedback variable, appears to be copypasta from other upgrade APIs
• install_themes_feature_list() has no $features variable, return array() instead of a new return type of WP_Error

see #24210.

[nacin]

Services_JSON was fixed upstream in ​http://svn.php.net/viewvc/pear/packages/Services_JSON/trunk/JSON.php?r1=330164&r2=330165&. Going to merge in that patch here as there have been some other changes since 1.0.3.

[nacin]

In 24587:

Patch Services_JSON to use the proper function name and avoid a fatal error. see #24210.

[nacin]

In 24589:

SimplePie: Merge 5e046a3 from upstream. props rlerdorf, see #24210.

​https://github.com/simplepie/simplepie/commit/5e046a3e681f82b1294c4418d8c773e9cf7c9fa0

[nacin]

In 24590:

Use correct variable when calling request_filesystem_credentials() in delete_theme(). see #24210.

[nacin]

Here's what is left: ​https://gist.github.com/nacin/5533549. With the exception of the WP filesystem ssh2 class — see #24277 — all of these are in external libraries. I'll be working to submit these upstream. Thanks again, rlerdorf.

[nacin]

In 24591:

Use correct variable in the deprecated and abandoned Snoopy HTTP client. see #24210.

See also:

• ​http://sourceforge.net/p/snoopy/bugs/60/
• ​http://sourceforge.net/p/snoopy/bugs/62/
• ​http://sourceforge.net/p/snoopy/bugs/73/
• ​http://sourceforge.net/p/snoopy/bugs/77/
• and probably more. This project is abandoned.

If any plugins are using this (instead of the WordPress HTTP API), this at least ensures that temp files are cleaned up.