PHP 8.2: seal all properties configuration

[danog]

No description provided.

[orklah]

I quite agree with the sentiment, but I'm not sure how PHP 8.2 changed anything here, so I don't see why the default should change based on this version

Basically, PHP 8.2 throw a deprecated when using an undeclared property without __get or __set.

Here, you suggest emitting an issue in Psalm even when __get or __set are present based only on @Property presence

I'd be up for adding this config with a false default for Psalm 4 and switching it to true for Psalm 5. The default based on PHP version seems weird to me.

(Note: I'm confused by the fact that there is an error on the assignment but not on the fetch here: https://psalm.dev/r/ce07ecb689 given that my config has the Use PHPDoc methods and properties without magic call. checked, I'd not expect issue on the assignment either)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/ce07ecb689

<?php
/** 
 * @property bool $foo 
 * @property bool $foo2 
 */
class A {
    //public function __get(string $name) {}
    //public function __set(string $name, bool $value) {}
}
class B extends A {}
$b = new B();
$b->foo = true;
$a = $b->foo2;

Psalm output (using commit 7b18673):

ERROR: UndefinedPropertyAssignment - 12:1 - Instance property B::$foo is not defined

INFO: UnusedVariable - 13:1 - $a is never referenced or the value is not used

[VincentLanglet]

I quite agree with the sentiment, but I'm not sure how PHP 8.2 changed anything here, so I don't see why the default should change based on this version

Basically, PHP 8.2 throw a deprecated when using an undeclared property _without __get or _set.

Here, you suggest emitting an issue in Psalm even when __get or __set are present based only on @Property presence

I'd be up for adding this config with a false default for Psalm 4 and switching it to true for Psalm 5. The default based on PHP version seems weird to me.

I think the idea was that people shouldn't do deprecated stuff. I don't think that's a bad idea but as a user I would also consider weird to have such impact when bumping my php version. The psalm4/psalm5 BC-break seems more natural.

Also, I might not understand all the changes but

public function shouldSealAllProperties(ClassLikeStorage $storage): bool
    {
        return $storage->sealed_properties
            || $this->config->seal_all_properties
            || ($this->php_major_version >= 8 && $this->php_minor_version >= 2);
    }

is returning always true for version 8.2 ; which means I cannot override the value with my config.

And speaking about PHP 8.2, this should supports the #[AllowDynamicProperties] attributes.

[danog]

You're both right, I may have confused this task with another, different task regarding PHP 8.2 migration, sorry about this.
All fixed now!

[orklah]

Thanks!