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

ReadWritePropertiesExtension is not respected with readonly property #6632

Closed
noemi-salaun opened this issue Feb 11, 2022 · 13 comments
Closed
Labels

Comments

@noemi-salaun
Copy link

Bug report

I have Doctrine entities with an $id properties that are never initialized in my code. I made PHPStan aware of it with a ReadWritePropertiesExtension and it works perfectly.

But if I add the new readonly keyword on my property, PHPStan give me this error

Class MyEntity has an uninitialized readonly property $id. Assign it in the constructor.

Code snippet that reproduces the problem

#[ORM\Entity]
class MyEntity
{
    #[ORM\Column, ORM\Id, ORM\GeneratedValue]
    private readonly int $id;

    #[ORM\Column]
    private string $name;

    public function __construct(string $name)
    {
        $this->name = $name;
    }
}

Expected output

I expect no error from PHPStan, since the extension tell it that the property is always initialized

@mergeable
Copy link

mergeable bot commented Feb 11, 2022

This bug report is missing a link to reproduction on phpstan.org.

It will most likely be closed after manual review.

@noemi-salaun
Copy link
Author

I think I cannot reproduce it on the playground since it needs an Extension

@ondrejmirtes
Copy link
Member

I'm not sure about the nature of this problem. Do you experience this with phpstan-doctrine, or with your own custom ReadWritePropertiesExtension?

@noemi-salaun
Copy link
Author

It's my own ReadWritePropertiesExtension

final class DoctrineEntityPropertiesExtension implements ReadWritePropertiesExtension
{
    public function isAlwaysRead(PropertyReflection $property, string $propertyName): bool
    {
        return $this->hasDoctrineAnnotation($property);
    }

    public function isAlwaysWritten(PropertyReflection $property, string $propertyName): bool
    {
        return $this->hasDoctrineAnnotation($property);
    }

    public function isInitialized(PropertyReflection $property, string $propertyName): bool
    {
        return $this->hasDoctrineAnnotation($property);
    }

    private function hasDoctrineAnnotation(PropertyReflection $property): bool
    {
        if (!$property instanceof \PHPStan\Reflection\Php\PhpPropertyReflection) {
            return false;
        }

        $attributes = $property->getNativeReflection()->getAttributes();
        foreach ($attributes as $attribute) {
            if (str_starts_with($attribute->getName(), 'Doctrine\ORM')) {
                return true;
            }
        }

        return false;
    }
}

@ondrejmirtes
Copy link
Member

Looks like MissingReadOnlyPropertyAssignRule doesn't work with ReadWritePropertiesExtension but should.

@herndlm Could you please try reproducing the problem with a new RuleTestCase child class (that allows you to register a custom extension in getAdditionalConfigFiles()) and see why it doesn't work? Thanks :)

@ondrejmirtes
Copy link
Member

@herndlm This looks very similar #7337 and is actually about phpstan-doctrine. I'd love if you could find some time to fix this, thanks :)

@herndlm
Copy link
Contributor

herndlm commented May 28, 2022

Sure, is on my virtual list to be looked at next.

@ondrejmirtes
Copy link
Member

ondrejmirtes commented May 29, 2022

Fixed: phpstan/phpstan-src#1357

@herndlm
Copy link
Contributor

herndlm commented May 30, 2022

@noemi-salaun did you check already if this is working for you now? Just asking, because we seem to be still having a related problem in the phpstan-doctrine extension and this might tell me already if phpstan or phpstan-doctrine is the cause.

@noemi-salaun
Copy link
Author

@herndlm I did not, sorry. I was on a extended weekend :)
I think I'll be able to check it today.
Is there a new stable version I can use to test it, or do I have to use a dev version ?

@ondrejmirtes
Copy link
Member

It's been released already https://github.com/phpstan/phpstan/releases/tag/1.7.3.

@noemi-salaun
Copy link
Author

It looks good :)

PHPStan warns me about an unused "Ignored error pattern"

        -
            message: '#Class .* has an uninitialized readonly property \$.*\. Assign it in the constructor\.#'
            paths:
                - src/Entity

Thanks for the awesome job ;)

@github-actions
Copy link

github-actions bot commented Jul 1, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants