When theme_item_list is encountered it should be upgraded to use theme() helper function

[izmeez]

When coder_upgrade encounters Drupal 7 code with theme_item_list it leaves it unchanged. For example:

   return theme_item_list(
    array(
    'items' => $items,
    'title' => '',
    'type' => 'ul',
    'attributes' => array(),
    )
  );
}

This results in a warning:

Warning: Undefined array key "wrapper_attributes" in theme_item_list() (line 2434 of core\includes\theme.inc).

This is because in Backdrop new parameters have been added to theme_item_list().

The proper upgrade is to use the helper function theme() and changing the code to:

  return theme('item_list',
    array(
      'items' => $items,
      'title' => '',
      'type' => 'ul',
      'attributes' => array(),
    )
  );
}

Can coder_upgrade make this change or provide a note to alert what needs to be changed?

[argiepiano]

Interesting idea. The use of theme_item_list() is actually bad coding in the original module - so the question is, should coder_upgrade also serve to "evaluate" bad coding in the original module? This is really a pandora's box. For example, why limit this to theme_item_list, rather than catching ALL theme_X() calls, which are also incorrect in most instances? And, is there any instance when calling the theming function directly may b desirable?

[bugfolder]

As usual, @argiepiano beat me to responding (I was going to say more or less the same thing).

And I did think of an instance when calling the theming function directly may be desirable: I might want to override the theming function in my own theme, but use it internally, something like this, where in my theme I want to add a wrapper div but otherwise use the core theme function:

function mytheme_item_list($variables) {
  return '<div class="my-wrapper">' . theme_item_list($variables) . '</div>';
}

If we are going to try to catch all uses of core theme functions, there's a lot to have to check.

[oadaeh]

If you were to decide to pursue this, maybe what could be done when finding them, rather than attempting to fix them, is to add a todo comment before the function mentioning the issue and maybe adding a URL to the documentation, and then let the person doing the upgrade decide if it should be changed or not.