sirbrillig / phpcs-variable-analysis

Find undefined and unused variables with the PHP Codesniffer static analysis tool.
Other
136 stars 14 forks source link

Add support for enum methods #289

Closed sirbrillig closed 1 year ago

sirbrillig commented 1 year ago

This adds support for enum methods to behave like class methods.

Fixes https://github.com/sirbrillig/phpcs-variable-analysis/issues/288

sirbrillig commented 1 year ago

🤔 This will need a way to identify enum scopes without T_ENUM in order to work prior to PHP 7.3 (enums were introduced in PHP 8 but it appears that T_ENUM works in 7.3 🤷 ).

sirbrillig commented 1 year ago

These test failures are really hard to debug. I consistently get failures for PHP 7 and 5.6, but when I run those same tests locally using those versions, everything passes.

Screenshot 2023-03-11 at 6 37 41 PM Screenshot 2023-03-11 at 6 38 31 PM

Even running the sniff itself works as expected (the only error should be on line 33):

Screenshot 2023-03-11 at 6 41 09 PM Screenshot 2023-03-11 at 6 41 30 PM

So why does it fail in Github Actions?

sirbrillig commented 1 year ago

🤔 Ah, it may be something to do with how php or phpcs is installed... T_ENUM is somehow defined on my local machine even for php 5.6 which doesn't seem possible. I have to refactor how we detect classes so it can work for enums without T_ENUM entirely. Still difficult since I can't examine the failing code locally 👎

sirbrillig commented 1 year ago

I figured out how to reproduce the bug! I have to explicitly downgrade phpcs to 3.5.6. I think the issue is that in that version of phpcs, the enum does not get added to the conditions array at all (in fact, it looks like it gets confused by the syntax and adds things like the unrelated case statements to the conditions array). Now maybe I'll get somewhere.

jrfnl commented 1 year ago

PHPCS 3.5.6 was released in August 2020, way before the idea of Enums ever made it into a PHP RFC,

Support for enums was added in PHPCS 3.7.0.

sirbrillig commented 1 year ago

Ah hah! I think I've got it now. The only failing tests now are the coverage reports which I don't think are related to this PR:

Screenshot 2023-03-11 at 7 58 32 PM
sirbrillig commented 1 year ago

💥 Fixed. I've disabled code coverage for now until it can be fixed but that's outside the scope of this PR.

jrfnl commented 1 year ago

Hang on, I actually have a commit ready to fix that code coverage for you. Give me a moment to send you a PR.