php / php/php-src

ReflectionProperty inconsistently checks the instance type

Đang mở
#17,730 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Bug Extension: reflection Status: Needs Triage
Ngôn ngữ chính
C
Star
40.4k
Fork
8.2k
Merge trung bình
2 ngày 13 giờ
Pull request đã merge (30 ngày)
96

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu bằng cách so sánh các phép kiểm tra instance trong các phương thức getValue(), isInitialized(), getRawValue(), setRawValueWithoutLazyInitialization(), setValue() và setRawValue() của ReflectionProperty. Xem xét bug 72209, các commit được tham chiếu và các ví dụ được liên kết trước khi quyết định liệu một quy tắc nhất quán có tương thích hay không; được coi là hoàn thành khi đã thống nhất hành vi kèm theo các kiểm thử tương ứng.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
php
Lĩnh vực
backend
Loại issue
Lỗi
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.