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

feat: introduce PhpUnitAttributesFixer #7831

Merged

Conversation

kubawerlos
Copy link
Contributor

@kubawerlos kubawerlos commented Feb 8, 2024

#7832 and #7833 are reverted here - it was a (naive) thinking, a better approach is NOT to migrate to attributes that are not supported in a given scope (like @large and @medum are fine on method level, but #[Large] and #[Medium] are not allowed ion method level).

@coveralls
Copy link

coveralls commented Feb 8, 2024

Coverage Status

coverage: 96.048% (+0.04%) from 96.013%
when pulling ab8f00c on 6b7562617765726c6f73:feat_php_unit_attributes
into 8713dfd on PHP-CS-Fixer:master.

@kubawerlos kubawerlos force-pushed the feat_php_unit_attributes branch 2 times, most recently from 8c12a44 to 218f687 Compare February 8, 2024 15:19
Copy link
Member

@Wirone Wirone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick feedback after taking a look 😉. More when it comes out of the draft.

src/Fixer/PhpUnit/PhpUnitAttributesFixer.php Outdated Show resolved Hide resolved
@kubawerlos kubawerlos force-pushed the feat_php_unit_attributes branch 7 times, most recently from 5b173ad to 537eb1b Compare February 9, 2024 21:21
@kubawerlos kubawerlos marked this pull request as ready for review February 9, 2024 21:24
private static function fixWithSingleStringValue(Tokens $tokens, int $index, Annotation $annotation): array
{
Preg::match(
sprintf('/@%s\s+(.*\S)(?:\R|\s*\*+\\/$)/', $annotation->getTag()->getName()),
Copy link
Contributor

@mvorisek mvorisek Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the regex would be good to be deduplicated
  • link to a parsing phpunit source would be good so the parsing can be checked & documented if 1:1 with native phpunit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the regex would be good to be deduplicated

done

  • link to a parsing phpunit source would be good so the parsing can be checked if 1:1 with native phpunit

I'd rather not, for 2 reasons:

  1. it is not a single place:
  1. let's focus on providing good tests, so they keep the fixer working properly

@Wirone Wirone self-requested a review March 11, 2024 23:06
@Wirone Wirone added topic/phpdoc PHPDoc tags, Doctrine Annotations etc. topic/phpunit Fixers related to PHPUnit topic/attributes PHP8 native attributes kind/feature labels Mar 11, 2024
Copy link
Member

@Wirone Wirone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @kubawerlos 👏. Personally I would drop handle from test cases' names because when you consider --testdox output it looks weird (e.g. "Fix with handle·After"). Actually I would switch to testdox entirely, but it's a separate story 😉. Anyway, only one minor comment from me, please merge on your own with or without the fix.

tests/Fixer/PhpUnit/PhpUnitAttributesFixerTest.php Outdated Show resolved Hide resolved
@kubawerlos kubawerlos enabled auto-merge (squash) April 15, 2024 14:56
@kubawerlos kubawerlos merged commit 6a69160 into PHP-CS-Fixer:master Apr 15, 2024
27 checks passed
@kubawerlos kubawerlos deleted the feat_php_unit_attributes branch April 15, 2024 15:05
@stof
Copy link
Contributor

stof commented Apr 17, 2024

it would be great to have an option to add the attributes without removing the annotation (which also requires checking if the attribute is already there to avoid "fixing" it multiple times). When you need to support both PHPUnit 11 (to support the latest PHPUnit version) and PHPUnit 9.6 (to support older PHP versions), the way to go is to define both the attributes (for PHPUnit 10+) and the annotation (used by PHPUnit 9 and ignored in 10+ when the attributes are present).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature topic/attributes PHP8 native attributes topic/phpdoc PHPDoc tags, Doctrine Annotations etc. topic/phpunit Fixers related to PHPUnit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants