Checking variable variables for object properties.

[voku]

With this pull request, we have the possibility to check variable variables object properties in foreach loops.

For static and non-static variable variables we check if the key from the array shape is available as property, and we check if the type from the array shape (or from the current variable) is compatible with the type from the class.

⇾ PropertyReflectionFinder->findPropertyReflectionFromNode() is now
marked as internal. New findPropertyReflectionsFromNode() method is added,
which return an array of FoundPropertyReflection objects and I replaced all findPropertyReflectionFromNode() calls in the codebase.

Example:

class Foo
{
	/**
	 * @var string
	 */
	public static $foo = '';

	/**
	 * @var null|\stdClass
	 */
	public static $lall;

	/**
	 * @phpstan-return array{foo: 'bar', lall: string, noop: int}
	 */
	public function data(): array
	{
		return ['foo' => 'bar', 'lall' => 'lall', 'noop' => 1];
	}

	public function create(): self {
		$self = new self();

		foreach($this->data() as $property => $value) {
			self::${$property} = $value; // <- will report usage of "noop", because the static property is not available
                                                                   //  <- will report Foo::$foo (string) does not accept int
                                                                   //  <- will report Foo::$lall (stdClass|null) does not accept string
		}

		return $self;
	}
}

---

This change is 

[ondrejmirtes]

Also please rebase and clean up the commit history.

[voku]

Also please rebase and clean up the commit history.

done

[ondrejmirtes]

Hi, I've reworked your PR: #348

I reused your tests and did the most simple implementation to make the tests pass.

This case is problematic as it currently leads to false positives:

		$data = $this->data();

		foreach($data as $property => $value) {
			$self->{$property} = $value;
		}

Can you point out what you did in your PR to make the tests pass?

I feel like it's not worth to change the property rules around this case as the analysis itself needs to be more precise - the rules themselves will start working correctly once the analysis is correct.

I've did a PoC of the better foreach analysis: #349 but it needs more work...

[ondrejmirtes]

I'm just realizing this will need a completely different solution than what I did in #349. It will need something that gets closer to https://github.com/phpstan/phpstan/milestone/12.

[ondrejmirtes]

Alright, so I merged #348 without changes to TypesAssignedToPropertiesRule which would require some analyser engine changes, which I can't tackle right now (I'm busy working on PHP 8 support).

Thank you.

[voku]

Alright, so I merged #348 without changes to TypesAssignedToPropertiesRule which would require some analyser engine changes, which I can't tackle right now (I'm busy working on PHP 8 support).

Thank you

No problem, I am happy if we can push this forward, step by step. 👍

And yes, I also realized while playing with the code and types that there are some missing information for arrays.

The variable variable part needed more code as expected because I could not find the correct types. I only got a merged UnionType object (without keys / index from the array-annotation) [e.g. UnionType<IntType|FloatType> vs. array{foo: int, bar: float, 'lall': float}] or an UnionType object with only information about the types of the keys from array-annotation.

What I missed here was the original information from the array or from the array-shape in the elements of the array. I only found the information in ConstantArrayType object, from the array-shape (Foreach_->expr) itself. This is why I searched for it via findArrayShapeTypesFromAssignmet().

[ondrejmirtes]

@voku FYI I brought the expressions check back to TypesAssignedToPropertiesRule, thanks to these commits:

• 54284c8
• c28092e
• 7458d1e

Please test the latest dev-master on a codebase that heavily uses this so we can verify it works as expected 👍