Skip to content

Do not treat lines spanned by attributes as executable#1149

Merged
sebastianbergmann merged 1 commit intosebastianbergmann:12.5from
Slamdunk:skip_attributes
Apr 13, 2026
Merged

Do not treat lines spanned by attributes as executable#1149
sebastianbergmann merged 1 commit intosebastianbergmann:12.5from
Slamdunk:skip_attributes

Conversation

@Slamdunk
Copy link
Copy Markdown
Contributor

Fix #1148

Beware the targeted branch is 12.5, so releases are needed also for 13 and 14 branches.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.21%. Comparing base (e70f9f8) to head (22adabf).
⚠️ Report is 1 commits behind head on 12.5.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               12.5    #1149   +/-   ##
=========================================
  Coverage     88.21%   88.21%           
- Complexity     1400     1401    +1     
=========================================
  Files           105      105           
  Lines          4718     4719    +1     
=========================================
+ Hits           4162     4163    +1     
  Misses          556      556           

☔ 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

The proposed changes mark every line spanned by an attribute as non-executable. This makes sense to me and I will merge these changes.

This does not really explain the --processes=1 vs --processes=16 discrepancy, though. That smells like a static-analysis cache / merge-path difference where single-process likely hit a cached/short-circuit path that skipped attributes, while parallel runs went through the visitor. Once attributes are unconditionally unset, both paths converge, so the user-visible bug should disappear even though the underlying merging asymmetry isn't touched.

@sebastianbergmann sebastianbergmann merged commit 122ccfa into sebastianbergmann:12.5 Apr 13, 2026
20 checks passed
@sebastianbergmann sebastianbergmann changed the title Executable lines: skip Attributes Do not treat lines spanned by attributes as executable Apr 13, 2026
@sebastianbergmann
Copy link
Copy Markdown
Owner

releases are needed also for 13 and 14 branches

I have released phpunit/php-code-coverage 12.5.5 and phpunit/php-code-coverage 14.1.1 with this fix, but there will be no more releases for the 13.x version series.

@Slamdunk Slamdunk deleted the skip_attributes branch April 13, 2026 17:53
@Slamdunk
Copy link
Copy Markdown
Contributor Author

This does not really explain the --processes=1 vs --processes=16 discrepancy, though. That smells like a static-analysis cache / merge-path difference where single-process likely hit a cached/short-circuit path that skipped attributes, while parallel runs went through the visitor. Once attributes are unconditionally unset, both paths converge, so the user-visible bug should disappear even though the underlying merging asymmetry isn't touched.

Yeah: one process lets PHP decide the executable lines of MyClassWithAttributes because it uses it, while the other process has to leverage ExecutableLinesFindingVisitor because it doesn't use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants