Including api.php in DrupalAutoloader throws PHP redeclare error

[peterwcm]

Including api.php in DrupalAutoloader throws PHP redeclare error

[eiriksm]

Just to note, this is the case for many other contrib modules. Usually because they somewhat do not follow conventions in the api.php file, but even still. It's a documentation piece of a file, so it's kind of annoying.

Here is just a couple I experienced this morning:

Fatal error: Cannot redeclare elasticsearch_helper_aws_elasticsearch_helper_client_builder_alter() (previously declared in /__w/nymedia_commerce_profile/nymedia_commerce_profile/web/modules/contrib/elasticsearch_helper/elasticsearch_helper_aws/elasticsearch_helper_aws.module:17) in /__w/nymedia_commerce_profile/nymedia_commerce_profile/web/modules/contrib/elasticsearch_helper/elasticsearch_helper.api.php on line 19

Fatal error: Cannot redeclare ckeditor_media_embed_ckeditor_media_embed_object_alter() (previously declared in /__w/store/store/drupal/modules/contrib/ckeditor_media_embed/ckeditor_media_embed.module:136) in /__w/store/store/drupal/modules/contrib/ckeditor_media_embed/ckeditor_media_embed.api.php on line 21

[eiriksm]

Added a PR to be able to opt out of loading api.php files here: #641

[mglaman]

Sorry about this. I didn't think folks had that broken of .api.php files. I like the idea of opting out.

I'm going to rework your PR @eiriksm. I want to establish a bleeding-edge concept like in PHPStan. New rules are added as off by default, but including that file will enable them.

[eiriksm]

Wow you are the most kind, knowledgeful maintainer ever ❤️ I have a feeling you more or less posted this just after waking up, so taking the time here to appreciate your work and a new release breaking stuff just highlights the impact of your work. Thank you, Matt!

[mglaman]

Huh. The file should be loaded with a catch for even fatal errors.

That module needs to be fixed.

But also see if we can harden the error catching here.

[peterwcm]

Fatal errors cannot be caught as far as I know so I don't think you can work around that easily. While we can 'fix' the contrib module by adding a patch to have that hook commented, it might not make sense to the contrib maintainers.

From my understanding, api.php file is more of a documentation piece for custom hooks or related hook usage that's related to the module. They are just code examples and it's not meant to be included by others so logically it's not an issue from their end. What do you think?

[eiriksm]

To add to this: eiriksm#2 is a PR I created against my own fork, which demonstrates the bug. A redeclaration in an api.php file does not cause any issues in running a drupal site, but it totally crashes phpstan here ✌️

[eiriksm]

Tried a couple of hacks to catch such a fatal error, but came up short. Seems like we should either

• Revert the commit
• or make it either opt-in or possible to opt out