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 concretecreate()withnew self()to every leaf class - Add missing
): static/): selfreturn types to all remainingcreate()methods for consistency - Narrow
applies()return type from?booltoboolin final example plugins (valid covariant narrowing; PHPStan can now provenullis never returned) - Move hidden-plugin removal from
getDefinitions()intoalterDefinitions(), 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 withwillReturnCallbackusing a call counter. - By-reference reflection call:
ReflectionMethod::invoke($sut, $definitions)does not pass by reference. Fixed by switching toinvokeArgs($sut, [&$definitions]). - URL generator test mocks: The test was calling
getSetting()directly on theSiteProxyInterfacemock, but the production code callsgetSite()->getSetting(). Fixed by wiringgetSite()to return aSiteInterfacemock. Also fixed:Request::create('/')uses hostlocalhost, not the intended test host — requests now useRequest::create('https://host.example/'). ThesetContextmock was a no-op so context restoration could not be verified — fixed by making the callback actually propagate the host change. - SitesServiceTest:
buildRouteMatchKeycallsgetRawParameters()->all()andgetParameters()->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(). TheEntityTypeManagerInterfacemock was not set up for this, causing a fatal error. Fixed by mockinggetStorage('node')returning a storage whereload()returnsNULL. - 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'swith()constraint callbacks receive arguments by value, so the modification was silently dropped andextrawas always inherited. Fixed by switching towillReturnCallback(function (string $hook, array &$ignore): void { ... }), which correctly passes$ignoreby reference.
All 22 unit tests pass after these changes.
Issue fork sites-3601636
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 #3
hydra commented