Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypassing readonly properties without visibilty can change behavior #49

Open
ms2ad opened this issue Jan 30, 2024 · 1 comment
Open

Comments

@ms2ad
Copy link

ms2ad commented Jan 30, 2024

Reproduction example:

  1. composer require dg/bypass-finals --dev
  2. index.php:
<?php

require_once __DIR__ . '/vendor/autoload.php';

\DG\BypassFinals::enable();

require_once __DIR__ . '/include.php';

new A('A');
new B('B');
  1. include.php:
<?php

class A {
    public function __construct(
        readonly string $a
    ) {
        var_dump('A: ' . $this->a);
    }
}

class B {
    public function __construct(
        public readonly string $a
    ) {
        var_dump('B: ' . $this->a);
    }
}

Result:

php index.php
PHP Warning:  Undefined property: A::$a in /…/test-bypassfinals/include.php on line 7

Warning: Undefined property: A::$a in /…/test-bypassfinals/include.php on line 7
string(3) "A: "
string(4) "B: B"

The problem stems from the fact that readonly is just removed from the code thus changing the promoted property to a simple constructor argument and thus changing behavior.

Context: We use a third-party library that does uses this kind of code.


I am unsure whether this package can really fix this issue because at the moment, a "simple" replace logic without considering the context is used. Here, the context of being a promoted property would have to be known and readonly replaced with public.

@dg
Copy link
Owner

dg commented May 16, 2024

Resolving this issue is not easy because the parser must now distinguish when readonly is used with a class, when with a property with visibility, and when with a property without it. I will probably stick with the fact that it will be incompatible with code written this way.

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

No branches or pull requests

2 participants