Problem/Motivation
This may be fixable downstream, but since it affects two contrib modules, I'm opening upstream first in case it is a regression.
#3311365: Use PHP attributes for route discovery added attribute route discovery. The code assumes that all attributes are strings, but contrib modules JSON:API Resources and JSON:API Extras both use arrays because they impersonate the core JSON:API. My understanding is that these modules need to override core JSON:API to provide their functionality, but I don't know a lot about the details.
As a result, whenever the modules get loaded, there is a [warning] Array to string conversion AttributeRouteDiscovery.php:41 in the logs.
I noticed this because my functional tests started failing because I had failOnWarning=true, and this array to string conversion warning failed the tests.
Steps to reproduce
Enable JSON:API Extras and JSON:API Resources.
✗ ddev drush cr
[warning] Array to string conversion AttributeRouteDiscovery.php:41
[warning] Array to string conversion AttributeRouteDiscovery.php:41
One warning per module.
Proposed resolution
Not sure whose responsibility this is.
Issue fork drupal-3600697
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
Comment #2
ptmkenny commentedComment #3
longwaveComment #4
longwaveOh it's only a warning, still we should fix it for 11.4. Contrib shouldn't need to change anything here.
Comment #5
quietone commentedComment #7
kieran.cottComment #8
kieran.cottUn-assigning as per https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-...
Comment #9
kieran.cottI've also been able to reproduce this issue.
It looks like
AttributeRouteDiscovery::collectRoutes()currently assumes every namespace value is a string path,$directory .= '/Controller';.However, core already supports namespace values being either a string or an array of PSR-4 base directories.
DrupalKernel::classLoaderAddMultiplePsr4()explicitly documents and handles both shapes when registeringcontainer.namespaces.So I think what happens is that when the route builder calls the new
router.builder.attributesservice with the fullcontainer.namespacesservice, any namespace mapped to multiple paths causes the scanner to append'/Controller'to an array. That then triggers the warning and effectively checksArray/Controller,so attribute route discovery is skipped for that namespace.I'm working on a fix to
AttributeRouteDiscoverythat normalises each namespace value to one or more directories before appending/Controllerand scanning. I'll see if I can add a test covering a namespace value passed as an array of directories as well.Comment #13
kieran.cottI've opened an MR 16058 to implement the proposed fix in #9.
I've closed the erroneous MR 16057 since the issue fork should have been created from
11.4.xrather thanmain.Interested in your thoughts and suggestions.
Comment #14
guillaumeg commentedI can confirm the patch gets rid of:
[warning] Array to string conversion AttributeRouteDiscovery.php:41when clearing the Drupal cache with the contrib module jsonapi_resources enabled, for example.
Comment #15
longwaveWe need to fix this in main as well, but there is a merge conflict likely due to #3584793: Use PHP attributes for form route discovery - can you raise a separate MR against main please?
Comment #17
kieran.cottThanks @longwave - I've reopened MR 16057 and brought this up to date against
main.Comment #18
catchThe MR looks good to me - just iterating over the potential array. I was going to ask for an inline comment for the nested foreach but @longwave pointed out that the docblock change already points out we could be dealing with an array of directories, so that's fine too.
Comment #22
longwaveCommitted and pushed 97a0dbb3324 to main and a30f11b1c67 to 11.x. Thanks!
Committed and pushed 620cbeb9637 to 11.4.x. Thanks!