php / php/php-src

ReflectionProperty inconsistently checks the instance type

Aperta
#17,730 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Bug Extension: reflection Status: Needs Triage
Lingua principale
C
Stelle
40.4k
Fork
8.1k
Merge medio
2g 13h
PR unite (30g)
96

Descrizione

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

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia confrontando i controlli sull'istanza nei metodi getValue(), isInitialized(), getRawValue(), setRawValueWithoutLazyInitialization(), setValue() e setRawValue() di ReflectionProperty. Esamina il bug 72209, i commit a cui si fa riferimento e gli esempi collegati prima di decidere se una regola coerente è compatibile; il lavoro è completato quando sono stati concordati un comportamento e i relativi test.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
php
Ambito
backend
Tipo di issue
Bug
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.