Difference between Instance and Static properties

[VincentLanglet]

Closes https://github.com/phpstan/phpstan-src/pull/12775

[VincentLanglet]

I encounter some issues/questions for you @ondrejmirtes.

First, HasPropertyType is tricky

• since it's used for both static and non static properties.
• and property_exists returns true either it's a static or non static properties

So we have

class Foo
{
	public $foo;
	public static $staticFoo;
}


var_dump(property_exists(new Foo(), 'foo')); // TRUE
var_dump(property_exists(new Foo(), 'staticFoo')); // TRUE
Foo::$foo; // CRASH
(new Foo())->foo; // Works
Foo::$staticFoo; // Works
(new Foo())->staticFoo; // CRASH

Second, and kinda related, how should be considered properties from ObjectShapeType ? Are they only instance properties ?

Also if you already have some comment on the existing work.

[ondrejmirtes]

Yeah, these kinds of questions are great and generally are a positive effect of deprecating old methods - it forces us to do stuff correctly and fix existing problems along the way.

My thoughts:

• HasPropertyType is used alongside property_exists so representing both instance and static properties is fine. Not sure what kind of problem you have with that now. If you don't know what to do about this we can leave it for later (it's fine to leave some failing tests here for now).
• ObjectShapeType - should work only for instance properties.

[VincentLanglet]

In the new list of todo to see with you @ondrejmirtes I have:

• PHPClassReflectionExtension::createProperty to maybe split into createInstanceProperty and createStaticProperty (?) ; I'm not familiar with the implementation yet.
• HasPropertyType::isSuperTypeOf => remplace usage of hasProperty
• HasProperty::isSubTypeOf => remplace usage of hasProperty
• Having or not a difference between HasProperty::hasInstanceProperty and HasProperty::hasStaticProperty and updating the new HasProperty() eventually.
• Deciding if the LevelsIntegrationTest change is ok. (It's UnionType related, when one of the class has not the property and the other has the static property of the same name)

HasPropertyType is used alongside property_exists so representing both instance and static properties is fine. Not sure what kind of problem you have with that now. If you don't know what to do about this we can leave it for later (it's fine to leave some failing tests here for now).

Currently the TypeSpecifier does

if (
					$var instanceof PropertyFetch
					&& $var->name instanceof Node\Identifier
				) {
					$types = $types->unionWith(
						$this->create($var->var, new IntersectionType([
							new ObjectWithoutClassType(),
							new HasPropertyType($var->name->toString()),
						]), TypeSpecifierContext::createTruthy(), $scope)->setRootExpr($expr),
					);
				} elseif (
					$var instanceof StaticPropertyFetch
					&& $var->class instanceof Expr
					&& $var->name instanceof Node\VarLikeIdentifier
				) {
					$types = $types->unionWith(
						$this->create($var->class, new IntersectionType([
							new ObjectWithoutClassType(),
							new HasPropertyType($var->name->toString()),
						]), TypeSpecifierContext::createTruthy(), $scope)->setRootExpr($expr),
					);
				}

Which I think, means that accessing to Foo::$bar makes PHPStan thinks that foo->bar exists. (and the opposite)

So should we add a param to the HasPropertyType construct which would be STATIC|INSTANCE|UNKNOWN

[phpstan-bot]

This pull request has been marked as ready for review.

[VincentLanglet]

Rebased and fix, it should be ready for a new review @ondrejmirtes :)

[VincentLanglet]

It's rebased and fixed I think.

I know it's not an easy PR to review @ondrejmirtes but it would be awesome if we succeed closing it :) Is there something I can do to help the review ?

[VincentLanglet]

It's rebased and fixed again.

This PR is really hard to maintain, are you (still ?) interested in it @ondrejmirtes ? Can I do something to help you to review it ?

[VincentLanglet]

Rebased and I had to introduce new fixes because of recent bugfixes.

Should be re-ready for review @ondrejmirtes

[staabm]

nit: PR description contains a wrong link

[VincentLanglet]

nit: PR description contains a wrong link

Thanks, updated to https://github.com/phpstan/phpstan/issues/12775

[ondrejmirtes]

Hi, I'm taking over this PR and taking it over the finish line. Already have a local branch with some changes. Stay tuned!

[ondrejmirtes]

Thank you!

[VincentLanglet]

Thank you too !