Add parameter type hints to tests #2369
Conversation
franmomu
force-pushed
the
add_types_6
branch
2 times, most recently
from
cc67707 to
fba6112
Compare
| * @dataProvider provideProxiedExprMethods | ||
| */ | ||
| public function testProxiedExprMethods($method, array $args = []): void | ||
| public function testProxiedExprMethods(string $method, $args = []): void |
There was a problem hiding this comment.
It doesn't look like there is a Closure in use. The previous array type seemed to be fitting. What do I miss?
There was a problem hiding this comment.
Nothing 😬 , I do missed this because of the name of the function is used in other tests, thanks!
franmomu
force-pushed
the
add_types_6
branch
from
fba6112 to
7d87cad
Compare
| @@ -119,6 +121,9 @@ public function testAtomicCollectionUnset($clearWith): void | |||
| $this->assertCount(0, $user->phonenumbers); | |||
| } | |||
|
|
|||
| /** | |||
| * @return array<array{array|ArrayCollection|null}> | |||
There was a problem hiding this comment.
This looks like an overkill and I myself as a maintainer wouldn't like to write this, let alone contributors. Each array gets passed as arguments to a function that can be typed so I'd argue that's enough. Is there any way of opting out of defining types for data providers?
There was a problem hiding this comment.
I was trying to add this information to also fix issues with phpcs sniff SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingTraversableTypeHintSpecification and fix issues with static analysis tools (phpstan level 6) where you should define the iterable type.
But I understand this may seem overkill, we could do the same here as the return types I guess, keeping the rule and for phpstan we could use something like phpstan/phpstan-phpunit#63 (comment).
There was a problem hiding this comment.
I think I already removed those phpdoc (I may have left some, I'll check tomorrow)
Summary
I can split it in several PRs if it's necessary.