Skip to content

Detect dead enum cases#197

Merged
janedbal merged 22 commits intomasterfrom
enums
Jul 18, 2025
Merged

Detect dead enum cases#197
janedbal merged 22 commits intomasterfrom
enums

Conversation

@janedbal
Copy link
Copy Markdown
Member

@janedbal janedbal commented Apr 4, 2025

Enable by:

parameters:
    shipmonkDeadCode:
        detect:
            deadEnumCases: true

@ruudk
Copy link
Copy Markdown
Contributor

ruudk commented Apr 4, 2025

Just my 2 cents: I'm not sure this is a good idea. I once made a PHPStan rule to detect them. And most of the time, our enums are mapped to database state. Not every case is tested. So it will produce lots of errors. I couldn't find anything worth deleting.

@janedbal
Copy link
Copy Markdown
Member Author

janedbal commented Apr 4, 2025

@ruudk Thx, I can imagine such problems, but you can always disable the analysis:

ignoreErrors:
   - identifier: shipmonk.deadEnumCase

And some codebases may benefit from this. Also, those mapped to database can be easily detected in custom usage provider (assuming Doctrine):

#[Column(type: Types::STRING, enumType: InvoiceStatus::class]
private InvoiceStatus $status;

@ruudk
Copy link
Copy Markdown
Contributor

ruudk commented Apr 4, 2025

I just realized that we should indeed create a usage detector, great 😁

@ruudk
Copy link
Copy Markdown
Contributor

ruudk commented Jun 26, 2025

I just had an idea, whenever from or tryFrom is called on an enum, we should mark the enum as not dead.

Together with the fact that it's used inside #[Column(enumType: TheEnum::type))

@janedbal
Copy link
Copy Markdown
Member Author

janedbal commented Jun 26, 2025

I just had an idea, whenever from or tryFrom is called on an enum, we should mark the enum as not dead.

Together with the fact that it's used inside #[Column(enumType: TheEnum::type))

Both those are already implemented. The only issue with this being enabled by default are typically API input objects, where enums are typically deserialized into the objects. Those cannot be easily natively supported and each app need to add own provider.

Basically, this PR is done and working (I even fully used it in our huge codebase), but I'm still thinking it might be disabled by default.

@janedbal janedbal marked this pull request as ready for review July 17, 2025 11:12
@janedbal janedbal merged commit 58ef3d3 into master Jul 18, 2025
33 checks passed
@janedbal janedbal deleted the enums branch July 18, 2025 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants