Problem/Motivation

Found while working on #3404246: [META] Fix strict type errors detected by phpstan.

This isn't doing what it looks like. In install_check_class_requirements():

if (class_exists($class_name) && class_implements($class_name, InstallRequirementsInterface::class)) { }

The second param of class_implements is a boolean to opt in or out of autoloading, and the return value is an array of interfaces the class implements. So as long as the class implements one or more interfaces, this condition is satisfied, regardless of if its the right interface. This is why strict types is so useful and why I'm pushing for the phpstan check in the meta.

Steps to reproduce

  1. Install standard profile
  2. Edit \Drupal\media\Install\Requirements\MediaRequirements to implement some other interface instead
  3. Remove MediaRequirements::getRequirements
  4. Install the media module and see the fatal error

Proposed resolution

Replace call to class_implements with is_subclass_of

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3552521

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:

Comments

mstrelan created an issue. See original summary.

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

luismagr’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Seems straight forward not sure if this needs test coverage

nicxvan’s picture

Thanks for catching this!

longwave’s picture

Version: 11.x-dev » 11.2.x-dev
Status: Reviewed & tested by the community » Fixed

Nice find. Backported down to 11.2.x as an eligible bug fix. Doesn't apply to 10.6.x as I don't think we have the new requirements API there.

Committed and pushed 3a1a6c0aed2 to 11.x and ea8f200ff51 to 11.3.x and 79a2ab03164 to 11.2.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • longwave committed 79a2ab03 on 11.2.x
    fix: #3552521 Fatal error caused by install_check_class_requirements
    
    By...

  • longwave committed ea8f200f on 11.3.x
    fix: #3552521 Fatal error caused by install_check_class_requirements
    
    By...

  • longwave committed 3a1a6c0a on 11.x
    fix: #3552521 Fatal error caused by install_check_class_requirements
    
    By...

Status: Fixed » Closed (fixed)

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