Problem/Motivation

\Drupal\Core\Entity\EntityTypeInterface::getKey is incorrectly typed and causes static analysis tools like PHPStan to trip up.

Steps to reproduce

Run PHPStan on max where code calls \Drupal\Core\Entity\EntityTypeInterface::getKey. PHPStan will assume the returned variable is either string or bool. Where in fact there is never TRUE.

Proposed resolution

Change typehint from string|bool to string|false.

Remaining tasks

Implement

Issue fork drupal-3269215

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dpi created an issue. See original summary.

dpi’s picture

Issue tags: +Quickfix

slucero made their first commit to this issue’s fork.

slucero’s picture

Status: Active » Needs review
dpi’s picture

Status: Needs review » Reviewed & tested by the community

LGTM!

quietone’s picture

I often forget about this one so I checked again Indicating data types in documentation and confirmed that the 'false' is allowed.

This looks fine to me. I'll wait for feedback from a committer.

xjm’s picture

Title: EntityTypeInterface::getKey is typhinted as possibly returning bool, but never returns true » EntityTypeInterface::getKey() is typhinted as possibly returning bool, but never returns true

Just fixing the title. :)

yogeshmpawar made their first commit to this issue’s fork.

dpi’s picture

Not sure why #9 was needed, as the only file in the original MR hasn't had any new changes upstream. Anyway, MR is still fine.

dpi’s picture

Title: EntityTypeInterface::getKey() is typhinted as possibly returning bool, but never returns true » EntityTypeInterface::getKey() is typehinted as possibly returning bool, but never returns true

Typo typhinted->typehinted

alexpott’s picture

@yogeshmpawar thank you for looking into this issue.

The push in comment #9 is unnecessary as merge request could still be applied to HEAD.

So, I've removed the issue credit for that push. In the future, you can get credit for updates to issues that are required. More information on the issue credit guidelines.

Thank you!

Crediting @dpi for filing the issue.

alexpott’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 9e8f4c8ed3 to 10.0.x and d99a0f626e to 9.5.x and 70fc227fa7 to 9.4.x. Thanks!

Backported to 9.4.x as this is a docs fix.

  • alexpott committed 9e8f4c8 on 10.0.x
    Issue #3269215 by slucero, dpi: EntityTypeInterface::getKey() is...

  • alexpott committed d99a0f6 on 9.5.x
    Issue #3269215 by slucero, dpi: EntityTypeInterface::getKey() is...

  • alexpott committed 70fc227 on 9.4.x
    Issue #3269215 by slucero, dpi: EntityTypeInterface::getKey() is...

Status: Fixed » Closed (fixed)

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