PHPStan: Add extension for typing Registry::call()#892
Conversation
7f4cdcb to
d1d2961
Compare
d1d2961 to
aa72efb
Compare
|
I would recommend to move this extension class inside the tests folder, maybe |
|
I do not think it is really a fixture. I am also considering adding more maintenance scripts (e.g. Rector rules for automatic migration to SimplePie 2). |
|
Actually, looking at |
781b40a to
2943662
Compare
|
Moved the file and added a |
2943662 to
55130ed
Compare
While reading the rector book I noticed that rector recommends
|
Art4
left a comment
There was a problem hiding this comment.
LGTM, but we should move the src/PHPStan files into utils/phpstan/. This way we won't need to maintain the build/compile.php and are open for future rector rules.
|
Is there any suggestion for namespaces? In the past, I have used |
|
The suggested namespaces are: I've looked into SimplePie folders and namespaces (ignoring the
Following this pattern the
But I don't like that we restrict us from using the potential What do you think about moving the tests into a separate
The PSR-4 rules could be condensed into: {
"autoload": {
"psr-4": {
"SimplePie\\": "src"
},
"psr-0": {
"SimplePie": "library"
}
},
"autoload-dev": {
"psr-4": {
"SimplePieTests\\": "tests",
"SimplePieUtils\\": "utils"
}
},
} |
|
Good points about Basically, I would say that the The correct(™️) way to provide an installable PHPStan extension or Rector rules might be separate repositories. But I think this work is not justifiable atm. But having the extension would be very nice. To have it near our code the If someone really like to use the PHPStan extension in their project they could copy the extension code or (not recommended) could install the extension manually in their project like this: composer.json {
"require": {
"simplepie/simplepie": "^1.9"
},
"autoload-dev": {
"psr-4": {
"SimplePieUtils\\PHPStan\\": "vendor/simplepie/simplepie/utils/PHPStan/"
}
}
}phpstan.neon includes:
- vendor/simplepie/simplepie/utils/PHPStan/extension.neonFor potential Rector rules the "installation" could work like this: composer.json {
"require": {
"simplepie/simplepie": "^1.9"
},
"autoload-dev": {
"psr-4": {
"SimplePieUtils\\Rector\\": "vendor/simplepie/simplepie/utils/Rector/"
}
}
}rector.php <?php
use Rector\Config\RectorConfig;
use SimplePieUtils\Rector\Rector\DoingSomeStuffRector;
return RectorConfig::configure()
->withRules([
DoingSomeStuffRector::class
]); |
This will be needed for PHPStan to pass with the next commit.
`Registry::call()` is widely used in SimplePie to allow extensibility. Unfortunately, the pattern hides the actual method from PHPStan so it cannot determine the return value type, and there is no annotation we can add to make it work. , This results in an inadequate analysis. Thankfully, most cases allow us to determine the method statically so we can create a PHPStan extension doing just that. We use `SimplePieUtils` instead of `Utils` namespace since this extension might be useful for some SimplePie consumers and do not want to subject them to symbol conflicts. However, we do not currently want to commit to making this a proper Composer package so use it on your own risk, stability not guaranteed. We may want to add more type checking extensions in the future.
55130ed to
32e7828
Compare
|
Registry::call()is widely used in SimplePie to allow extensibility. Unfortunately, the pattern hides the actual method from PHPStan so it cannot determine the return value type, and there is no annotation we can add to make it work. , This results in an inadequate analysis.Thankfully, most cases allow us to determine the method statically so we can create a PHPStan extension doing just that.
The extension might be useful for SimplePie consumers so let’s expose it in
composer.json.We may want to add more type checking extensions in the future.