Allow DeprecatedScopeResolvers to be aware of the node being processed

[mglaman]

This stems from work in mglaman/phpstan-drupal#714 and mglaman/phpstan-drupal#659 in phpstan-drupal.

It's complicated, but it's based on this code:

$result = DeprecationHelper::backwardsCompatibleCall(
        currentVersion: \Drupal::VERSION,
        deprecatedVersion: '10.3',
        currentCallable: fn() => Role::loadMultiple(),
        deprecatedCallable: fn() => user_roles(),
      );

We want to use a DeprecatedScopeResolver to ensure the code called in the deprecatedCallable is within a deprecated scope. That's been accomplished in the PR with DeprecationHelperScope so far. However, it is including currentCallable (we want to find and error on deprecated code within that callable.)

Using a NodeVisitor it is possible to traverse the children in the deprecatedCallable tree and set a deprecated attribute. But, this attribute cannot be read using the Scope alone.

There are problems with backwards compatibility to modify \PHPStan\Rules\Deprecations\DeprecatedScopeResolver::isScopeDeprecated. So, this is my idea.

Introduce NodeAwareDeprecatedScopeResolver which DeprecatedScopeResolver implementations can also implement.

interface NodeAwareDeprecatedScopeResolver
{

	public function withNode(Node $node);

}

DeprecatedScopeHelper needs to be modified to support this:

	public function isScopeDeprecated(Scope $scope, Node $node): bool
	{
		foreach ($this->resolvers as $checker) {
			if ($checker instanceof NodeAwareDeprecatedScopeResolver) {
				$checker->withNode($node);
			}
			if ($checker->isScopeDeprecated($scope)) {
				return true;
			}
		}

		return false;
	}

[ondrejmirtes]

So what is the problem here? You have this code:

$result = DeprecationHelper::backwardsCompatibleCall(
        currentVersion: \Drupal::VERSION,
        deprecatedVersion: '10.3',
        currentCallable: fn() => Role::loadMultiple(),
        deprecatedCallable: fn() => user_roles(),
      );

And when a deprecated function is called inside deprecatedCallable argument, it should not be reported, but when a deprecated function is called from inside other arguments, it should still be reported?

[mglaman]

Correct. Given that user_roles is deprecated, but we want to support multiple minor versions,

The following is OK, callable in deprecatedCallable considered.

DeprecationHelper::backwardsCompatibleCall(
    currentVersion: \Drupal::VERSION,
    deprecatedVersion: '10.3',
    currentCallable: fn() => Role::loadMultiple(),
    deprecatedCallable: fn() => user_roles(),
  );

This is not OK because the deprecated code is in currentCallable. It's not common, but it could happen without using named parameters and mixing up the arguments. Or if the currentCallable uses code that later also becomes deprecated.

DeprecationHelper::backwardsCompatibleCall(
    currentVersion: \Drupal::VERSION,
    deprecatedVersion: '10.3',
    currentCallable: fn() => user_roles(),
    deprecatedCallable: fn() => Role::loadMultiple(),
  );

[ondrejmirtes]

So how about phpstan/phpstan-src@b87e5c4 ? Would calling Scope::getFunctionCallStackWithParameters() work for you?

[mglaman]

🤔 maybe! I didn't see that method call, only the getFunctionCallStack. I will try that and report back.

[ondrejmirtes]

Because I just added it.

[mglaman]

It worked! mglaman/phpstan-drupal@23bf3ef (#714)

This allowed me to target the specific parameter in the function call stack. I love it because this proposal felt super messy.

[mglaman]

I'll close this issue, then :) An example below for anyone who happens to come across this issue.

    public function isScopeDeprecated(Scope $scope): bool
    {
        if (!class_exists(DeprecationHelper::class)) {
            return false;
        }
        $callStack = $scope->getFunctionCallStackWithParameters();
        if (count($callStack) === 0) {
            return false;
        }
        [$function, $parameter] = $callStack[0];
        if (!$function instanceof MethodReflection) {
            return false;
        }
        if ($function->getName() !== 'backwardsCompatibleCall'
            || $function->getDeclaringClass()->getName() !== DeprecationHelper::class
        ) {
            return false;
        }
        return $parameter !== null && $parameter->getName() === 'deprecatedCallable';
    }

[ondrejmirtes]

Awesome! You can expect PHPStan 1.10.56 released in the next few days.

[mglaman]

Thank you @ondrejmirtes, amazing as ever. I appreciate it!

[bbrala]

Yay! Great work, and thank you @ondrejmirtes! <3

[ondrejmirtes]

Released: https://github.com/phpstan/phpstan/releases/tag/1.10.56

[github-actions]

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.