Fix AccessPropertiesCheck for checkThisOnly

[VincentLanglet]

Closes phpstan/phpstan#13271

[ondrejmirtes]

Why is this error happening only on level 1 right now? I don't get it.

[ondrejmirtes]

Also I'm worried that if you just silence this situation for checkThisOnly=false, it will still happen in: https://phpstan.org/r/c822e0d3-9d45-4531-aced-23ce661afdcb

[ondrejmirtes]

I just don't get why is this happening, please explain how you debugged this :)

[VincentLanglet]

Also I'm worried that if you just silence this situation for checkThisOnly=false, it will still happen in: phpstan.org/r/c822e0d3-9d45-4531-aced-23ce661afdcb

The "special check" for thisOnly already exists for some other property-related rules

phpstan-src/src/Rules/Properties/ReadingWriteOnlyPropertiesRule.php

Lines 45 to 51 in cb331f5

	if (
	$node instanceof Node\Expr\PropertyFetch
	&& $this->checkThisOnly
	&& !$this->ruleLevelHelper->isThis($node->var)
	) {
	return [];
	}

The fact is the line

$nameTypeResult = $this->ruleLevelHelper->findTypeToCheck(
			$scope,
			$nodeName,
			'',
			static fn (Type $type) => $type->toString()->isString()->yes(),
		);
$nameType = $nameTypeResult->getType();

• Return an ErrorType on level 1
• Return the correctType on level > 1

So then the early return

		if (
			!$nameType instanceof ErrorType
			&& !$nameType->toString() instanceof ErrorType
			&& $nameType->toString()->isString()->yes()
		) {
			return [];
		}

only works for level > 1.

And why do we get ErrorType on level 1 ?
Because of the early return

if ($this->checkThisOnly && !$this->isThis($var)) {
			return new FoundTypeResult(new ErrorType(), [], [], null);
		}

in

phpstan-src/src/Rules/RuleLevelHelper.php

Lines 168 to 181 in cb331f5

	public function findTypeToCheck(
	Scope $scope,
	Expr $var,
	string $unknownClassErrorPattern,
	callable $unionTypeCriteriaCallback,
	): FoundTypeResult
	{
	if ($this->checkThisOnly && !$this->isThis($var)) {
	return new FoundTypeResult(new ErrorType(), [], [], null);
	}
	$type = $scope->getType($var);
	return $this->findTypeToCheckImplementation($scope, $var, $type, $unknownClassErrorPattern, $unionTypeCriteriaCallback, true);
	}

I tried first to change the early return in the rule to

if (
			$nameType instanceof ErrorType
			|| (!$nameType->toString() instanceof ErrorType && $nameType->toString()->isString()->yes())
		) {
			return [];
		}

but some tests are failing which missing error like

Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but object was given

[ondrejmirtes]

I noticed that the conditions in AccessPropertiesCheck are wrong.

This is the concerning line:

phpstan-src/src/Rules/Properties/AccessPropertiesCheck.php

Line 68 in 74e8432

if ($nameType instanceof ErrorType || $nameType->toString() instanceof ErrorType || !$nameType->toString()->isString()->yes()) {

The error CANNOT be reported when $nameType instanceof ErrorType. There was a missing !.

Additionally, I synced up the rest of the condition with the callback passed to findTypeToCheck.

So now, the callback says:

static fn (Type $type) => !$type->toString() instanceof ErrorType && $type->toString()->isString()->yes()

And the whole condition after is:

				if (
					!$nameType instanceof ErrorType
					&& (
						$nameType->toString() instanceof ErrorType
						|| !$nameType->toString()->isString()->yes()
					)
				) {

See the commit: 81a8d34

This is how all the other occurences of findTypeToCheck work. So the issue had nothing to do with checkThisOnly.

The fact that errors about object disappeared is correct, they're supposed to be reported only on level 7+ and that's not what the test checks.

[ondrejmirtes]

/cc @zonuexe