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

Incorrect behavior of CompleteDynamicPropertiesRector #7574

Closed
Jimbolino opened this issue Nov 4, 2022 · 2 comments
Closed

Incorrect behavior of CompleteDynamicPropertiesRector #7574

Jimbolino opened this issue Nov 4, 2022 · 2 comments
Labels

Comments

@Jimbolino
Copy link

Bug Report

Subject Details
Rector version last dev-main
Installed as composer dependency

Minimal PHP Code Causing Issue

See https://getrector.org/demo/d322d190-3178-4bf7-9877-921b8ccb698b

<?php

class BaseClass
{
    public function run()
    {
        $this->test = true;
    }
}

class NormalClass extends BaseClass
{
    protected bool $test;
    
    public function run()
    {
        $this->test = true;
    }
}

Responsible rules

  • CompleteDynamicPropertiesRector

Expected Behavior

Should try to match the visibility of the other class

@hungtrinh
Copy link

#6806 (comment)

  1. Due to PHPStan design that we inherit here, we can only work with current context. That means PHPStan does not know about all use cases, just about current class. E.g. dead public methods cannot be reported etc. This might change in the future, but so far there is nothing we can do about it.

oops look like limit of PHPStan

@TomasVotruba
Copy link
Member

Indeed, thanks for checking.
It's better to avoid property juggling, protected properties and inheritance in general. They allow such leaky design that is hard to process and error prone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants