This issue tracks the changes needed to make the GitLab CI pipeline pass on a clean checkout of the 1.x branch. It covers three areas: PHPStan static analysis, PHPCS coding standards, and the unit test suite.

PHPStan (level 4)

The main source of errors was unsafe new static() in plugin create() factory methods. PHPStan flags this because new static() in a non-final class returns the wrong type for subclasses. The fix is to give each concrete leaf class its own create() using new self(), and to add ): static or ): self return types throughout.

  • Remove create() from abstract base classes (SitePluginBase, SiteWithContentBase, ContentSiteActionBase); add concrete create() with new self() to every leaf class
  • Add missing ): static / ): self return types to all remaining create() methods for consistency
  • Narrow applies() return type from ?bool to bool in final example plugins (valid covariant narrowing; PHPStan can now prove null is never returned)
  • Move hidden-plugin removal from getDefinitions() into alterDefinitions(), which is the correct location for definition mutations and makes the behaviour unit-testable in isolation
  • PHPStan baseline reduced from 477 original errors to 11 genuinely unfixable entries

Unit test suite

Several tests were broken or could not exercise the code they were supposed to test:

  • PHPUnit 11 compatibility: withConsecutive() was removed in PHPUnit 11. Replaced with willReturnCallback using a call counter.
  • By-reference reflection call: ReflectionMethod::invoke($sut, $definitions) does not pass by reference. Fixed by switching to invokeArgs($sut, [&$definitions]).
  • URL generator test mocks: The test was calling getSetting() directly on the SiteProxyInterface mock, but the production code calls getSite()->getSetting(). Fixed by wiring getSite() to return a SiteInterface mock. Also fixed: Request::create('/') uses host localhost, not the intended test host — requests now use Request::create('https://host.example/'). The setContext mock was a no-op so context restoration could not be verified — fixed by making the callback actually propagate the host change.
  • SitesServiceTest: buildRouteMatchKey calls getRawParameters()->all() and getParameters()->all(), but the route match mocks did not stub those methods. Added empty parameter bag stubs.
  • SitesServiceRouteMatchCacheTest: A scalar route parameter triggers the entity-upcast path, which calls getStorage()->load(). The EntityTypeManagerInterface mock was not set up for this, causing a fatal error. Fixed by mocking getStorage('node') returning a storage where load() returns NULL.
  • SiteSettingsInheritanceSubscriberTest: The test used ->with($this->callback(function (&$ignore) { $ignore[] = 'extra'; })) to verify the alter hook call and simultaneously add a key to the ignore list. PHPUnit's with() constraint callbacks receive arguments by value, so the modification was silently dropped and extra was always inherited. Fixed by switching to willReturnCallback(function (string $hook, array &$ignore): void { ... }), which correctly passes $ignore by reference.

All 22 unit tests pass after these changes.

Issue fork sites-3601636

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:

Comments

hydra created an issue. See original summary.

hydra’s picture

Status: Needs review » Fixed

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.