Modernize PHP#83
Merged
Merged
Conversation
malberts
force-pushed
the
phase-2/php-modernization
branch
5 times, most recently
from
610be58 to
d0449d9
Compare
PHP 7.4 native property types make `@var Type` docblocks above already-typed property declarations pure duplication. Strip the docblocks that contain nothing but the `@var` annotation; leave docblocks that carry an actual description. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PHP 8.0+ supports promoting constructor parameters directly to declared properties; the existing pattern of "declare each property, then immediately reassign it from a constructor argument with the same name" is pure boilerplate under that syntax. Convert the classes whose properties are pure-passthrough from ctor parameters: * `HooksHandler` (3 props promoted; the lazy-initialised `$config` stays as a regular declaration) * `OutputPageParserOutput` (both props promoted, dropping the vestigial by-reference declaration on `OutputPage` which no caller relied on) * `ImageModalTrigger` (both props promoted, with types added in the process) * `ModalBuilder` (3 ctor-driven props promoted; the eight setter-managed fields stay untyped for now) * `BootstrapComponentsService` (one promoted; the two initialised defaults move to property-level default expressions) * `ParserOutputHelper` (same pattern; the two boolean "have-we-done-X" trackers default to `false` at declaration time) * `ParserFirstCallInit` (three promoted; the `$parserOutputHelper` field is still fetched in the body) * `AbstractComponent` (three promoted; the four method-call- initialised fields stay declared with native types and the `$name` field gets its real `string` type) * `ImageModal` (four promoted; `$parserOutputHelper` keeps the fallback-via-`ApplicationFactory` body, and the `$null` legacy placeholder is left as a regular parameter) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PHP 8.1's per-property `readonly` modifier enforces "assigned in the constructor, never reassigned afterwards" at runtime, which documents intent and prevents accidental mutation by subclasses or future contributors. Apply it to every promoted constructor parameter, plus the fields that are initialised once in the constructor body and never reassigned afterwards (audited per class to confirm no caller, subclass, or method writes them again). Properties that participate in observable mutation - lazy-init caches (`HooksHandler::$config`, `BootstrapComponentsService::$nameOfActiveSkin`), mutable state (`activeComponents`, `modalsSuppressedByMagicWord`, `articleTracked`, `articleTrackedOnError`), setter-managed fields (ModalBuilder's body/dialog/outer/header/footer slots, ImageModal's `disableSourceLink`), and AbstractComponent's initComponentData-set fields (`$id`, `$parserRequest`, `$sanitizedAttributes`) - keep the regular declaration. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Card extends AbstractComponent, whose constructor signature is now typed. Untyped Card parameters left a "less strict override" gap that worked at runtime (the parent's type check kicked in on forwarding) but read inconsistently. Type them to match, and drop the @param docblock lines that the typed signature now documents. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`ImageModal::__construct()` carried a leading `$null` parameter, a placeholder that used to be a `DummyLinker` back when MediaWiki's `ImageBeforeProduceHTML` hook still passed one. MediaWiki has been passing `null` in that slot for years, and the param was never read inside the constructor; it just sat there to keep the positional shape stable. Drop it. Update the one production caller (`HooksHandler::onImageBeforeProduceHTML`) and the `ImageModalTest::createImageModalWithMocks` test helper plus its callsites. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
malberts
force-pushed
the
phase-2/php-modernization
branch
from
d0449d9 to
2cc9265
Compare
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.
Tighten the extension's PHP under the 8.1 baseline raised in #82,
ahead of the Bootstrap 5 migration.
🤖 Generated with Claude Code