Skip to content

Conversation

paxal
Copy link
Contributor

@paxal paxal commented Jun 26, 2019

  • Add deprecations errors for functions typehints ‒ InFunctionNode
  • Also make errors prettier for anonymous class in deprecations for class method typehints

@paxal paxal force-pushed the typehint_function_signature branch from ee435ce to ebf68c2 Compare July 23, 2019 09:50
@paxal
Copy link
Contributor Author

paxal commented Jul 23, 2019

I've added closures as discussed here #13 (comment)

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really good! I've found a few minor issues but otherwise this is great! 👍

return [];
}

/** @var FunctionReflection $function */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line with @var does not have to be here if we have that if right below.


$errors = [];
foreach ($functionSignature->getParameters() as $i => $parameter) {
/** @var ParameterReflection $parameter */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this @var has to exist - $functionSignature->getParameters() is already properly annotated.

private function getClassDeprecationDescription(ClassReflection $class): string
{
$description = null;
if (method_exists($class, 'getDeprecatedDescription')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're on 0.12 already, this method always exists.


$errors = [];
foreach ($functionSignature->getParameters() as $i => $parameter) {
/** @var ParameterReflection $parameter */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This @var does not have to be here.

private function getClassDeprecationDescription(ClassReflection $class): string
{
$description = null;
if (method_exists($class, 'getDeprecatedDescription')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDeprecatedDescription always exists

use PHPStan\Broker\Broker;
use PHPStan\Reflection\ClassReflection;

trait DeprecationHelper
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really dislike traits. Make this a service that's injected into the constructor instead. Thank you.

@@ -10,4 +10,6 @@ rules:
- PHPStan\Rules\Deprecations\InheritanceOfDeprecatedInterfaceRule
- PHPStan\Rules\Deprecations\InstantiationOfDeprecatedClassRule
- PHPStan\Rules\Deprecations\TypeHintDeprecatedInClassMethodSignatureRule
- PHPStan\Rules\Deprecations\TypeHintDeprecatedInClosureSignatureRule
- PHPStan\Rules\Deprecations\TypeHintDeprecatedInFunctionSignatureRule
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeHintDeprecatedInFunctionSignatureRule should be in different commit.

@paxal paxal force-pushed the typehint_function_signature branch from ebf68c2 to 51e7861 Compare July 24, 2019 17:10
@paxal
Copy link
Contributor Author

paxal commented Jul 25, 2019

I updated the branch. I'm sorry ‒ I reorganized things a bit so I had to force-push.
Thanks for your patience BTW :)

@ondrejmirtes
Copy link
Member

Perfect, thank you!

@ondrejmirtes ondrejmirtes merged commit e932fb5 into phpstan:master Jul 25, 2019
@ondrejmirtes
Copy link
Member

@paxal If you'd be interested, I can always throw more work at you in PHPStan core :) There are many simple bugs that can be fixed if you're willing to dip your toes in the internals :) Since you already know how writing rules work, it should be easy for you to learn a little bit more...

If you're interested, I can give you more info on each issue.

@paxal
Copy link
Contributor Author

paxal commented Jul 25, 2019

Thank you Ondřej, I bookmarked your link :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants