rectorphp / rector-phpunit

Rector upgrade rules for PHPUnit
http://getrector.com
MIT License
62 stars 47 forks source link

Add PropertyExistsWithoutAssertRector #158

Closed jrjohnson closed 1 year ago

jrjohnson commented 1 year ago

Removes the now deprecated property/method exists PHPUnit assertions and replaces them with plain PHP equivalents. This is the inverse of AssertPropertyExistsRector.php to account for their removal in PHPUnit v10.

TomasVotruba commented 1 year ago

I'll look into this, thank you :+1:

TomasVotruba commented 1 year ago

Rebased and merged in https://github.com/rectorphp/rector-phpunit/pull/202

eerison commented 12 months ago

Hey @TomasVotruba

I guess this rule should be introduced in phpunit 9 rules instead of 10, don't you?

https://github.com/rectorphp/rector-phpunit/pull/202/files#diff-1d1786275faa487b46d3179a731083128edc0bb6238271e77c70c5c0ccf1881e

TomasVotruba commented 12 months ago

Hey, the PHPUnit 10 is fine here as that's where the change happens. See milestone in: https://github.com/sebastianbergmann/phpunit/issues/4601

The idea is that after running PHPUNIT_100 set, the code should run on PHPUnit 10.

eerison commented 12 months ago

But I can not run PHPUNIT_100 using phpunit 9, and it is deprecated in version 9. IMO it could be in version 9, it is the version where it was deprecated.

my pipelines are failing when I add deprecated methods :(

the same thing for https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_overview.md#addprophecytraitrector

eerison commented 12 months ago

well I made this work adding

    $rectorConfig->rule(PropertyExistsWithoutAssertRector::class);
    $rectorConfig->rule(AddProphecyTraitRector::class);

But if it's in version 9, I could avoid search about those 2 rules.

TomasVotruba commented 12 months ago

You can run PHPUNIT_100 set on any PHPUnit version, then upgrade to PHPUnit 10 in your composer.

What exact error are you getting?

eerison commented 12 months ago

I saw that it is applying php 8 code, and I'm still on php 7.4 :'(

                return match ($matcher->numberOfInvocations()) {
                    1 => $expectedSleepValues,
                };
eerison commented 12 months ago

before

    ->withConsecutive(...$expectedSleepValues);

after

            ->willReturnCallback(function () use ($matcher) {
                return match ($matcher->numberOfInvocations()) {
                    1 => $expectedSleepValues,
                };
            });
eerison commented 12 months ago

Ok it's the issue

this rule is applying php 8 code

Should we downgrade this code to php 7 ?

TomasVotruba commented 12 months ago

That's the pain point. Perfect! This rule should run only on PHP 8 indeed.

Should we downgrade this code to php 7 ?

That's not necessary. The withConsecutive() still runs on PHPUnit 9, and PHPUnit 10 already requires on PHP 8.1, so it's supported there.

Instead, this rule should implement MinPhpVersionInterface with PHP 8. Could you add it?

eerison commented 12 months ago

That's the pain point. Perfect! This rule should run only on PHP 8 indeed.

Should we downgrade this code to php 7 ?

That's not necessary. The withConsecutive() still runs on PHPUnit 9, and PHPUnit 10 already requires on PHP 8.1, so it's supported there.

Instead, this rule should implement MinPhpVersionInterface with PHP 8. Could you add it?

Yep, I will open a PR ;)