Skip to content

Report missing and invalid #[\Override] on properties (PHP 8.5)#4

Open
alies-dev wants to merge 2 commits into
fix/11890-override-property-targetfrom
feat/11890-override-property-missing
Open

Report missing and invalid #[\Override] on properties (PHP 8.5)#4
alies-dev wants to merge 2 commits into
fix/11890-override-property-targetfrom
feat/11890-override-property-missing

Conversation

@alies-dev

Copy link
Copy Markdown
Owner

PHP 8.5 extends #[\Override] to properties (the "marking overridden properties" RFC). vimeo#11891 made Psalm accept the attribute there; this is stacked on top of it so Psalm now reports the same two override diagnostics for properties that it already reports for methods, and can add the attribute automatically.

MissingOverrideAttribute is emitted (and, when ensureOverrideAttribute is enabled, fixed by vendor/bin/psalter --issues=MissingOverrideAttribute) when a property overrides a parent or implemented-interface property without carrying #[\Override]. InvalidOverride is emitted when a property carries the attribute but overrides nothing, matching the fatal the engine throws. Regular, static, and constructor-promoted properties are all covered.

Detection follows the engine's rules. Private parent properties do not count as overridable, trait-declared properties are left unchecked (the attribute lives on the shared trait, the same way trait methods are already treated), and interface properties are resolved through the implemented-interface set, since they are not copied into the implementing class. The checks are gated to PHP 8.5+, because property #[\Override] does not exist before then, while the method checks stay ungated. The autofix is withheld for multi-property statements (public int $a, $b;), where one attribute group would attach to every declared property and could break the ones that do not override.

The logic lives in a small PropertyOverrideAnalyzer helper called from ClassAnalyzer (regular properties) and FunctionLikeAnalyzer (promoted properties), mirroring the existing method handling. Interface abstract properties declared via property hooks are matched when a class implements them, but a property hook redeclared in a child interface is not yet flagged (a safe gap, never a false positive).

Stacked on vimeo#11891, so the base here is fix/11890-override-property-target. Once that PR merges, this can be rebased onto 6.x. Refs vimeo#11890.

PHP 8.5 extends #[\Override] to properties, so this mirrors the
existing method handling. MissingOverrideAttribute is reported (and
fixable with --alter) when a property overrides a parent or
implemented interface property without the attribute; InvalidOverride
is reported when the attribute is present but nothing is overridden.
Regular, static, and constructor-promoted properties are covered.

The property checks are gated to PHP 8.5+, since property #[\Override]
does not exist before then (the method check stays ungated). Builds on
vimeo#11891, which made the attribute valid on properties.
@alies-dev alies-dev self-assigned this Jun 23, 2026
@alies-dev alies-dev changed the title Report missing and invalid #[\\Override] on properties (PHP 8.5) Report missing and invalid #[\Override] on properties (PHP 8.5) Jun 23, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for the #\Override attribute on properties for PHP 8.5 and above, including documentation updates, a new PropertyOverrideAnalyzer, and integration into ClassAnalyzer and FunctionLikeAnalyzer for constructor-promoted properties. The feedback suggests refactoring PropertyOverrideAnalyzer to avoid global state by retrieving the ProjectAnalyzer directly from the $source parameter, and using lowercase keys from $class_storage->class_implements to prevent potential case-sensitivity lookup issues.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +90 to +94
if (!$can_apply_fix
|| !$codebase->alter_code
|| $property_storage->stmt_location === null
|| !isset(ProjectAnalyzer::getInstance()->getIssuesToFix()['MissingOverrideAttribute'])
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of using the global static ProjectAnalyzer::getInstance(), it is cleaner and more idiomatic to retrieve the ProjectAnalyzer instance directly from the $source parameter via $source->getProjectAnalyzer(). This avoids global state access and makes the analyzer more robust and testable.

        if (!$can_apply_fix
            || !$codebase->alter_code
            || $property_storage->stmt_location === null
            || !isset($source->getProjectAnalyzer()->getIssuesToFix()['MissingOverrideAttribute'])
        ) {

Comment on lines +125 to +130
foreach ($class_storage->class_implements as $interface_fqcln) {
if (!$codebase->classlike_storage_provider->has($interface_fqcln)) {
continue;
}

$interface_storage = $codebase->classlike_storage_provider->get($interface_fqcln);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The $class_storage->class_implements array is associative, where keys are lowercase fully qualified interface names and values are the original-cased names. Since ClassLikeStorageProvider::has() and get() expect lowercase class/interface names, iterating over the lowercase keys ($interface_fqcln_lc) is safer, more efficient, and avoids potential case-sensitivity lookup issues.

        foreach ($class_storage->class_implements as $interface_fqcln_lc => $interface_fqcln) {
            if (!$codebase->classlike_storage_provider->has($interface_fqcln_lc)) {
                continue;
            }

            $interface_storage = $codebase->classlike_storage_provider->get($interface_fqcln_lc);

Apply the Gemini review feedback on the property `#[\Override]` checks:

- Retrieve the ProjectAnalyzer from the analysis source
  (`$source->getProjectAnalyzer()`) instead of the global
  `ProjectAnalyzer::getInstance()` singleton. The first parameter is
  narrowed from `StatementsSource` to `SourceAnalyzer`, which exposes the
  accessor; both call sites (ClassAnalyzer, FunctionLikeAnalyzer) already
  pass a SourceAnalyzer.
- Look up implemented interfaces by the lowercase `class_implements` key
  rather than the original-cased value, matching how the storage provider
  keys its entries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant