Generated with the help of an LLM.

Problem/Motivation

DrupalDebugClassLoader fails when Symfony stores NULL for a generic return type. Drupal reads this entry as an array and passes NULL to getExtensionName(), which requires a string.

This occurs for Drupal\Core\Template\TwigEnvironment::getRuntime() during test setup.

Steps to reproduce

  1. Use Drupal main-dev with symfony/error-handler 8.1.10.
  2. Enable the debug class loader in the PHPUnit configuration.
  3. Run a kernel test that starts Drupal.

The test stops with this error:

TypeError: Drupal\TestTools\ErrorHandler\DrupalDebugClassLoader::getExtensionName(): Argument #1 ($class) must be of type string, null given

See the failed CI job.

Proposed resolution

Skip NULL return-type entries before reading their array values.

Remaining tasks

  • Add the fix and a regression test.
  • Make sure that valid return-type warnings still occur.

Issue fork drupal-3634044

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:

About issue forks

Comments

joelpittet created an issue. See original summary.

joelpittet opened merge request !17619

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Title: DrupalDebugClassLoader throws TypeError when Symfony records NULL metadata for generic return types » symfony/error-handler 8.1.10 - DrupalDebugClassLoader throws TypeError when Symfony records NULL metadata for generic return types
Status: Active » Needs review

symfony/error-handler 8.1.10 can store NULL for generic PHPDoc return types. Drupal destructures that entry and passes NULL to getExtensionName(), causing a TypeError.

The fix skips NULL entries before destructuring. The string parameter is unchanged.

Locally (on my views_ajax_history issue branch) , I temporarily used symfony/error-handler 8.1.10 package files with the existing DDEV+PHPUnit setup. The regression failed with the reported TypeError, passed with the fix, and failed again after removing only the guard.

The test-only job here passes. Its script retains changes under core/tests/, including this fix. It also uses symfony/error-handler 8.1.5 without the updated-depencies pipeline. This job does not demonstrate failure without the fix. https://git.drupalcode.org/issue/drupal-3634044/-/jobs/13208327 😖

The updated-dependencies pipeline uses symfony/error-handler 8.1.10 https://git.drupalcode.org/issue/drupal-3634044/-/pipelines/1022319. Its class-loader tests pass. Functional jobs 4/8 and 5/8 fail. I have not investigated their causes. 🙈

Generated with the help of an LLM 🧮. Read and understood by a human 🐒.

japerry’s picture

japerry
Location KOMK
commented

Oh man, I was just about to post the same issue! You can see it failing here on Captcha: https://git.drupalcode.org/project/captcha/-/jobs/13208931

I tested Joel's MR !17619 against captcha on 12.0-dev with symfony/error-handler 8.1.10 and `SYMFONY_PATCH_TYPE_DECLARATIONS=deprecations=1`, as GitLab CI runs it:

  • Without the MR: every test errors with `DrupalDebugClassLoader::getExtensionName(): Argument #1 ($class) must be of type string, null given`.
  • With the MR: the Kernel suite passes, and CaptchaTest passes (12 tests).

The NULL entries come from symfony/symfony#67238 ("Fix DebugClassLoader suggesting template types as native return types"). Symfony merged it from the 6.4 branch up, but has only released it in 8.1.10 so far. It doesn't affect 11.x, because DrupalDebugClassLoader only exists on main.

The fix and the regression fixture look right to me. Skipping a NULL entry matches what Symfony intends: a template has no native return type to suggest. Valid cross-module warnings still fire, and the existing `testCrossModuleReturnTypeDeprecation()` cases are unchanged.

+1 to RTBC!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Reproduced the issue by updating symfony/error-handler and running the test-only version of this patch. The patch fixes it as expected. Also confirmed the issue by reading the upstream diff at https://github.com/symfony/symfony/pull/67238/changes

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

xjm’s picture

xjm
she/her
Primary language English
commented

The updated deps job is still failing, but confirmed it is just this deprecation:
Since twig/twig 3.29: Not passing a value for the "default_admin_is_sticky" argument of macro "table_header" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "core/themes/default_admin/templates/views/views-view-table.html.twig" at line 109).

  • xjm committed 6a60affd on 12.0.x 
    fix: #3634044 symfony/error-handler 8.1.10 - DrupalDebugClassLoader...

  • xjm committed 555acb12 on main 
    fix: #3634044 symfony/error-handler 8.1.10 - DrupalDebugClassLoader...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed to main and 12.0.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.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@xjm RE #7 Yes, that was the 🙈 I was referring to in #4, there was more than meets the eye... sorry wasn't explicit but thanks for being explicit. Trying not to scope creep this/myself.