Proxy copies default values of inherited methods as written: class names resolve against the child file's imports

Đang mở
  • Có gì không ổn?
#836 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

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

Đánh giá

Độ khó
4/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
56/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
php
Lĩnh vực
backend

Hướng nghiên cứu

Read src/Proxy/Generator/ParameterGenerator.php and AbstractInterceptedPropertyGenerator.php, then inspect AttributeGroupsGenerator::$argumentCloner and ResolvedNameQualifyingVisitor for the existing name-qualification pattern. Add coverage for inherited defaults from another file, including colliding imports, self::CONST, and promoted or intercepted properties; run the listed composer checks with opcache.jit=disable. Done means the parent defaults are preserved and existing proxy snapshots remain unchanged.

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

Mô tả

Bug

Found during #826 (the fix went out with that declined change, see PR #831). Reproduced on master dc64ef7.

Problem

When a woven class gets an intercepted method from a parent class (or a trait) declared in another file, the proxy re-declares that method. ParameterGenerator builds the parameter type from the resolved AST names, which is correct and fully qualified. The default value, however, is copied as written (ValueGenerator::fromExprNode($astParam->default), src/Proxy/Generator/ParameterGenerator.php).

The proxy lives in the child's file and carries the child's use imports. A short class name in an inherited default (Level::LEVEL, Foo::class, new Foo()) therefore resolves against the child's imports, not the parent's.

Reproduction

// ImportedDefaultsBase.php
namespace Go\Instrument\Transformer\Stubs;
use Go\Stubs\Collision\The as Level;
class ImportedDefaultsBase
{
    public function log(string $level = Level::LEVEL, ?Level $target = null): string { return $level; }
}

// ImportedDefaultsChild.php
namespace Go\Instrument\Transformer\Stubs;
use Go\Stubs\Collision\Interceptor as Level;   // same short name, another class
class ImportedDefaultsChild extends ImportedDefaultsBase { /* … */ }

With a before advice on ImportedDefaultsChild->log, the generated proxy of the child contains:

use Go\Stubs\Collision\Interceptor as Level;
…
public function log(string $level = Level::LEVEL, ?\Go\Stubs\Collision\The $target = null): string

The type is right (\Go\Stubs\Collision\The), but the default now means Go\Stubs\Collision\Interceptor::LEVEL. Both stubs happen to define LEVEL = 'debug', which hides it in the test fixtures.

In real code, depending on the classes involved, the intercepted method then:

  • silently gets a different default (another class's constant);
  • fails on the call with "Undefined constant";
  • fails with "Class not found" when the child file has no such import.

All of this happens only when the argument is omitted, so the bug is easy to miss.

Scope

  • Affected: default values of parameters, and of promoted and intercepted properties (AbstractInterceptedPropertyGenerator copies the AST default the same way), declared in a parent class or trait from another file.
  • Not affected: types, which are already qualified from resolvedName, and attribute arguments, which AttributeGroupsGenerator already clones and qualifies with ResolvedNameQualifyingVisitor.
  • Unaffected case: members declared in the woven file itself, because the proxy shares that file's imports.
  • Related edge case: self::CONST in an inherited default refers to the declaring class in PHP. In the re-declared proxy method, self is the proxy (child) class, which matters when the child overrides the constant.

Proposed decision for the implementing agent

Keep it small and in line with how master already treats types and attribute arguments. Do not reintroduce the declined #826 alias machinery.

  1. When copying a default AST node (parameters, promoted properties, intercepted properties), clone it and qualify its names with the existing ResolvedNameQualifyingVisitor (same traverser pattern as AttributeGroupsGenerator::$argumentCloner). Do this only when the member's declaring file differs from the file being woven. Compare the declaring class's getFileName() with the woven file, which also covers trait members. Proxies of a class's own members stay byte-identical, so no existing snapshot changes.
  2. In the same foreign-file case, replace self in the default with the declaring class name (rooted). This preserves PHP's meaning of an inherited self::CONST. static can't appear in a constant expression, and parent should resolve to the declaring class's parent.
  3. Make sure the qualifying visitor does not turn self/parent into \self/\parent. A fix for that was part of the reverted #826; re-apply it if needed.

Acceptance

  • A new stub pair (the reproduction above, with different constant values in the two colliding classes). The child's proxy emits \Go\Stubs\Collision\The::LEVEL, and a functional or unit test calling log() without arguments gets the parent's value.
  • A case with self::CONST in an inherited default, with the child overriding the constant: the parent's value is kept.
  • Promoted/intercepted property defaults from a parent in another file are covered.
  • Existing proxy snapshots under tests/Instrument/Transformer/_files are unchanged (own-file members are untouched).
  • composer test, composer analyze, composer cs, and composer test:performance:transformation (generator change). Run with opcache.jit=disable: the PHP 8.5.12 tracing-JIT bug breaks the functional subprocess tests.

Scoring

Score Why
Effort Low One reused visitor on the default-copy path of the parameter and property generators, a declaring-file check, plus stubs and tests.
Impact Medium It needs an inherited, intercepted method with a class-referencing default and a colliding short name across files, so it is rare. When it hits, it is a silent wrong value or a call-time fatal.
Ngôn ngữ chính
PHP
Star
1.7k
Fork
166
Merge trung bình
23 giờ 58 phút
Pull request đã merge (30 ngày)
40

Chuẩn bị môi trường

Dự án này không cung cấp dev container, Dockerfile hay hướng dẫn đóng góp, nên bạn cần tự thiết lập môi trường: hãy bắt đầu từ README và xem hướng dẫn đóng góp lần đầu của chúng tôi để biết các bước chung.

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.

Issue khác của goaop/framework

Tất cả issue của goaop/framework

Issue tương tự

Thêm issue về PHP

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.