[policy, no patch] All new Drupal 10 code should use appropriate type hints wherever possible

Created on 25 June 2020, over 4 years ago
Updated 17 June 2023, over 1 year ago

Problem/Motivation

As per #1158720-69: [policy, no patch] Add parameter type hinting to function declaration coding standards β†’ this issue proposes that all new Drupal 9 code should use scalar typehints and return typehints since our minimum PHP version is 7.3.

This is only for new code to avoid BC implications. We will work out how to change existing code (probably in Drupal 10) in 🌱 [Meta] Implement strict typing in existing code Active .

The related Coding Standards issue explains that there is an existing policy for using strict type hints. See comment #17 β†’ for the details.

Proposed resolution

New methods and functions added in Drupal 9 should use scalar typehints and return typehints.

Example (pseudo-code):

public function checkAccess(string $permission): AccessResult {
  $access_result = AccessResult::allowedIfHasPermission($this->user, $permission);
  return $access_result;
}

Post something on groups.drupal.org/core

Remaining tasks

Add examples to Parameter and return typehinting β†’

Draft and publish a change record that links that coding standards section.

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

N/a

🌱 Plan
Status

Needs review

Version

10.0 ✨

Component
BaseΒ  β†’

Last updated 1 day ago

Created by

πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States moshe weitzman Boston, MA

    Do we really need a change record for an MR that is policy and has no code change at all? This has been sitting near the finish line for a year.

  • Status changed to RTBC 26 days ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    In the list of reasons β†’ for adding a change record I don't see anything that covers this. The closest is "For Coding standard issues when the rule requires significant code changes" but that would apply to existing code, not new code. Based on that I am removing the tag.

    We have been doing this for some time and the Coding Standard docs have changed. Unless I am missing something there is nothing more to do in this issue.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Given this has been sitting here for two years and in practice we have been doing it already, a change record seems pointless now.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024