Skip to content

CoversClass does not transitively target traits used by enumerations#1145

Closed
no-simpler wants to merge 1 commit intosebastianbergmann:mainfrom
no-simpler:fix/enum-trait-transitive-coverage
Closed

CoversClass does not transitively target traits used by enumerations#1145
no-simpler wants to merge 1 commit intosebastianbergmann:mainfrom
no-simpler:fix/enum-trait-transitive-coverage

Conversation

@no-simpler
Copy link
Copy Markdown
Contributor

Problem

When a class uses a trait, #[CoversClass(TheClass::class)] transitively whitelists the trait's lines for coverage. When an enum uses the same trait, the trait's lines are not whitelisted — PHPUnit flags the test as risky:

This test executed code that is not listed as code to be covered or used: …

This forces authors to add #[UsesTrait] or #[CoversTrait] to enum tests, which is inconsistent with the class behavior and creates friction with static analysis tools that don't accept those attributes.

A minimal reproduction is available at https://github.com/no-simpler/phpunit-psalm-trit-catch22 (rows 1 vs 4 in its test matrix).

Root cause

CodeUnitFindingVisitor::leaveNode() resolves trait usages only for Class_ and Trait_ nodes. Enum_ extends ClassLike (not Class_), so it is excluded by the early-return gate. Enums are correctly discovered in enterNode() via processClass(), but their trait lists are never populated.

Changes

Three one-line fixes in CodeUnitFindingVisitor:

  1. leaveNode() — include Enum_ in the gate so enums proceed to trait resolution
  2. postProcessClassOrTrait() signature — widen the type union to accept Enum_
  3. postProcessClassOrTrait() body — route Enum_ into the class branch (enums are stored in $this->classes)

Plus a regression test (testHandlesEnumUsingTrait) and fixture file mirroring the existing testHandlesTraitUsingTrait pattern.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.97%. Comparing base (8d03d7a) to head (ecbea80).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1145   +/-   ##
=========================================
  Coverage     96.97%   96.97%           
- Complexity     1480     1482    +2     
=========================================
  Files           110      110           
  Lines          5057     5057           
=========================================
  Hits           4904     4904           
  Misses          153      153           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sebastianbergmann
Copy link
Copy Markdown
Owner

As a bug fix, this should target the 12.5 branch. Thank you!

@sebastianbergmann sebastianbergmann changed the title Fix: CoversClass does not transitively whitelist traits used by enums CoversClass does not transitively target traits used by enumerations Apr 2, 2026
@no-simpler no-simpler changed the base branch from main to 12.5 April 2, 2026 13:40
@sebastianbergmann
Copy link
Copy Markdown
Owner

This branch cannot be rebased due to conflicts, likely because something went wrong when changing the target branch. Might be easier to close this and send a fresh pull request against 12.5.

@no-simpler
Copy link
Copy Markdown
Contributor Author

I switched target branch in UI and immediately felt stupid. I will re-create the change on correct branch when I get back to my desk.

@sebastianbergmann
Copy link
Copy Markdown
Owner

I switched target branch in UI and immediately felt stupid. I will re-create the change on correct branch when I get back to my desk.

No worries :) Thank you for working on this.

@no-simpler
Copy link
Copy Markdown
Contributor Author

Superseded by !1147.

@no-simpler no-simpler closed this Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants