Closed mglaman closed 8 months ago
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?
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(),
);
So how about https://github.com/phpstan/phpstan-src/commit/b87e5c4c7e33a91c61341f0335a221c32df603b2 ? Would calling Scope::getFunctionCallStackWithParameters()
work for you?
🤔 maybe! I didn't see that method call, only the getFunctionCallStack
. I will try that and report back.
Because I just added it.
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.
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';
}
Awesome! You can expect PHPStan 1.10.56 released in the next few days.
Thank you @ondrejmirtes, amazing as ever. I appreciate it!
Yay! Great work, and thank you @ondrejmirtes! <3
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.
This stems from work in https://github.com/mglaman/phpstan-drupal/pull/714 and https://github.com/mglaman/phpstan-drupal/pull/659 in phpstan-drupal.
It's complicated, but it's based on this code:
We want to use a
DeprecatedScopeResolver
to ensure the code called in thedeprecatedCallable
is within a deprecated scope. That's been accomplished in the PR with DeprecationHelperScope so far. However, it is includingcurrentCallable
(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 adeprecated
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
whichDeprecatedScopeResolver
implementations can also implement.DeprecatedScopeHelper
needs to be modified to support this: