I noticed that book_entity_info_alter() and hook_entity_info_alter() had different parameters ($entity_info vs $info), and that bothered me.
Digging through the hooks I noticed a couple more that were different (hook_menu_link_alter() vs user_menu_link_alter()).
And about half of the hook_form_FORM_ID_alter()s were missing the $form_id parameter.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
34.29 KB

Attached.

tim.plunkett’s picture

Okay, 34KB is pretty big, this is split into two patches, one adding $form_id to form_alters, the other with the parameter name tweaks.

Status: Needs review » Needs work

The last submitted patch, drupal-1209786-form_alter-2.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
25.61 KB
8.84 KB

Fixed the failure, should work this time.

chx’s picture

Status: Needs review » Needs work

While i like unifying variable names in the first patch and giving people a reminder that form_id is readily available in the second patch, I can't agree with taking everything by reference just because we can. In fact, it's safer not to -- dont take anything by ref unless you must.

sun’s picture

+++ b/modules/update/tests/update_test.module
@@ -27,7 +27,7 @@ function update_test_menu() {
-function update_test_system_info_alter(&$info, $file) {
+function update_test_system_info_alter(&$info, $file, $type) {

not all hook implementations have to take all parameters.

typical example is hook_theme() and hook_theme_registry_alter() -- only edge-case implementations really need the additional parameters

+++ b/modules/user/user.module
@@ -1837,29 +1837,29 @@ function user_menu_site_status_alter(&$menu_site_status, $path) {
 /**
  * Implements hook_menu_link_alter().
  */
-function user_menu_link_alter(&$link) {
+function user_menu_link_alter(&$item) {
...
 /**
  * Implements hook_translated_menu_link_alter().
  */
-function user_translated_menu_link_alter(&$link) {
+function user_translated_menu_link_alter(&$item, $map) {

Do we have some bogus docs elsewhere?

The variable for hook_menu_link_*() should be $link, not $item, because it's a link, not a router item.

19 days to next Drupal core point release.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
32.75 KB

Noted. Now it's just unifying names and adding $form_id. Nothing to break here!

tim.plunkett’s picture

Status: Needs review » Needs work

Cross-post. Will read and follow up on #6.

tim.plunkett’s picture

FileSize
33.33 KB

I fixed the bits about hook_menu_link_alter, but I agree with chx over sun in terms of including arguments like $form_id.

Paraphrasing myself from IRC, "newer developers often look to core implementations AS the documentation, core should set a good example"

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Needs work » Needs review
tim.plunkett’s picture

FileSize
61.89 KB

I still have to double check the following hooks:

hook_field_formatter_info_alter(&$info) {
hook_field_formatter_prepare_view($entity_type, $entities, $field, $instances, $langcode, &$items, $displays) {
hook_field_formatter_view($entity_type, $entity, $field, $instance, $langcode, $items, $display) {
hook_field_attach_form($entity_type, $entity, &$form, &$form_state, $langcode) {
hook_field_attach_load($entity_type, $entities, $age, $options) {
hook_field_attach_validate($entity_type, $entity, &$errors) {
hook_field_attach_submit($entity_type, $entity, $form, &$form_state) {
hook_field_attach_presave($entity_type, $entity) {
hook_field_attach_insert($entity_type, $entity) {
hook_field_attach_update($entity_type, $entity) {
hook_field_attach_preprocess_alter(&$variables, $context) {
hook_field_attach_delete($entity_type, $entity) {
hook_field_attach_delete_revision($entity_type, $entity) {
hook_field_attach_purge($entity_type, $entity, $field, $instance) {
hook_field_attach_view_alter(&$output, $context) {
hook_field_attach_prepare_translation_alter(&$entity, $context) {
hook_field_language_alter(&$display_langcode, $context) {
hook_field_available_languages_alter(&$langcodes, $context) {
hook_field_attach_create_bundle($entity_type, $bundle) {
hook_field_attach_rename_bundle($entity_type, $bundle_old, $bundle_new) {
hook_field_attach_delete_bundle($entity_type, $bundle, $instances) {
hook_field_storage_info_alter(&$info) {
hook_field_storage_details($field) {
hook_field_storage_details_alter(&$details, $field) {
hook_field_storage_load($entity_type, $entities, $age, $fields, $options) {
hook_field_storage_write($entity_type, $entity, $op, $fields) {
hook_field_storage_delete($entity_type, $entity, $fields) {
hook_field_storage_delete_revision($entity_type, $entity, $fields) {
hook_field_storage_query($query) {
hook_field_storage_create_field($field) {
hook_field_storage_delete_field($field) {
hook_field_storage_delete_instance($instance) {
hook_field_storage_pre_load($entity_type, $entities, $age, &$skip_fields, $options) {
hook_field_storage_pre_insert($entity_type, $entity, &$skip_fields) {
hook_field_storage_pre_update($entity_type, $entity, &$skip_fields) {
hook_field_info_max_weight($entity_type, $bundle, $context) {
hook_field_display_alter(&$display, $context) {
hook_field_display_ENTITY_TYPE_alter(&$display, $context) {
hook_field_extra_fields_display_alter(&$displays, $context) {
hook_field_widget_properties_ENTITY_TYPE_alter(&$widget, $context) {
hook_field_create_field($field) {
hook_field_create_instance($instance) {
hook_field_update_forbid($field, $prior_field, $has_data) {
hook_field_update_field($field, $prior_field, $has_data) {
hook_field_delete_field($field) {
hook_field_update_instance($instance, $prior_instance) {
hook_field_delete_instance($instance) {
hook_field_read_field($field) {
hook_field_read_instance($instance) {
hook_field_purge_field($field) {
hook_field_purge_instance($instance) {
hook_field_storage_purge_field($field) {
hook_field_storage_purge_field_instance($instance) {
hook_field_storage_purge($entity_type, $entity, $field, $instance) {
hook_field_access($op, $field, $entity_type, $entity, $account) {
hook_field_settings_form($field, $instance, $has_data) {
hook_field_instance_settings_form($field, $instance) {
hook_field_widget_settings_form($field, $instance) {
hook_field_formatter_settings_form($field, $instance, $view_mode, $form, &$form_state) {
hook_field_formatter_settings_summary($field, $instance, $view_mode) {
hook_file_download_access($field, $entity_type, $entity) {
hook_file_download_access_alter(&$grants, $field, $entity_type, $entity) {
hook_filter_info_alter(&$info) {
hook_filter_FILTER_settings($form, &$form_state, $filter, $format, $defaults, $filters) {
hook_filter_FILTER_prepare($text, $filter, $format, $langcode, $cache, $cache_id) {
hook_filter_FILTER_process($text, $filter, $format, $langcode, $cache, $cache_id) {
hook_filter_FILTER_tips($filter, $format, $long) {
hook_filter_format_insert($format) {
hook_filter_format_update($format) {
hook_filter_format_disable($format) {
hook_help($path, $arg) {
hook_image_effect_info_alter(&$effects) {
hook_image_style_save($style) {
hook_image_style_delete($style) {
hook_image_style_flush($style) {
hook_image_styles_alter(&$styles) {
hook_language_presave($language) {
hook_language_insert($language) {
hook_language_update($language) {
hook_language_delete($language) {
hook_menu_insert($menu) {
hook_menu_update($menu) {
hook_menu_delete($menu) {
hook_node_grants($account, $op) {
hook_node_access_records(Drupal\node\Node $node) {
hook_node_access_records_alter(&$grants, Drupal\node\Node $node) {
hook_node_grants_alter(&$grants, $account, $op) {
hook_node_predelete(Drupal\node\Node $node) {
hook_node_delete(Drupal\node\Node $node) {
hook_node_revision_delete(Drupal\node\Node $node) {
hook_node_insert(Drupal\node\Node $node) {
hook_node_load($nodes, $types) {
hook_node_access($node, $op, $account, $langcode) {
hook_node_prepare(Drupal\node\Node $node) {
hook_node_search_result(Drupal\node\Node $node, $langcode) {
hook_node_presave(Drupal\node\Node $node) {
hook_node_update(Drupal\node\Node $node) {
hook_node_update_index(Drupal\node\Node $node, $langcode) {
hook_node_validate(Drupal\node\Node $node, $form, &$form_state) {
hook_node_submit(Drupal\node\Node $node, $form, &$form_state) {
hook_node_view(Drupal\node\Node $node, $view_mode, $langcode) {
hook_node_view_alter(&$build, Drupal\node\Node $node) {
hook_node_type_insert($info) {
hook_node_type_update($info) {
hook_node_type_delete($info) {
hook_file_load($files) {
hook_file_validate(Drupal\Core\File\File $file) {
hook_file_presave(Drupal\Core\File\File $file) {
hook_file_insert(Drupal\Core\File\File $file) {
hook_file_update(Drupal\Core\File\File $file) {
hook_file_copy(Drupal\Core\File\File $file, Drupal\Core\File\File $source) {
hook_file_move(Drupal\Core\File\File $file, Drupal\Core\File\File $source) {
hook_file_predelete(Drupal\Core\File\File $file) {
hook_file_delete(Drupal\Core\File\File $file) {

Status: Needs review » Needs work

The last submitted patch, drupal-1209786-11.patch, failed testing.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned

I don't personally have time to work on this, but it's still worthwhile.

ryan.gibson’s picture

@tim.plunkett I was going to help out and do this, but I am not quite sure where the discussion left off. Maybe there was something on IRC that clarified that I missed?

YesCT’s picture

Issue tags: +Novice

Here is some clarification:

+++ b/core/modules/block/block.moduleundefined
@@ -229,7 +229,7 @@ function block_block_info() {
  * Implements hook_block_configure().
  */
-function block_block_configure($delta = 0) {
+function block_block_configure($delta = '') {

the patch contains some examples. more of this needs to be done.

For all those listed functions, find the definition, check the parameter list, and then compare it to the documentation for what the parameter list should be. Make changes to make them match.

YesCT’s picture

Status: Needs work » Needs review
Issue tags: -Novice

#11: drupal-1209786-11.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Novice

The last submitted patch, drupal-1209786-11.patch, failed testing.

underq’s picture

Status: Needs work » Needs review
FileSize
48.35 KB

I reroll patch #11 to try to pass bot testing.

Status: Needs review » Needs work

The last submitted patch, drupal-1209786-18.patch, failed testing.

underq’s picture

Status: Needs work » Needs review
FileSize
48.41 KB

I replace

use \Drupal\comment\Comment;

by

use Drupal\comment\Plugin\Core\Entity\Comment;

Status: Needs review » Needs work

The last submitted patch, drupal-1209786-20.patch, failed testing.

underq’s picture

Status: Needs work » Needs review
FileSize
47.6 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, drupal-1209786-22.patch, failed testing.

underq’s picture

Status: Needs work » Needs review
FileSize
47.58 KB

Retry with last commit :)

YesCT’s picture

underq, thanks for moving this forward!

dman’s picture

FileSize
45.36 KB

Re-roll due to the HUGE menu refactoring that just weet in #916388: Convert menu links into entities

- multiple offset changes.
core/modules/system/system.api.php implementations of hook_menu_alter() things : removed.
core/modules/user/user.module hook_menu_alter() removed.

locale_translate_file_directory was renamed into translation_path during another patch elsewhere, so that needed to be cleaned.

So, this may be a replacement for #24
Try it out testbot...

socketwench’s picture

Issue summary: View changes
Issue tags: -Novice

Novice issue cleanup.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhedstrom’s picture

Status: Needs review » Needs work

Another huge reroll will be needed. Note that some of these simpler ones may be caught by the coding standard cleanup, but the ones where hook implementations are missing parameters, or have misnamed parameters, will not be caught there.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.