Implement a rule to check deprecated hook implementations #634
Conversation
Kingdutch
force-pushed
the
feature/hook-deprecations
branch
from
732c9a7 to
39183e8
Compare
| public function processNode(Node $node, Scope $scope) : array | ||
| { | ||
| assert($node instanceof Function_); | ||
| if (!str_ends_with($scope->getFile(), ".module") && !str_ends_with($scope->getFile(), ".inc")) { |
There was a problem hiding this comment.
phpstan-drupal still supports PHP 7.4 technically for Drupal 9. But we can trust the PHP 8 polyfill exists for D9, so it should be okay right?
There was a problem hiding this comment.
Also, Drupal 9 officially doesn't support 7.4 anymore ;)
There was a problem hiding this comment.
"officially" – or wait, does it actually have breaking code? Or still "supports" it via polyfill?
There was a problem hiding this comment.
Probably no breaking code, haven't noticed, just bo promised I think
There was a problem hiding this comment.
I've used str_ends_with and str_starts_with for pretty much the entirety of Drupal 9's lifetime :D The symfony/polyfill-php80 package takes care of the functions (and Drupal depends on it).
Not sure if we need to add a dependency on that package explicitly in phpstan-drupal?
There was a problem hiding this comment.
Nah, I think we're OK and I should probably just drop PHP 7.4. I was waiting for PHPStan to drop it, but the Drupal community has pretty much moved onto PHP 8.1 (it seems.)
Nothing to do here, though.
|
I am surprised this works given |
I don't see these changes? We'd have to add something here https://github.com/mglaman/phpstan-drupal/blob/main/src/Drupal/DrupalAutoloader.php#L110-L113 which is how we load post update files |
Kingdutch
force-pushed
the
feature/hook-deprecations
branch
from
39183e8 to
90ae00c
Compare
|
Modified the DrupalAutoloader for Moved the hook declarations to the fixture's Edit: removed the stray |
This will analyse hook implementations in `.inc` and `.module` files. It detects a hook by looking at the pattern `mymodule_[hook]` which is detected from the file-name the function is in. It will replace `mymodule` with `hook` and then try to find a function with that name. For this to work PHPStan Drupal's `DrupalAutoloader` class must be adjusted to load `.api.php` files as well so that the hooks can be discovered. In testing on the Open Social code-base this did not cause any issues. The rule will provide the deprecation message in the error if it's provided, if no deprecation message was provided (i.e. a lonesome `@deprecated` tag) then the hook will simply be shown as deprecated without advice.
Kingdutch
force-pushed
the
feature/hook-deprecations
branch
from
90ae00c to
ad2b929
Compare
| public function processNode(Node $node, Scope $scope) : array | ||
| { | ||
| assert($node instanceof Function_); | ||
| if (!str_ends_with($scope->getFile(), ".module") && !str_ends_with($scope->getFile(), ".inc")) { |
There was a problem hiding this comment.
Nah, I think we're OK and I should probably just drop PHP 7.4. I was waiting for PHPStan to drop it, but the Drupal community has pretty much moved onto PHP 8.1 (it seems.)
Nothing to do here, though.
This will analyse hook implementations in
.incand.modulefiles. It detects a hook by looking at the patternmymodule_[hook]which is detected from the file-name the function is in.It will replace
mymodulewithhookand then try to find a function with that name. For this to work PHPStan Drupal'sDrupalAutoloaderclass must be adjusted to load.api.phpfiles as well so that the hooks can be discovered. In testing on the Open Social code-base this did not cause any issues.The rule will provide the deprecation message in the error if it's provided, if no deprecation message was provided (i.e. a lonesome
@deprecatedtag) then the hook will simply be shown as deprecated without advice.Fixes #126