php / php/php-src

ReflectionProperty inconsistently checks the instance type

Offen
#17,730 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Bug Extension: reflection Status: Needs Triage
Vorherrschende Sprache
C
Sterne
40.4k
Forks
8.1k
Ø Merge
2 T. 13 Std.
Gemergte PRs (30 T.)
96

Beschreibung

Description

Methods of ReflectionProperty that accept an object instance, are checking the object type inconsistently.

Given a ReflectionProperty created with new ReflectionProperty($className, $propName), here is a breakdown of the checks performed by the different methods:

  • getValue(), isInitialized(): $object must be an instance of $className or of $propName's declaring class ($className itself or a parent)
  • getRawValue(), setRawValueWithoutLazyInitialization(): $object must be an instance of $className
  • setValue(), setRawValue(): $object must be an object

Technically, these methods would work with any object, except setRawValueWithoutLazyInitialization which accesses the property backing store directly. However it makes sense that methods of new ReflectionProperty($className, $propName) don't accept instances unrelated to $className.

Historically, the check in getValue() was added as part of https://bugs.php.net/bug.php?id=72209, and only accepted $className. It was then changed to accept the declaring class in c97b8bbf8252, and both in 0e3045ae69d1, along with a TODO comment suggesting to accept only $className (but doing so would have been a BC). The check was propagated to getRawValue, setRawValueWithoutLazyInitialization with the TODO comment applied.

The inconsistency is not a huge issue, but it makes it impossible to cache the check result in https://github.com/php/php-src/pull/17698 (if the check were consistent, it would be possible to bypass the instanceof check when the cache is populated for the object's class).

Unfortunately, making the tests consistent would be a BC break:

  • Changing to "$object must be an instance of $className or of $propName's declaring class" would break https://3v4l.org/Tuej7
  • Changing to "$object must be an instance of $className" would break the same use-case, and also https://3v4l.org/E0srl

I'm not sure it's worth changing.

PHP Version

PHP-8.0

Operating System

No response

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne damit, die Instanzprüfungen in den Methoden getValue(), isInitialized(), getRawValue(), setRawValueWithoutLazyInitialization(), setValue() und setRawValue() von ReflectionProperty zu vergleichen. Prüfe Bug 72209, die referenzierten Commits und die verknüpften Beispiele, bevor du entscheidest, ob eine konsistente Regel kompatibel ist; als erledigt gilt die Aufgabe, wenn ein abgestimmtes Verhalten mit entsprechenden Tests vorliegt.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
php
Bereich
backend
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.