Skip to content

fix: PhpUnitAttributesFixer - convert correctly version constraint#8439

Merged
kubawerlos merged 3 commits intoPHP-CS-Fixer:masterfrom
6b7562617765726c6f73:fix_PhpUnitAttributesFixer
Feb 15, 2025
Merged

fix: PhpUnitAttributesFixer - convert correctly version constraint#8439
kubawerlos merged 3 commits intoPHP-CS-Fixer:masterfrom
6b7562617765726c6f73:fix_PhpUnitAttributesFixer

Conversation

@kubawerlos
Copy link
Copy Markdown
Member

@kubawerlos kubawerlos commented Feb 15, 2025

To test different/more cases:

  1. Make sure you use PHP 8.
  2. Checkout out the branch with the fix.
  3. Run composer update -> PHPUnit 11 will be installed which supports both - annotations and attributes.
  4. Create file generate.php:
<?php
const CASES = [
    '5',
    '5.3',
    '5.3.7',
    '8',
    '8.3',
    '8.4',
    PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.(PHP_RELEASE_VERSION - 1),
    PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.PHP_RELEASE_VERSION,
    PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.(PHP_RELEASE_VERSION + 1),
    '8.5',
    '5|6',
    '5|8',
    '5||8',
    '5|'.PHP_MAJOR_VERSION.'.'.(PHP_RELEASE_VERSION - 1),
    '5|'.PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION,
    '5|'.PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.(PHP_RELEASE_VERSION - 1),
    '5|'.PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.PHP_RELEASE_VERSION,
    '5|'.PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.(PHP_RELEASE_VERSION + 1),
];

$php = "<?php\nclass FooTest extends PHPUnit\\Framework\\TestCase\n{";
foreach (CASES as $index => $case) {
    $php .= sprintf(
        <<<'PHP'

                /**
                 * @requires PHP %s
                 */
                public function test%d(): void
                {
                    self::assertTrue(true);
                }

            PHP,
        $case,
        $index
    );
}
$php .= "}\n";

file_put_contents('Foo1Test.php', $php);
  1. Update CASES to what you want verify.
  2. Run these:
php generate.php
cp Foo1Test.php Foo2Test.php
PHP_CS_FIXER_ENFORCE_CACHE=0 php php-cs-fixer fix Foo1Test.php --allow-risky=yes --rules=psr_autoloading
PHP_CS_FIXER_ENFORCE_CACHE=0 php php-cs-fixer fix Foo2Test.php --allow-risky=yes --rules=psr_autoloading
PHP_CS_FIXER_ENFORCE_CACHE=0 php php-cs-fixer fix Foo2Test.php --rules=php_unit_attributes
vendor/bin/phpunit Foo1Test.php --order-by=default --testdox-text=.result1.txt | tee log1.log
vendor/bin/phpunit Foo2Test.php --order-by=default --testdox-text=.result2.txt | tee log2.log
  1. Compare log1.log (annotations) with log2.log (attributes) to see if the same tests were skipped.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 15, 2025 •
edited
Loading

Coverage Status

coverage: 94.952%. remained the same
when pulling 2467b07 on 6b7562617765726c6f73:fix_PhpUnitAttributesFixer
into 6c56cf8 on PHP-CS-Fixer:master.

@kubawerlos kubawerlos enabled auto-merge (squash) February 15, 2025 13:06
kubawerlos
kubawerlos commented Feb 15, 2025
View reviewed changes
Comment thread src/Fixer/PhpUnit/PhpUnitAttributesFixer.php
return self::createAttributeTokens($tokens, $index, $attributeName, ...$attributeTokens);
}

private static function fixVersionConstraint(string $version): string
Copy link
Copy Markdown
Member Author

@kubawerlos kubawerlos Feb 15, 2025

Choose a reason for hiding this comment

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

@sebastianbergmann is this how version constraint from annotation should be converted for attributes? The argument here is what is in annotation (e.g. @requires PHP 8.2) and the returned value is what will be in the attribute (e.g. #[RequiresPhp('>= 8.2')].

Copy link
Copy Markdown
Member

@keradus keradus Feb 15, 2025

Choose a reason for hiding this comment

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

overall yes, if there is no operator, it is defaulted to >=
ref https://github.com/sebastianbergmann/phpunit/blob/8.5.12/src/Util/Test.php#L161

keradus
keradus reviewed Feb 15, 2025
View reviewed changes
Comment thread src/Fixer/PhpUnit/PhpUnitAttributesFixer.php Outdated

private static function fixVersionConstraint(string $version): string
{
if (Preg::match('/^[\d\.]+$/', $version)) {
Copy link
Copy Markdown
Member

@keradus keradus Feb 15, 2025

Choose a reason for hiding this comment

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

we shall cover non-stable releases too, like PHPUnit does, eg 5.4.0-alpha1
ref https://github.com/sebastianbergmann/phpunit/blob/8.5.12/src/Util/Annotation/DocBlock.php#L33

keradus
keradus reviewed Feb 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@keradus keradus left a comment

Choose a reason for hiding this comment

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

overall almost 👍🏻 , thanks for working on it.
i found one failing case

@keradus keradus mentioned this pull request Feb 15, 2025
Closed
@kubawerlos kubawerlos merged commit de9c8c6 into PHP-CS-Fixer:master Feb 15, 2025
@kubawerlos kubawerlos deleted the fix_PhpUnitAttributesFixer branch February 15, 2025 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@keradus keradus keradus approved these changes

Assignees

No one assigned

Labels

kind/bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kubawerlos @coveralls @keradus