Closed nederdirk closed 3 years ago
Hi @dereuromark, thank you for your review. I implemented your suggestion!
HI @dereuromark, I think this is done. What's the best procedure to get this reviewed and merged?
HI @dereuromark, I think this is done. What's the best procedure to get this reviewed and merged?
Oh, if it helps, I am happy to approve the changes. Though I think it still does not handle null values correctly. Theses statements would be flagged, though they are perfectly valid:
->filterByStringColumn(null)
->filterByStringColumn()
->filterByBoolColumn(null)
->filterByBoolColumn()
->filterByBoolColumn(null, Criteria::ISNULL)
->filterByBoolColumn(null, Criteria::ISNOTNULL)
and also
->filterByStringColumn(['valA', 'valB'])
Am I missing something here?
Shall we move forward? How can we resolve the open issues regarding null?
@nederdirk Regarding the null value handling, can we check and possibly add those in the annotations as requested?
@nederdirk Regarding the null value handling, can we check and possibly add those in the annotations as requested?
I have tried to come up with a solution, but couldn't get anything to work properly. I think this PR can better be closed, and we'll just ignore this class of psalm-warnings in our code-base
The generated
filterByXXX
functions actually allow for arrays to be passed as a first parameter, in the case when the comparison operator isIN
orNOT IN
. This patch extends the phpdoc annotations to allowtype|type[]
for these functions, and an even more specific@psalm-param
annotation to projects using Psalm will have even better type checking.