FlagConfirmForm and UnflagConfirmForm declare protected readonly RendererInterface $renderer in their constructors. On PHP < 8.4, DependencySerializationTrait::__wakeup() cannot initialize a readonly property declared in a child class, so PHPStan reports dependencySerializationTraitProperty.unsupportedReadOnlyProperty (src/Form/FlagConfirmForm.php:27, src/Form/UnflagConfirmForm.php:27).

Two functional tests also call filter_default_format(), deprecated in Drupal 11.4.0 and removed in 13.0.0 (tests/src/Functional/FlaggingViewTest.php:41, tests/src/Functional/LinkOutputLocationTest.php:64).

Steps to reproduce: run the PHPStan job on 5.1.x.

Proposed resolution

  • Drop readonly from the promoted $renderer property in both forms.
  • Replace filter_default_format() with FilterFormatRepositoryInterface::getDefaultFormat() in both tests, keeping core compatibility in mind.

Remaining tasks

  • Apply the changes and confirm the PHPStan job no longer reports these four errors.

Out of scope (separate follow-up, may affect BC with older core): the missing CacheableMetadata parameter on hook_entity_operation in flag.module:137 and FlagHooks.php:554, and the deprecated trustData() calls in FlagHooks.php:607 and :621.

Issue fork flag-3630532

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

andres alvarez created an issue. See original summary.

andres alvarez opened merge request !233

andres alvarez’s picture

andres alvarez commented

The MR scope grew: besides the readonly constructor properties and filter_default_format() (first commit), the second commit makes PHPStan completely clean. Since core 11.3 hook_entity_operation receives a CacheableMetadata $cacheability argument (CR 3533080); it is added as an optional parameter (?CacheableMetadata $cacheability = NULL) on FlagHooks::entityOperation() and the procedural wrapper, so core 10.3 through 11.2, which only pass the entity, keep working. When provided, the flag is added as a cacheable dependency, since the enable/disable operations depend on its status. trustData()->save() is also replaced by save() when creating the flag/unflag actions: trustData() is deprecated in 11.4 and removed in 13.0 with no replacement (CR 3348180), and it only skipped schema casting as a performance shortcut.

PHPStan with the module's phpstan.neon goes from 4 errors to 0 (the pipeline phpstan job as well). Kernel 45/45, Functional 27/27 and FunctionalJavascript 12/12 on core 11.4.6; a new Kernel test checks that cacheability bubbles through core's real list builder, and it fails without the change. 10.3 compatibility was verified by reading core code only, not by running it.

andres alvarez’s picture

andres alvarez commented
Status: Active » Needs review

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

ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Fixed

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.