-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NEW Add method annotation checks #1
NEW Add method annotation checks #1
Conversation
56acbee
to
3cbf892
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also validate the that classes like HasManyList are imported when they're used for method annotations
Do you mean checking for missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally with steps in description, though I tried running on all of framework rather than just the Security dir. I got this:
Internal error: Internal error: Too few arguments to function
SilverStripe\ORM\Hierarchy\Hierarchy::get_extra_config(), 0 passed in
/path/to/somesite/vendor/silverstripe/standards/src/PHPStan/MethodAnnotationsRule.php
on line 320 and exactly 3 expected while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/Hierarchy/Hierarchy.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Child process error (exit code 1):
Internal error: Internal error: Internal error. while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/PaginatedList.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Child process error (exit code 1):
Internal error: Internal error: Internal error. while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/ListDecorator.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Child process error (exit code 1):
Internal error: Internal error: Internal error. while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/Connect/MySQLDatabase.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Internal error: Internal error: Too few arguments to function
SilverStripe\ORM\Search\FulltextSearchable::get_extra_config(), 0 passed
in
/path/to/somesite/vendor/silverstripe/standards/src/PHPStan/MethodAnnotationsRule.php
on line 320 and exactly 3 expected while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/Search/FulltextSearchable.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Internal error: Internal error: Internal error. while analysing file
/path/to/somesite/vendor/silverstripe/framework/src/ORM/GroupedList.php
Run PHPStan with -v option and post the stack trace to:
https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml
Child process error (exit code 1):
Ahh, yes, looks like in my haste I didn't notice that the method signature for |
73a7b0e
to
de41123
Compare
Turns out the method is ignored if it's declared on a /**
* @param string $class The FQCN of the class being extended (will always be DataObject from PHPStan's pov)
* @param string $extension The FQCN of this class
* @param array $args Extra arguments as part of the extension configuration (will always be an empty array from PHPStan's pov)
*/
public static function get_extra_config($class, $extension, $args): array; That's accounted for now. |
The remaining errors don't seem to be related to the custom rule. They occur before any of the code in this repository is executed. Those errors are:
I've raised phpstan/phpstan#10509 about that, but IMO that shouldn't be a blocker for merging this PR (though it will be a blocker for adding the PHPstan config to the framework repo). |
de41123
to
47862a9
Compare
Since this doesn't have CI yet, you'll have to check the CI in creative-commoners instead of on this PR directly.
See https://github.com/creative-commoners/silverstripe-standards/actions/workflows/ci.yml
Note that the PDO failure will be fixed by silverstripe/gha-ci#97
How to test this manually
phpstan/extension-installer
as dev dependenciesphpstan.neon
orphpstan.neon.dist
file to your project root with this content:vendor/bin/phpstan
This will run it against the path set above, and due to the rules set in this repo it will only run our custom rule.
@method
annotations.Issue