Check whether function parameter+return has typehint #20
Conversation
|
Missing tests for closures, which must not be covered by this rule. |
|
Again, I don’t see rules.neon modified.
On Sun, 6 May 2018 at 22:41 Michael Moravec ***@***.***> wrote:
Missing tests for closures, which must not be covered by this rule.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGZuMiq8rDLkKAOC8DVKNpNN6xUk8ELks5tv2AUgaJpZM4T0Le_>
.
--
Ondřej Mirtes
|
|
@Majkl578 closures as direct parameter of function or stored into variable? |
adaamz
force-pushed
the
master
branch
3 times, most recently
from
5d0143d to
c60ca9d
Compare
|
@Majkl578 Added closure test. |
Both. There is no supported way to document closure signature. |
| public function processNode(Node $node, Scope $scope): array | ||
| { | ||
| if ($scope->isInClass()) { | ||
| throw new \PHPStan\ShouldNotHappenException(); |
| throw new \PHPStan\ShouldNotHappenException(); | ||
| } | ||
|
|
||
| $functionReflection = $this->broker->getCustomFunction(new Node\Name($node->name, $node->getAttributes()), $scope); |
There was a problem hiding this comment.
I think that passing the attributes is not necessary.
|
|
||
| if ($parameterType instanceof MixedType && !$parameterType->isExplicitMixed()) { | ||
| return sprintf( | ||
| 'Function %s() has parameter $%s with no typehint specified', |
There was a problem hiding this comment.
Please add a dot . at the end. If there are other rules from you without this trailing dot, please add it in a new commit as well 😊
|
|
||
| public function setBroker(Broker $broker): void | ||
| { | ||
| $this->broker = $broker; |
There was a problem hiding this comment.
In rules, you can pass broker directly into the constructor.
| private $broker; | ||
|
|
||
| /** | ||
| * @return string Class implementing \PhpParser\Node |
There was a problem hiding this comment.
This phpDoc is not necessary.
Closures have different node type so I think the test is not necessary. |
|
Fixed and merged, thanks! |
Hi,