Skip to content
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

validate dataProvider return/parameter types #152

Closed
wants to merge 1 commit into from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 7, 2022

Closes #70

@staabm staabm force-pushed the data-provider-types branch 3 times, most recently from 861a160 to c8d5d0d Compare December 7, 2022 20:38
@ondrejmirtes
Copy link
Member

I'd like a completely different approach than this.

  1. Data providers usually don't come with precise typehints. So instead of comparing the return type of a data provider against the test case method signature, I'd like to inspect return, yield, and yield from in data providers against the methods that expects them.
  2. I don't want to reimplement checking types passed into methods. PHPStan itself already does this, and contains years of edge case fixes. But it's possible that the logic (https://github.com/phpstan/phpstan-src/blob/1.9.x/src/Rules/FunctionCallParametersCheck.php) isn't reusable the best way so maybe first we need to refactor it.

@villfa
Copy link
Contributor

villfa commented Dec 22, 2022

It may be important to note that @dataProvider can be used in combination with @depends, and it affects the tests signature, see documentation.

@mvorisek
Copy link

mvorisek commented Aug 4, 2023

  1. Data providers usually don't come with precise typehints. So instead of comparing the return type of a data provider against the test case method signature, I'd like to inspect return, yield, and yield from in data providers against the methods that expects them.

I agree. With more and more complex method types, sometimes even conditional, it makes sense to actually not have any return type above data provider methods and infer the type from what the tests that require them expect.

@stof
Copy link

stof commented Nov 29, 2023

Well, I would say that if the data provider method defines its return type, it should be used by the check instead of inferring it (as other rules will validate that the returned values match this type). But inferring it automatically when the type is not defined (or is just iterable or array without specifying the iterable value type), inferring is a good idea.

@mvorisek
Copy link

Well, I would say that if the data provider method defines its return type, it should be used by the check instead of inferring it (as other rules will validate that the returned values match this type). But inferring it automatically when the type is not defined (or is just iterable or array without specifying the iterable value type), inferring is a good idea.

Infered/accepting type (defined by accepting test method) should be always used.

However, data provider type might be narrower, the infered/accepting type should be intersected with this type.

@staabm
Copy link
Contributor Author

staabm commented Feb 17, 2024

I will re-evaluate based on the given feedback and start a new PR if I have something to review

@staabm staabm closed this Feb 17, 2024
@staabm staabm deleted the data-provider-types branch February 17, 2024 10:09
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.

typecheck dataproviders
5 participants