-
-
Notifications
You must be signed in to change notification settings - Fork 23
Infer default-result type when sql queries cannot be inferred #373
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
Conversation
| $stmt = $pdo->query($q); | ||
| assertType('PDOStatement<array<bool|float|int|string>>', $stmt); | ||
| foreach ($stmt as $row) { | ||
| assertType('array<bool|float|int|string>', $row); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Seldaek could I get your opinion on this one?
is it usefull to get a type like bool|float|int|string in cases where phpstan-dba cannot know the actual db query beeing used?
before this PR this was only a PDOStatement without any information about the types.
with this PR we now get types dependent on the used fetch-type - if available - even if the sql query cannot be inferred.
see the other tests below in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also: I am not sure, whether a bool can actually be returned from PDO, or whether its always encoded as a int 0/1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO the PR makes sense, vague types are better than mixed.
Raw PDO I don't think it can do bool, doctrine I think yes (maybe depending on API used).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I might have missed null here.. need to double check
closes #372