do not skip Generator for MissingTypehintCheck#4209
do not skip Generator for MissingTypehintCheck#4209ondrejmirtes merged 17 commits intophpstan:2.1.xfrom
Conversation
ebcdd50 to
1dc38d3
Compare
|
Explained the wanted changes here phpstan/phpstan#7498 (comment) |
1dc38d3 to
24bbb81
Compare
|
I would love to this moving forward. Are you still interested in your PR @Flyingmana ? I think you just need to
|
|
Yes, might have time for it later today, but latest next weekend |
ac69196 to
e262f13
Compare
|
I think now all is fixed which belongs to this repository, the rest is separate repositories |
|
This pull request has been marked as ready for review. |
ondrejmirtes
left a comment
There was a problem hiding this comment.
I'd like to see more tests with the other no-longer-skipped types like Generator.
tests/PHPStan/Rules/Functions/MissingFunctionParameterTypehintRuleTest.php
Outdated
Show resolved
Hide resolved
8d0f11f to
47bbfb1
Compare
I added new tests/examples for generics in |
ondrejmirtes
left a comment
There was a problem hiding this comment.
I may have forgotten some context here but why is this necessary when for example Traversable return type already gets return type has no value type specified in iterable type Traversable? https://phpstan.org/r/53c35e3b-1a18-40c9-9ca1-6199bdfa1ed5
might have slipped my mind that it was already covered properly when I worked on it the last time. Will remove this part again. (and fix the conflict, check the tests again) |
# Conflicts: # conf/bleedingEdge.neon # conf/config.neon # conf/parametersSchema.neon
d51ed98 to
a5bf510
Compare
|
another try, Hope its now good :) |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Thank you, this makes more sense to me now. Can you please add tests for a pre-existing getIterableTypesWithMissingValueTypehint function (for example in MissingMethodReturnTypehintRule) that would check all ITERABLE_GENERIC_CLASS_NAMES to make sure that each of them is either covered by getIterableTypesWithMissingValueTypehint or by the code you've written here?
I'm a little bit worried that the current differentiation for Traversable vs. the others is brittle so I want to be sure it doesn't break in the future. The method getIterableTypesWithMissingValueTypehint does not specifically mention Traversable, it rests only on $type->isIterable()->yes() + $iterableValue instanceof MixedType && !$iterableValue->isExplicitMixed().
|
like the tests I added in |
|
Yes, it's okay. Thank you! |
|
Please send PRs to the failing extension repositories. Thanks! |
references #7498
only a draft for now, I want to see which test are all failing and for what reasons.