antonioribeiro / google2fa

A One Time Password Authentication package, compatible with Google Authenticator.
MIT License
1.84k stars 200 forks source link

PHPStan level 9 with bleeding edge #197

Closed spaze closed 1 month ago

spaze commented 7 months ago

Update PHPStan level to 9/max with bleeding edge. You can't go any higher :-) #172 inspired me to do this, to make sure the types are correct throughout the lib.

Native types could be used instead of docblocks if support for older PHP version is dropped, but first I wanted to make sure the lib is on a highest possible PHPStan level to make adding native types a bit safer. Can add them in a next PR once this is merged for example.

This branch is based on #196 branch to make tests run on all PHP versions, probably should be rebased before merging, can take care of that.

I'm not sure what to do with the failing Style CI test that wants me to update a preexisting code like this:

- exit();
+ exit;

I can update it if it looks like what's expected.

antonioribeiro commented 1 month ago

Hey @spaze, I like the idea of having this done. I'm going PHPStan max on whenever I can, but I believe we should go a step further on this one: what about making it fully strict typed?

Thanks for your hard work on this PR! ❤️

mfn commented 1 month ago

Not OP and I ❤️ strict types too, but they have potential to break uses of the library so should be considered carefully & only with a major version bump (which I support :) )

antonioribeiro commented 1 month ago

Yes, this would be version 9, @mfn, and we would have to drop support for a bunch of PHP versions, unfortunately.

spaze commented 1 month ago

I'm all for native and strict types (both!) and I see this PR is a prerequisite for doing it.

Seems like dropping 7.1-7.3 (current usage 1.5% combined) support should not be that painful (and even the tests I've added in #196 do not run on 7.1), possibly even 7.4 (3.6%) and 8.0 (5%). Here's the version breakdown of the 8.x version, from https://packagist.org/packages/pragmarx/google2fa/php-stats#8:

image

I can update this PR and would be nice to merge the others as well :-) Let me know if you need a co-maintainer.

antonioribeiro commented 1 month ago

Let's try to keep 8.0 and above for 9.0 and drop support for the 7.4 and prior as 7.4 hit EOL in 2022. I can still support security issues, but not the evolution.

spaze commented 1 month ago

Got it. Do you want to create the 9.x branch so I can target it with this PR?

Would you prefer to add the native types in this PR too?

antonioribeiro commented 1 month ago

Branch 9.x created @spaze. I believe you can merge https://github.com/antonioribeiro/google2fa/pull/196 into this one, also merge 8.x back to spaze:spaze/phpstan++ to fix the conflicts with the changes that I merged recently

antonioribeiro commented 1 month ago

I just disabled Style-CI too. I will try to move to something else, as I currently prefer Prettier over any other formatter out there...

spaze commented 1 month ago

Rebased and this now targets the 9.x branch