Add array return to all *_info hook implementations

Created on 21 November 2024, 3 months ago

Problem/Motivation

See 📌 [META] Add return types to hook implementations Active

All info hooks should return array, except hook_config_translation_info which takes the $info param as a reference.

Steps to reproduce

Proposed resolution

Add array return to the following hook implementations:

  • hook_entity_bundle_info
  • hook_entity_base_field_info
  • hook_entity_bundle_field_info
  • hook_entity_field_storage_info
  • hook_entity_extra_field_info
  • hook_hook_info
  • hook_updater_info
  • hook_filetransfer_info
  • hook_token_info
  • hook_language_types_info

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

other

Created by

🇦🇺Australia mstrelan

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mstrelan
  • Pipeline finished with Canceled
    3 months ago
    Total: 316s
    #345448
  • Pipeline finished with Success
    3 months ago
    Total: 866s
    #345450
  • 🇺🇸United States nicxvan

    Looks good! Baseline looks right too.

  • 🇳🇿New Zealand quietone

    I left some questions in the MR. Also, why are these not changed?

    $ grep -r 'function.*hook_info(' core | grep -v array
    core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test/module_handler_test.module:function module_handler_test_hook_info() {
    core/tests/Drupal/Tests/Core/GroupIncludesTestTrait.php:function test_module_hook_info() {
    core/lib/Drupal/Core/Hook/HookCollectorPass.php:   * A list of functions implementing hook_hook_info().
    core/modules/views/views.module:function views_hook_info() {
    core/modules/system/tests/modules/module_test/module_test.module:function module_test_hook_info() {
    core/modules/system/system.module:function system_hook_info() {
    
  • 🇦🇺Australia mstrelan

    Responded to some of the MR feedback, need further advice. Will look at the entity.api.php feedback too, it wasn't loading in the MR though.

    Re #5:

    • The *_hook_infohooks were missed because my script was looking for a #[Hook] attribute in Hook classes. Could possibly be skipped due to 📌 Deprecate hook_hook_info() Active
    • test_module_hook_info is actually in a heredoc string for writing a .module in a test, but I guess it could be included too
    • HookCollectorPass is just a comment talking about info hooks
  • Pipeline finished with Canceled
    3 months ago
    Total: 477s
    #348987
  • 🇦🇺Australia mstrelan

    Addressed most feedback, 2 outstanding items in the MR for re-review.

  • Pipeline finished with Success
    3 months ago
    #348992
  • 🇦🇺Australia mstrelan

    Needs rebase

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    #353052
  • 🇮🇳India akulsaxena

    Hi @mstrelan

    I resolved the Merge conflict in Baseline and Rebased the branch to the latest.
    But the PHPSTAN pipeline shows some error in Baseline.
    Can you please look into it

  • 🇦🇺Australia mstrelan

    Best not to manually resolve conflicts in the baseline, just regenerate it.

  • 🇮🇳India akulsaxena

    I am a bit occupied with something right now, I rebased the branch but there were some PHPUnit errors, can someone please regenerate the baseline.
    Thanks

  • 🇦🇺Australia mstrelan

    Rebased with these steps:

    • git rebase -i 11.x
    • Drop the baseline commit
    • Regenerate baseline with ./vendor/bin/phpstan --configuration=core/phpstan.neon.dist --generate-baseline=core/.phpstan-baseline.php
    • git add core/.phpstan-baseline.php
    • Commit and force push
  • 🇺🇸United States nicxvan

    Those steps good but you mean --force-with-lease right?

  • Pipeline finished with Success
    3 months ago
    Total: 2446s
    #353768
  • Pipeline finished with Failed
    3 months ago
    Total: 586s
    #356085
  • 🇮🇳India akulsaxena

    Pipeline passed with all no errors/warnings

  • 🇺🇸United States smustgrave

    @akulsaxena unless the issue doesn't apply or the review bot kicks back a rebase is not needed.

    Appears to be 2 open threads so leaving in review. As far as the workspace thread, even though we know workspaces is being updated think we have to make sure nothing bad is introduced here as we don't fully know when those overhaul tickets will land.

  • 🇺🇸United States smustgrave

    Okay this has sat for a bit.

    The first thread appears to have an open discussion.

    For the workspace one should we postpone till workspace is refactored?

  • 🇦🇺Australia mstrelan

    Rebased the MR.

    The first thread appears to have an open discussion.

    I have refactored this as requested.

    For the workspace one should we postpone till workspace is refactored?

    Don't think so, there is no harm in leaving it in.

  • Pipeline finished with Success
    2 months ago
    Total: 1200s
    #373333
  • 🇺🇸United States smustgrave

    Now it actually does need a rebase.

  • First commit to issue fork.
  • Pipeline finished with Success
    about 2 months ago
    Total: 864s
    #378829
  • 🇮🇳India shalini_jha

    I have rebased the MR, resolved the conflict, and followed the steps outlined in #14. I have also verified the PHP Stan baseline. Since the pipeline is now green, Checked the threads , but nothing open so moving this back to 'Needs Review'.

  • 🇺🇸United States smustgrave

    Needs a rebase.

  • 🇮🇳India shalini_jha

    I am working on it

  • Pipeline finished with Success
    about 2 months ago
    Total: 878s
    #386905
  • 🇮🇳India shalini_jha

    Rebase & fixed conflict,moving this NR.

  • 🇺🇸United States smustgrave

    This rebase seems to hold for now.

  • 🇳🇿New Zealand quietone

    Sorry, needs another rebase.

  • Pipeline finished with Success
    about 1 month ago
    Total: 411s
    #394343
  • 🇮🇳India shalini_jha

    Rebased this without encountering any conflicts

  • 🇺🇸United States smustgrave

    Sorry still needs a rebase

    If you are another contributor eager to jump in, please allow the previous poster at 8 hours to respond to feedback first, so they have the opportunity to finish what they started!

  • 🇮🇳India shalini_jha

    Rebased & fixed conflicts :)

  • Pipeline finished with Success
    about 1 month ago
    Total: 759s
    #397383
  • 🇮🇳India akulsaxena

    The rebase seems to hold for now
    Can be moved to RTBC now, i worked on this so cant move it to RTBC, will wait for someone else to RTBC this.

  • Status changed to RTBC about 1 month ago
  • 🇺🇸United States smustgrave

    Thanks @shalini_jha!

  • 🇳🇿New Zealand quietone

    Before applying the diff these are the info hooks with a return type. The void agrees with the Issue Summary.
    (11.x)$ git grep -A1 "#\[Hook.*_info')" | grep \):
    core/modules/config_translation/src/Hook/ConfigTranslationHooks.php- public function configTranslationInfo(&$info): void {
    core/modules/file/src/Hook/TokenHooks.php- public function tokenInfo(): array {
    core/modules/system/tests/modules/plugin_test/src/Hook/PluginTestHooks.php- public function testPluginInfo(): array {
    core/modules/workspaces/src/Hook/EntityTypeInfo.php- public function entityBaseFieldInfo(EntityTypeInterface $entity_type): array {

    After applying the diff and search for info hooks with an array return, there is just the config one, as expected.
    (11.x)$ git grep -A1 "#\[Hook.*_info')" | grep function | grep -v ": array"
    core/modules/config_translation/src/Hook/ConfigTranslationHooks.php- public function configTranslationInfo(&$info): void {

  • 🇳🇿New Zealand quietone

    Committed 873f84f and pushed to 11.x. Thanks!

    • quietone committed 873f84fc on 11.x
      Issue #3488841 by mstrelan, shalini_jha, akulsaxena, smustgrave, nicxvan...
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • 🇺🇸United States nicxvan
Production build 0.71.5 2024