### Problem/Motivation
The `phpstan` job fails, so the pipeline is red. A clean pipeline is on the
checklist for the security advisory coverage application, so this blocks that.
At the level drupal.org's CI uses (level 0, with their standard ignore for
`Unsafe usage of new static`), the module reports **6 errors, all of them in
`src/DropupWidgetListBuilder.php`**:
At the level drupal.org's CI uses (level 0, with their standard ignore for
`Unsafe usage of new static`), the module reports **6 errors, all of them in
`src/DropupWidgetListBuilder.php`**:
| Line | Identifier |
|---|---|
| 40 | `drupal.entityStorageDirectInjection` |
| 78, 130, 207, 217 | `globalDrupalDependencyInjection.useDependencyInjection` |
| 379 | `drupal.entityListBuilderMissingCacheabilityParameter` |
They are not all the same kind of problem, and only one is a clean fix.
**Line 379 is a genuine defect.** `getDefaultOperations()` is missing the
`CacheableMetadata` parameter added in Drupal 11.3
EntityInterface $entity,
?CacheableMetadata $cacheability = NULL,
): array
**Line 40 may be unavoidable.** The rule objects to injecting
`EntityStorageInterface $storage`, but that parameter is part of
`EntityListBuilder::__construct()` in core and is passed by
`EntityListBuilder::createInstance()`. A list builder cannot drop it without
breaking its parent. If that reading is right, this one belongs in an ignore
or a baseline rather than being "fixed", and the reasoning should be recorded
where the next person will find it.
// Fetched rather than injected: a list builder is instantiated while the
// entity type is being built, and widening its constructor made the test
// runner's child process die during install.
That comment may or may not still hold — it should be re-tested rather than
trusted or dismissed. If injecting `FormBuilderInterface` and `RequestStack`
does now work, inject them. If it reproduces the install failure, the calls
stay and the constraint gets documented in the issue and in an ignore entry,
so the next contributor does not rediscover it the same way.
### Steps to reproduce
h to an issue fork and read the `phpstan` job, or locally with
`drupal/core-dev` installed, using drupal.org's configuration
(https://git.drupalcode.org/project/gitlab_templates/-/raw/main/assets/ph...):
vendor/bin/phpstan analyse -c phpstan.neon web/modules/custom/dropup
### Proposed resolution
1. Add the `CacheableMetadata` parameter to `getDefaultOperations()`.
2. Re-test whether `FormBuilderInterface` and `RequestStack` can be injected;
inject them if so, document the constraint if not.
3. Decide `drupal.entityStorageDirectInjection` — fix if there is a way that
does not fight the parent class, otherwise ignore it with a comment saying
why.
Prefer fixing over a committed baseline. Dependency-injection anti-patterns
are the kind of thing a security advisory coverage reviewer reads closely, and
a baseline that hides them reads as papering over rather than resolving.
### Remaining tasks
1. Patch.
2. Confirm the PHPStan job is green and the PHPUnit suite still passes — the
install-time failure the comment describes would show up there.
### User interface changes
None.
### API changes
`getDefaultOperations()` is `protected` and its signature changes, so a
subclass overriding it would need updating. There is no stable release yet, so
nothing published depends on it.
### Data model changes
None.
Issue fork dropup-3623207
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 #4
ibrahim tameme commentedComment #5
ibrahim tameme commented