Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AnnotationsToAttributes] Fix @covers qualified::method conversion #451

Merged

Conversation

andrewnicols
Copy link
Contributor

@andrewnicols andrewnicols commented Jan 29, 2025

The PHPUnit 9.6 @covers annotation supports specification of methods:
https://docs.phpunit.de/en/9.6/annotations.html#covers

That is:

/**
 * @covers \someClass::someMethod
 */

The CoversAnnotationWithValueToAttributeRector conversion is currently unaware of this and converting it to a CoversClass attribute, that is:

#[\PHPUnit\Framework\Attributes\CoversClass(\someClass::someMethod)

This is, of course, a syntax error. It should create a CoversMethod attribute:

#[\PHPUnit\Framework\Attributes\CoversClass(\someClass::class, 'someMethod')

This patch changes the behaviour to look for a :: in the annotation and, if found, grab the class and use the CoversMethod Attribute with the class and method name. The behaviour for an annotation that starts with :: is unaffected.

@samsonasik
Copy link
Member

Please run:

vendor/bin/rector && composer fix-cs

to fix CI

@andrewnicols
Copy link
Contributor Author

Apologies @samsonasik, I thought I'd created this as a draft request. I've cleaned up coding style now.

@samsonasik
Copy link
Member

did you forget git push?

@andrewnicols andrewnicols force-pushed the coversAnnotationClassMethodFailure branch from 6e74eb8 to cb2f9eb Compare January 29, 2025 06:47
@andrewnicols
Copy link
Contributor Author

Indeed I did. It's one of 'those' days.

@andrewnicols andrewnicols marked this pull request as draft January 29, 2025 06:50
@andrewnicols andrewnicols force-pushed the coversAnnotationClassMethodFailure branch from cb2f9eb to 928e6e3 Compare January 29, 2025 06:51
@andrewnicols andrewnicols marked this pull request as ready for review January 29, 2025 06:52
@samsonasik
Copy link
Member

I don't follow latest Covers* in PHPUnit for class method, last time old days, it was not exists, or I was looking to different things.

If you already verify it, let's give it a try :)

@samsonasik samsonasik merged commit e7e84da into rectorphp:main Jan 29, 2025
6 checks passed
@samsonasik
Copy link
Member

Thank you @andrewnicols

@andrewnicols
Copy link
Contributor Author

It looks like the CoversMethod attribute was added in PHPUnit 11. It was not present in PHPUnit 10, but there is no alternative in that version of PHPUnit.

Given there is no ability to specify this in PHPUnit 10, and removing the method altogether would cause data loss that is the loss of the coverage information) there are some options:

  • Use CoversMethod on it's own - this will work for PHPUnit 11, but for PHPUnit 10 the coverage data will be 'lost'
  • Use CoversClass on its own always - this will work for PHPUnit 10+, but may give more specific coverage data than desired
  • Use both CoversMethod and CoversClass - this is a bit pointless because the Method coveage is already covered by the class.

@samsonasik
Copy link
Member

Ugh, what a mess, it seems ok with coverage loss, as unknown class attribute seems not error, see https://3v4l.org/iqNL1 except you have other option

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