Modernise PHP code: MW 1.43 namespaces, native types, readonly#97
Merged
Conversation
Switch four top-level imports (Parser, IncludableSpecialPage, Message, WebRequest) to their MW 1.43 namespaced equivalents under MediaWiki\Parser, MediaWiki\SpecialPage, MediaWiki\Message, MediaWiki\Request. No behaviour change. Side benefit: resolves three pre-existing psalm UndefinedClass errors that flagged Parser and IncludableSpecialPage at the top-level namespace.
Replace @var docblocks with native PHP type declarations. NetworkConfig's properties become readonly — they are set once in the constructor (which computes them from MW main config) and only read thereafter. The one remaining docblock is the @var string[] hint on NetworkConfig::$excludedNamespaces, kept because native PHP arrays don't carry element-type information.
- NetworkFunction::handleParserFunctionCall() declares native array|string return type (PHP 8.0 union) - SpecialNetworkPresenter::getReturnValue() declares native string return type - SpecialNetwork::execute() keeps untyped $subPage (parent SpecialPage::execute is also untyped, so a narrower native type would be an LSP-narrowing violation that psalm and phpstan reject). Replaces phpcs:ignore with @inheritdoc, which satisfies the missing-docblock check without restating what the parent already documents.
- NetworkFunction and NetworkUseCase use PHP 8.0 constructor promotion with readonly, removing the separate property declaration + assignment boilerplate - SpyNetworkPresenter::$viewModel is now ?ResponseModel = null, matching its nullable getter (real type bug fix — the property is genuinely unset before buildGraph() is called) - All test methods in NetworkUseCaseTest and NetworkFunctionIntegrationTest get explicit : void return types
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Conservative pass on the PHP source to use:
Parser,IncludableSpecialPage,Message,WebRequest(was: bare top-level imports). Side benefit: resolves three pre-existing psalmUndefinedClasserrors.NetworkConfigandParserFunctionNetworkPresenter(was:@vardocblocks).NetworkConfigproperties are nowreadonly.NetworkFunction::handleParserFunctionCall(): array|stringandSpecialNetworkPresenter::getReturnValue(): string(was: docblock-only).readonlyonNetworkFunctionandNetworkUseCase.SpyNetworkPresenter::$viewModelis now?ResponseModel = nullto match its already-nullable getter (real bug, not just style).: voidadded to all test methods inNetworkUseCaseTestandNetworkFunctionIntegrationTest.PHP floor and MediaWiki floor are unchanged.
Deviation from spec
The spec proposed
SpecialNetwork::execute( ?string $subPage )to drop thephpcs:ignore. That can't actually happen —SpecialPage::execute( $subPage )in MW core has no native type, so adding?stringis an LSP-narrowing that both phpstan and psalm reject. Replaced thephpcs:ignorewith@inheritDocinstead, which satisfies the missing-docblock check without restating the parent.Out of scope (intentional)
MediaWikiServices::getInstance()inNetworkConfigwith proper DI.RequestModel/ResponseModelDTOs with constructor-based readonly objects.Extension::getFactory()static indirection.psalm-suppressannotations inSpecialNetwork.php.Test plan
Local verification (full quality suite green on combined HEAD against the dev wiki):
composer phpunit→ 19/19 tests passvendor/bin/phpcs -p -s→ 14/14 files cleanvendor/bin/phpstan analyse→ No errorsvendor/bin/psalm→ No errors foundCI (matrix from the recent CI-update PR):