BabDev / Pagerfanta

Pagination library for PHP applications with support for several data providers
Other
381 stars 170 forks source link

Please remove positive-int typehint from `Pagerfanta::setCurrentPage` #40

Closed ruudk closed 2 years ago

ruudk commented 2 years ago

By adding the positive-int typehint to Pagerfanta::setCurrentPage it only becomes annoying for consumers of the library. I understand that it's nice to add this but I think it should be just asserted in the method.

Currently PHPStan reports 20 errors on our project:

Parameter #1 $currentPage of method Pagerfanta\Pagerfanta<Entity>::setCurrentPage() expects int<1, max>, int given.

I have to add this:

Assert::greaterThanEq($page, 1);

in all 20 places where I call:

$pagerfanta->setCurrentPage($page);

Would it be possible to reconsider this?

mbabker commented 2 years ago

That's fair. Done in https://github.com/BabDev/Pagerfanta/commit/1f1345306c2adca8c1cf857298aae677eb9e2a56

ruudk commented 2 years ago

Thank you 💙 could you please tag a new release ?

mbabker commented 2 years ago

Consider it done!