Add return type to hook_file_download implementations

Created on 7 January 2025, 22 days ago

Problem/Motivation

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

This is the last group of hooks in Hook classes to be updated.

Steps to reproduce

grep "fileDownload\\\\.* has no return" core/.phpstan-baseline.php

Proposed resolution

Add array|int|null to the hook documentation. Add the same to hook implementations, or a narrower set where possible.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

file system

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
  • Merge request !10830Resolve #3497796 "File download" → (Open) created by mstrelan
  • Pipeline finished with Success
    22 days ago
    Total: 616s
    #388112
  • Pipeline finished with Success
    21 days ago
    Total: 1099s
    #389146
  • 🇺🇸United States nicxvan

    Well other than hook help still :/ Maybe I will take another crack at that.

    This looks pretty straightforward.

  • 🇳🇿New Zealand quietone

    After apply the diff there are still more

    (11.x)$ grep "fileDownload\\\\.* has no return" core/.phpstan-baseline.php
            'message' => '#^Method Drupal\\\\image\\\\Hook\\\\ImageHooks\\:\\:fileDownload\\(\\) has no return type specified\\.$#',
            'message' => '#^Method Drupal\\\\system\\\\Hook\\\\SystemHooks\\:\\:fileDownload\\(\\) has no return type specified\\.$#',
    
  • 🇦🇺Australia mstrelan

    Fixed the two methods mentioned in #5, not sure how they were missed before.

    Re #4:

    Well other than hook help still

    I mean those that don't already have an issue

  • Pipeline finished with Failed
    14 days ago
    Total: 98s
    #396213
  • Pipeline finished with Success
    14 days ago
    Total: 1219s
    #396251
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • First commit to issue fork.
  • Pipeline finished with Success
    4 days ago
    Total: 690s
    #404631
  • 🇮🇳India shalini_jha

    Fixed conflict and re base this MR, re run the "grep "fileDownload\\\\.* has no return" core/.phpstan-baseline.php", and it is not showing any other list.So moving this back to NR.

  • 🇺🇸United States smustgrave

    Rebase seems fine.

  • First commit to issue fork.
  • 🇮🇳India shalini_jha

    Based on the issue and its summary, it seems that this change is related to updating the return type. But last commit is updating the comments ?

  • 🇺🇸United States nicxvan

    @oily, I agree those changes are out of scope for this type of issue.

  • 🇬🇧United Kingdom oily Greater London

    @nicxvan Yes, it suddenly occurred to me before seeing #12 and #13 so I am now mindful to keep changes strictly within scope. I did the edits using online editor so tricky to revert right now. I agree best in a follow-up. But perhaps could permit on this occasion since the MR will not affect functionality since comments? Otherwise I will revert later.

  • 🇺🇸United States nicxvan

    I wouldn't revert unless a committer requests it, just keep it in mind for the future.

Production build 0.71.5 2024