search

phpstan / phpstan-nette

Nette Framework class reflection extension for PHPStan & framework-specific rules
MIT License
100 stars 35 forks source link

False positive on 0.12.15? #63

Closed dakorpar closed 3 years ago

dakorpar commented 3 years ago

image see here: https://github.com/contributte/datagrid/pull/957/checks?check_run_id=2015573206

code: image

I don't think this should be noted as error here...

ondrejmirtes commented 3 years ago

Yeah, this is an interesting side-effect of this change... you can get rid of the error by also having @param positive-int|0 $offset above the method. You can also use @phpstan-param if you don't want to confuse IDE etc.

I'm gonna keep it open so we can decide if the stub change was a mistake or not... /cc @lulco

lulco commented 3 years ago

Yes, that’s exactly kind of errors which should be found by that change. You don’t have any check if offset / limit is positive integer or zero. So the application will fail at this point. I understand it could be annoying (like almost all errors found by phpstan :)) but imho it is good to handle it another way like crashing app.

@ondrejmirtes feel free to remove it, if it is not compatible with the base ideas of phpstan

dakorpar commented 3 years ago

@lulco then again we're going to far and nette code returns int here so it fails... image

ondrejmirtes commented 3 years ago

I guess we could add @return positive-int|0 to paginator methods as well.

dakorpar commented 3 years ago

@ondrejmirtes @lulco any ETA on this?

ondrejmirtes commented 3 years ago

If you send a PR that adds a stub for those Paginator methods, I’ll release it right away.

dakorpar commented 3 years ago

@ondrejmirtes PR is there

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.