Adding node information to DeprecatedScopeResolver::isScopeDeprecated?
bbrala opened this issue · 11 comments
I wonder, sometimes I need more context while determining if a node in a scope is deprecated. Mostly to 'undeprecate' sometime. Would you mind if node was added as context/information to DeprecatedScopeResolver::isScopeDeprecated?
I'm trying to target the deprecated_function() in the following code:
\Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => deprecated_function_call(), fn() => count([]));
DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => deprecated_function_call(), fn() => count([]));
DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', function() {
deprecated_function_call();
}, function() {
count([]);
});
I also tried other avenues, like, trying to pragmaticly add lines to ignore through a visitor. I could also use NodeVisitor and then NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN, but that means no code checks in all of the children.
I'm kinda stumped right now, but would love to be able to only exclude deprecations from those node (be it anonimous functions, or whatever).
#73 is actually realated seeing the issue description, but the PR actually doesnt solve that issue it seems
With this change in PHPStan you'll be able to read it from Scope: phpstan/phpstan-src@9be1376
Oh wow. Thank you for the quick turnaround, I'll look into it right away
Require phpstan/phpstan 1.10.x-dev until this is released 😊
yeah i did that, unfrotunately i get an empty array in my test when i try the following in my test
<?php
declare(strict_types=1);
namespace mglaman\PHPStanDrupal\DeprecatedScope;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Deprecations\DeprecatedScopeResolver;
final class DeprecatedHelperScope implements DeprecatedScopeResolver
{
public function isScopeDeprecated(Scope $scope): bool
{
var_dump($scope->getFunctionCallStack());
return true;
}
}
Warning: No code coverage driver available
F 1 / 1 (100%)array(0) {
}
array(0) {
}
array(0) {
}
Not sure why, the scope is not a mutating scope though, or do i need to add is somewhere else also?
This is my composer.json
"require": {
"php": "^7.4 || ^8.0",
"symfony/finder": "^4.2 || ^5.0 || ^6.0 || ^7.0",
"phpstan/phpstan": "1.10.x-dev",
"phpstan/phpstan-deprecation-rules": "^1.1.4",
"symfony/yaml": "^4.2|| ^5.0 || ^6.0 || ^7.0",
"webflo/drupal-finder": "^1.2"
},
Php 8.1 also.
Seems the issue is that the DeprecatedScopeResolver does not trigger on anything below a function. When i add:
var_dump($scope->getFunctionName());
var_dump($scope->getFunctionCallStack());
return true;
I get
F 1 / 1 (100%)string(19) "methodCallingThings"
array(0) {
}
string(19) "methodCallingThings"
array(0) {
}
string(19) "methodCallingThings"
array(0) {
}
string(19) "methodCallingThings"
array(0) {
}
string(19) "methodCallingThings"
Which means it only gets to the method inwhich there are the statements. This is the full fixture:
<?php
namespace GroupLegacy;
use Drupal\Component\Utility\DeprecationHelper;
use function Deprecated\deprecated_function_call;
final class FooTest {
public function methodCallingThings(): void {
\Drupal\Component\Utility\DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => deprecated_function_call(), fn() => count([]));
DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', fn() => deprecated_function_call(), fn() => count([]));
deprecated_function_call();
DeprecationHelper::backwardsCompatibleCall(\Drupal::VERSION, '10.1.0', function() {
deprecated_function_call();
}, function() {
count([]);
});
}
}
Hmm, thinking about this. Do i need to write a rule that targets that specific deprecation (i hope not)? Or do i need to add a rule that targets arrow functions (and anonimous ones) so it can actually start checking those through the scope resolver?
- What's your vendor/bin/phpstan --version? I added one more commit that fixes this for anonymous functions.
- If you're struggling with this, you've seen the rule in phpstan-src that tests the Scope getter. You can add more test cases there and test if and where the call stack gets lost.
I mightve been too fast then. I'll look again tonight.
Thanks for the update @ondrejmirtes I'm going to help dig in :)
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.