phpstan-drupal icon indicating copy to clipboard operation
phpstan-drupal copied to clipboard

Fix supertype condition in LoadIncludes.

Open donquixote opened this issue 2 years ago • 5 comments

Direction is wrong.

As a test case you can use something like this:

class XClass {
  public function foo(ModuleHandler $a, ModuleHandlerInterface $b, object $c, mixed $d) {
    $a->loadInclude('non_existing_module', 'inc', 'something');
    $b->loadInclude('non_existing_module', 'inc', 'something');
    $c->loadInclude('non_existing_module', 'inc', 'something');
    $d->loadInclude('non_existing_module', 'inc', 'something');
  }
}

I have not looked deeply into your package so I just post this code here instead of actually writing a test :)

donquixote avatar Jul 10 '23 19:07 donquixote

  public function testLoadInclude() {
    $module_handler = $this->getModuleHandler();
    // Include exists.
    $this->assertEquals(__DIR__ . '/modules/module_handler_test/hook_include.inc', $module_handler->loadInclude('module_handler_test', 'inc', 'hook_include'));
    $this->assertTrue(function_exists('module_handler_test_hook_include'));

The module is in core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test/hook_include.inc. I'm guessing modules in this location aren't discovered by the test extension discovery.

That's because it's a mocked module handler:

  protected function getModuleHandler() {
    $module_handler = new ModuleHandler($this->root, [
      'module_handler_test' => [
        'type' => 'module',
        'pathname' => 'core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test/module_handler_test.info.yml',
        'filename' => 'module_handler_test.module',
      ],
    ], $this->cacheBackend);
    return $module_handler;
  }

So I guess this PR does fix a bug in its logic :)

mglaman avatar Jul 13 '23 13:07 mglaman

    // Register entity_test text field.
    $this->container->get('module_handler')->loadInclude('entity_test', 'install');

In core/tests/Drupal/KernelTests/Core/Field/FieldAccessTest.php seems like a weird failure:

 ------ -------------------------------------------------------------- 
  Line   core/tests/Drupal/KernelTests/Core/Field/FieldAccessTest.php  
 ------ -------------------------------------------------------------- 
  60     A file could not be loaded from                               
         Drupal\Core\Extension\ModuleHandlerInterface::loadInclude     
 ------ -------------------------------------------------------------- 

mglaman avatar Jul 13 '23 14:07 mglaman

That's because it's a mocked module handler:

The list of "known modules" for this check has nothing to do with the module handler instance used in that test. There is a registry of modules somewhere.. And it seems none of the test modules is included.

donquixote avatar Jul 13 '23 20:07 donquixote

I didn't forget about this. I just am waiting for time to figure out the "regression" for that unit test. The module inside of core/tests/Drupal/Tests/Core/Extension/modules/module_handler_test (source) isn't discoverable even with test discovery enabled.

mglaman avatar Jul 25 '23 14:07 mglaman

Maybe you have to disable the check for tests.

donquixote avatar Jul 25 '23 14:07 donquixote