is there any way to add a blank like between trait and method?

this rule https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_overview.md#addprophecytraitrector

Should apply

+    use ProphecyTrait;
+

But it is not adding this blank line after trait.

Should it be handled here or other tool should do it like phpcs?

Hey!
As the title suggests I'd like a rector the can turn an @test annotation into a test prefix for the test function.

Example:

/**
 * @test
 */
public function onePlusOneIsTwo(): void {
    $this->assertSame(2, 1+1);
}

Should turn into

/**
 * @test
 */
public function testOnePlusOneIsTwo(): void {
    $this->assertSame(2, 1+1);
}

The @test should be preserved and removed with a different rector in my opinion to keep this rector simple.

Currently, this class refactors all occurences of cover annotations, as described in the example. However, method-based attributes are not supported and never will be (see sebastianbergmann/phpunit#5175 and sebastianbergmann/phpunit#4502).

Applying this rule to a testsuite that uses method-based annotations will now result into a broken tests suite (Attribute "PHPUnit\Framework\Attributes\CoversClass" cannot target method (allowed targets: class)).

I think there are two possible solutions:

• ignore all method-based annotations altogether, possibly with some warning message provided to users to notify them on this behavior
• move all the annotations to be class-based, as using multiple covers attributes on a class is allowed

See https://stackoverflow.com/a/75686291/1348344

I wonder why the PreferPHPUnitSelfCallRector is not part of any setlist?

Well I faced a new issue

this rule: https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_overview.md#testwithannotationtoattributerector, is using PHP attribute

I will check others functions that are still using php 8 and add MinPhpVersionInterface

running phpunit on 8a1a163 I get errors like on my mac

➜  rector-phpunit git:(main) vendor/bin/phpunit 
PHPUnit 10.1.0 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.18
Configuration: /Users/staabm/workspace/rector-phpunit/phpunit.xml


.............................F.F...............................  63 / 182 ( 34%)
............................................................... 126 / 182 ( 69%)
........................................................        182 / 182 (100%)

Time: 00:18.524, Memory: 1.80 GB

There were 2 failures:

1) Rector\PHPUnit\Tests\Rector\ClassMethod\CreateMockToAnonymousClassRector\CreateMockToAnonymousClassRectorTest::test with data set #0
assert($startLine > 0)

phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:189
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:261
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/BetterReflectionProvider.php:211
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ReflectionProvider/MemoizingReflectionProvider.php:59
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1102
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:744
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1473
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:567
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:531
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:331
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:374
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:227
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ChangedNodeScopeRefresher.php:95
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:367
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:260
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:200
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:91
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/PhpParser/NodeTraverser/RectorNodeTraverser.php:33
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor.php:41
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:71
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ApplicationFileProcessor.php:127
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:188
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:128
/Users/staabm/workspace/rector-phpunit/tests/Rector/ClassMethod/CreateMockToAnonymousClassRector/CreateMockToAnonymousClassRectorTest.php:16

2) Rector\PHPUnit\Tests\Rector\ClassMethod\CreateMockToAnonymousClassRector\CreateMockToAnonymousClassRectorTest::test with data set #2
assert($startLine > 0)

phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:189
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/ReflectionClass.php:261
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/BetterReflectionProvider.php:211
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ReflectionProvider/MemoizingReflectionProvider.php:59
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1102
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:744
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1473
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:567
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:531
phar:///Users/staabm/workspace/rector-phpunit/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:331
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:374
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:227
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ChangedNodeScopeRefresher.php:95
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:367
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Rector/AbstractRector.php:260
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:200
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:114
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:223
/Users/staabm/workspace/rector-phpunit/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php:91
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/PhpParser/NodeTraverser/RectorNodeTraverser.php:33
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor.php:41
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/FileProcessor/PhpFileProcessor.php:71
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/src/Application/ApplicationFileProcessor.php:127
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:188
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:154
/Users/staabm/workspace/rector-phpunit/vendor/rector/rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php:128
/Users/staabm/workspace/rector-phpunit/tests/Rector/ClassMethod/CreateMockToAnonymousClassRector/CreateMockToAnonymousClassRectorTest.php:16

FAILURES!
Tests: 182, Assertions: 183, Failures: 2.

 
➜  rector-phpunit git:(main) php -v
PHP 8.1.18 (cli) (built: Apr 12 2023 12:31:46) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.18, Copyright (c) Zend Technologies

does anyone has an idea whats going on?

See https://stackoverflow.com/questions/65075204/method-setmethods-is-deprecated-when-try-to-write-a-php-unit-test

There exists a rector rule called AddVoidReturnTypeWhereNoReturnRector. It would be nice if we could add to rector-phpunit to the PHPUNIT_CODE_QUALITY a rule to add this to all classes extending from PHPUnits TestCase class or any other class extending from PHPUnit TestClass. The AddVoidReturnTypeWhereNoReturnRector is currently final.

What do you think the best way is to implement this. Currently we can not extend from AddVoidReturnTypeWhereNoReturnRector to achieve this simple here. Another option could be that we extend the AddVoidReturnTypeWhereNoReturnRector Rule in rector itself to filter by a class / interface and apply the rule only for that kind of classes, not sure if that kind of things make sense as a "General" Rector feature. That all rules could be limited to specific classes extending or implementing from a specific interface?

The implementation of WithConsecutiveRector seems to be incorrect.
Before:
->withConsecutive( [1, 2], [3, 4] );
After:
->willReturnCallback(function () use ($matcher) { return match ($matcher->numberOfInvocations()) { 1 => [1, 2], 2 => [3, 4] };

The function withConsecutive checks if a function is called with specific parameter. The replacement function willReturnCallback only gets a function which determines the return value but doesn't checks the parameter. Furthermore the function returns the parameter.

I will create a PR with a (hopefully correct) solution.

Hello guys I want to build a rule to replace @testWith to dataProvider.

In case you have any rule that do something similar, please post here, it could help me to provide a rule for it ;)

I'm using Rector\PHPUnit\Set\PHPUnitSetList::PHPUNIT_100

Here is the basic hierarchy before the rector run:

namespace App\Test;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
abstract class AbstractWebTest extends WebTestCase

namespace App\Test\Controller;
use App\Test\AbstractWebTest;
abstract class AbstractControllerTest extends AbstractWebTest

namespace App\Test\Controller;
class ExampleControllerTest extends AbstractControllerTest

AbstractWebTest

BEFORE

namespace App\Test;

use App\A;
use App\B;
use App\C;
use App\D;
use DateTime;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

abstract class AbstractWebTest extends WebTestCase
{
    ...
}

AFTER

namespace App\Test;

use DateTime;

# why FQCN below instead of use statement?
abstract class AbstractWebTestCase extends \Symfony\Bundle\FrameworkBundle\Test\WebTestCase
{
    # why are these down here?
    # why a leading slash?
    # what happened to the App\C and App\D imports, which are now direct FQCN references in the code below?
    use \App\A;
    use \App\B;
}

AbstractControllerTest

BEFORE

namespace App\Test\Controller;

use App\Test\AbstractWebTest;

abstract class AbstractControllerTest extends AbstractWebTest

AFTER

namespace App\Test\Controller;

# while the class name suffix was updated from Test to TestCase, the AbstractWebTest was not updated to AbstractWebTestCase
abstract class AbstractControllerTestCase extends \Coe\Cee\Tests\Common\AbstractWebTest

ExampleControllerTest

This class was completely untouched by rector, when its extends clause should have been changed to reflect the new name of the child abstract (AbstractControllerTest --> AbstractControllerTestCase)

class ExampleControllerTest extends AbstractControllerTestCase

I'm currently seeing some "fighting" between php-cs-fixer adding @coversNothing, and then rector removing it and adding the annotation (again).

I can suppress @coversNothing at php-cs-fixer, but annotations shouldn't be doubled up.

Would add

#[PHPUnit\Framework\Attributes\CoversNothing]

ref: sebastianbergmann/phpunit#4502

Bug Report

Using the MigrateAtToConsecutiveExpectationsRector rule. If a mock was configured without calling the expects() method, an error would be threw.

Omitting expects() while configuring a mock is valid syntax.

Minimal PHP Code Causing Issue

<?php

namespace Rector\PHPUnit\Tests\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector\Fixture;

class Foo
{
    public function someMethod()
    {
    }
}
final class SkipMissingExpects extends \PHPUnit\Framework\TestCase
{
    protected $mock;

    protected function setUp(): void
    {
        parent::setUp();

        $this->mock = $this->createMock(Foo::class);
    }

    public function test(): void
    {
        $this->mock
            ->method('someMethod')
            ->willReturn(1);

        $mock = $this->createMock(Foo::class);

        $mock
            ->method('someOtherMethod')
            ->willReturn(1);
    }
}

return static function (ContainerConfigurator $containerConfigurator): void {
    $services = $containerConfigurator->services();

    $services->set(\Rector\PHPUnit\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector::class);
};

Actual Behaviour

 [ERROR] Could not process "..." file,
         due to:                                                                                                        
         "Argument 1 passed to Rector\PHPUnit\NodeAnalyzer\ExpectationAnalyzer::isValidExpectsCall() must be an instance
         of PhpParser\Node\Expr\MethodCall, instance of PhpParser\Node\Expr\PropertyFetch given, called in              
         vendor/rector/rector-prefixed/vendor/rector/rector-phpunit/src/NodeAnalyzer/ExpectationAnalyzer.php:49".

We have a big codebase in @sulu and lots of our testcases are not really analyzeable by phpstan as there are lot of false typehints for mocks (we have a very very big baseline). We are using prophecy for mocking. And there are different false typehints in our codebase:

Variant A (class but didnt define mock):

class MyClassTest extends TestCase {
    /**
     * @var MyClass
     */
    private $my;

    public function setUp(): void
    {
         $this->my = $this->prophesize(MyClass::class);
    }
}

Variant B (mock but didnt define class):

class MyClassTest extends TestCase {
    /**
     * @var ObjectProphecy
     */
    private $my;

    public function setUp(): void
    {
         $this->my = $this->prophesize(MyClass::class);
    }
}

Variant C (mock or class (is still false but works sometimes)):

class MyClassTest extends TestCase {
    /**
     * @var ObjectProphecy|MyClass
     */
    private $my;

    public function setUp(): void
    {
         $this->my = $this->prophesize(MyClass::class);
    }
}

Variant D (nothing defined):

class MyClassTest extends TestCase {
    private $my;

    public function setUp(): void
    {
         $this->my = $this->prophesize(MyClass::class);
    }
}

Expected would be ObjectProphecy Generic with the class:

class MyClassTest extends TestCase {
    /**
     * @var ObjectProphecy<MyClass>
     */
    private $my;

    public function setUp(): void
    {
         $this->my = $this->prophesize(MyClass::class);
    }
}

So I asked myself if rector could be used to provide a rule to analyze all $this->prophesize calls and set the correct type for this classes. Think it would be a great rule which could be shipped with rector via a CodeQuality ruleset. Any hints are welcome how to achieve this :)

1) tests/system/Email/EmailTest.php:138

    ---------- begin diff ----------
@@ @@

     public function testDestructDoesNotThrowException()
     {
-        $email = $this->getMockBuilder(Email::class)
-            ->disableOriginalConstructor()
-            ->onlyMethods(['sendCommand'])
-            ->getMock();
+        $email = $this->createMock(Email::class);
         $email->method('sendCommand')
             ->willThrowException(new ErrorException('SMTP Error.'));
    ----------- end diff -----------

Applied rules:
 * GetMockBuilderGetMockToCreateMockRector (https://github.com/lmc-eu/steward/pull/187/files#diff-c7e8c65e59b8b4ff8b54325814d4ba55L80)

But when I run the test, I got the error:

$ vendor/bin/phpunit tests/system/Email/EmailTest.php
PHPUnit 9.5.21 #StandWithUkraine

Runtime:       PHP 8.0.20
Configuration: .../CodeIgniter4/phpunit.xml

........E                                                           9 / 9 (100%)E                                                           9 / 9 (100%)

Time: 00:00.358, Memory: 14.00 MB

There was 1 error:

1) CodeIgniter\Email\EmailTest::testDestructDoesNotThrowException
PHPUnit\Framework\MockObject\MethodNameNotConfiguredException: Method name is not configured

ERRORS!
Tests: 9, Assertions: 20, Errors: 1.

Hi, we have our directory structure in our project like this:

• src
• tests
  • E2E
  • Functional
  • Unit

So for class App\Services\SomeService we have test with FQCN App\Tests\Unit\Services\SomeService, the Unit being what breaks the default TestClassNameResolver implementation.

Do you think it would be a good idea to allow the TestClassNameResolver to be replaced via configuration by custom implementation, or perhaps by a callback, or should we try to find a different solution?

Thanks!

Desired behavior:

+ #[CoversClass(SomeService::class)]
class SomeServiceTest extends TestCase
{
}

Actually, I've already implemented it as a custom rule as we need it in Infection. It works great and covered by tests

infection/infection@03da557#diff-8a5ee371c08d2d066c7dfe04dea926a679e3ff5fb35ff93a228aa2a5c46e3699R65

But I would like to integrate it into rector/rector-phpunit itself, as I'm pretty sure it will be useful for other teams that would like to automatically add #[CoversClass()] to their test classes and automatically guess source file used as a value of this attribute. This is pretty convenient, and existing tools, including PHP-CS-Fixer, doesn't help with it.

But before creating a PR, I would like to know if it's going to be accepted to not waste time.

Also, this issue will popup in a search so even if it's not going to be accepted, one will have a link to a custom implementation.

It seems setMethods on mockBuilder is deprecated in PHPUNIT 10, this rule is currently under the codestyle setlist, but it seems it should be in the phpunit10 setlist? Am i missing something?

sebastianbergmann/phpunit#3687

RemoveSetMethodsMethodCallRector::class, is the class im talking about.

I noticed that the most correct type hint for yield would be Generator but YieldDataProviderRector adds Iterator.
I think this is because of $classMethod->returnType = new FullyQualified('Iterator'); in YieldDataProviderRector.

In rector, AddReturnTypeDeclarationFromYieldsRector does work correctly and adds Generator.

It even worked in my test, if I remove the mentioned line which assigns Iterator to returnType and let rector run. It executes YieldDataProviderRector & AddReturnTypeDeclarationFromYieldsRector, which leads to function provideData(): Generator

Given this situation, I'm not sure what would be the correct fix. Looking forward to your help.

Rector 130b634b5e348554c3349ed3b3f52fa3d08bfb6e
PHPUnit 8.5

PHP Fatal error:  Declaration of Rector\Testing\PHPUnit\PlatformAgnosticAssertions::assertStringEqualsFile($expectedFile, $actualString, $message = ''): void must be compatible with PHPUnit\Framework\Assert::assertStringEqualsFile(string $expectedFile, string $actualString, string $message = '', bool $canonicalize = false, bool $ignoreCase = false): void in /var/www/html/vendor/rector/rector/packages/Testing/PHPUnit/AbstractTestCase.php on line 41

Consider test method and its related methods:

public function testSomething(): void
{
    $this->mockSomething(); // This makes assertion internally by using `MockObject::expects()`
    $this->runSomething(true);
}

private function mockSomething(): void
{
    $this->something // This is mock prepared in setUp()
        ->expects(self::once())
        ->method('handle');
}

Currently AddDoesNotPerformAssertionToNonAssertingTestRector adds @doesNotPerformAssertions annotation to such tests, which is not true. Method itself does not have assertion calls, but other method(s) called within test makes assertions with MockObject::expects().

Given the code:

    /**
     *
     * @testWith ["foo"]
     *           ["bar"]
     *           ["baz"]
     */
    public function testFoo(string $foo): void {}

Is currently refactored to:

    /**
     *
     *  #[TestWith('["Foo"]
     *       ["Bar"]
     *       ["Baz"]')]
     */
    public function testFoo(string $foo): void {}

Expected:

    /**
     *
     *  #[TestWith(['Foo'])
     *  #[TestWith(['Bar'])
     *  #[TestWith(['Baz'])
     */
    public function testFoo(string $foo): void {}

cf.
https://docs.phpunit.de/en/10.2/attributes.html#testwith

Bug Report

Using the MigrateAtToConsecutiveExpectationsRector rule. If a mock was configured in the setUp() method and stored as a property, the at() would be ignore and not refactored.

Minimal PHP Code Causing Issue

class Foo
{
    public function someMethod()
    {
    }
}
final class HandlePropertyFetch extends \PHPUnit\Framework\TestCase
{
    protected $mock;

    protected function setUp(): void
    {
        parent::setUp();

        $this->mock = $this->createMock(Foo::class);
    }

    public function test(): void
    {
        $this->mock
            ->expects($this->at(0))
            ->method('someMethod')
            ->willReturn(1);

        $this->mock
            ->expects($this->at(1))
            ->method('someMethod')
            ->willReturn(2);
    }
}

return static function (ContainerConfigurator $containerConfigurator): void {
    $services = $containerConfigurator->services();

    $services->set(\Rector\PHPUnit\Rector\ClassMethod\MigrateAtToConsecutiveExpectationsRector::class);
};

Actual Behaviour

Test is not changed.

Shouldn't this rule DataProviderArrayItemsNewlinedRector be placed in this repository instead of main repository?

in theory yes https://github.com/rectorphp/rector/blob/main/rules/CodingStyle/Rector/ClassMethod/DataProviderArrayItemsNewlinedRector.php#L96

😄

Mockery provides a MockeryTestCase base class that extends the default \PHPUnit\Framework\TestCase that better integrates mockery into PHPUnit. One part of this is adding its expectations to PHPUnit's assertion count.

Currently the AddDoesNotPerformAssertionToNonAssertingTestRector rector does not take this into consideration, resulting in the @doesNotPerformAssertions being added to methods that technically have no core PHPUnit assertions, but do have Mockery assertions. For example, something like:

<?php

use \Mockery;
use \Mockery\Adapter\Phpunit\MockeryTestCase;

class ExampleTest extends MockeryTestCase
{
    public function test_run(): void
    {
        $dep = Mockery::mock('dep')
            ->expects('run')
            ->with(1, 2)
            ->getMock();

        $target = new class($dep) {
            public function __construct(
                private $dep
            ) { }

            public function run(): void
            {
                $this->dep->run(1, 2);
            }
        };

        $target->run();
    }
}

Running PHPUnit against that would pass, running rector adds the annotation, then PHPUnit could fail, e.g. if used with the --fail-on-risky flag since this test would be marked as risky.

Imagine a Test class, let's call it A, extending some other (potentially) abstract Test class, B, that has defined example dataProvider methods and class A is using data providers from class B.
This makes DataProviderClassMethodFinder throw exception as the docblock referenced method is not contained in the same class.

The simplest solution would be to change exception throwing into a continue step, disadvantage is that the extended class method won't be recognized as a data provider - but this is already happening for the base class that provides the data provider methods only 🙈.

abstract class SomeAbstractTest extends TestCase
{
    public function provideData()
    {
        yield [1];
    }
}

final class SomeTest extends SomeAbstractTest
{
    /**
     * @dataProvider provideData()
     */
    public function test()
    {
    }
}

Based on class/method name and test suffix

As described in sebastianbergmann/phpunit#5235 and sebastianbergmann/phpunit#5513 the CodeCoverageIgnore attribute was a mistake and is now deprecated, therefore Rector should not try to replace annotation with the attribute.

Addition to https://tomasvotruba.com/blog/how-to-remove-dead-mock-calls-from-phpunit-tests/

-$this->getMockBuilder(...)
-   ->onlyMethods()
-   ->getMock()
+$this->createMock(...);

Bug Report

Minimal PHP Code Causing Issue

Link to PR with failing test case: #16

Expected Behaviour

The rector should ignore assertions that are making use of a class that implements the magic __isset function as PHPUnit uses reflection to check the property and does not call isset(xxx)

PHPUnit will deprecate using non-static methods as data providers: sebastianbergmann/phpunit@9caafe2

It would be great to have a rector that automatically turns data provider methods (i.e. methods referenced in @dataProvider annotations or the equivalent attribute) into static methods (the extra safety would do that only if they are not called or overridden somewhere else)

Consider having a property that is typed as follows:

private ?string $myProperty;

Now using this property as the expected value in an assertEquals and running AssertEqualsToSameRector rule over it, will not change it to assertSame even though it would be desired.

Is there any other rule that takes care of that or is that just something that wasn't considered yet?

Given the following code:

$expected = ['date' => new DateTimeImmutable('2022-10-31 12:32:33')];
$date = new DateTimeImmutable('2022-10-31 12:32:33');
$actual = compact('date');

self::assertEquals($expected, $actual);

The new version of the rule will change the assertion to assertSame but this would lead to an error given this will check the two arrays contain exactly the same object.

Hello I don't know if it is an issue or a expect behaviour.

But there is some functions in codeception that contain indirect assert like this:
https://github.com/Codeception/module-doctrine2/blob/master/src/Codeception/Module/Doctrine2.php#L845

Should we prevent this in this rule?
Because even I check inside of the function it could have assert just in low level like ->MethodA()->b()->c().....
or should I check one level only, if yes did you have any example, how to check this?

See

• https://github.com/sebastianbergmann/phpunit/milestone/35?closed=1
• sebastianbergmann/phpunit#4502

\Rector\PHPUnit\PhpDoc\PhpDocValueToNodeMapper::mapGenericTagValueNode is used for both the expected exception and the expected exception message when refactoring php doc tags into exception methods, which opportunistically tries to use class constants.

If there happens to be :: in the exception message, then the exception message isn't treated as a string. Would it be possible to handle exception messages (and maybe regexps) as strings always? Otherwise invalid class constants can be written and generate invalid PHP.

Since PHPUnit 10, it is not allowed to name abstract test classes with *Test suffix, for example: abstract class MyBaseTest should be renamed to abstract class MyBaseTestCase, etc.
Many existing projects use this naming convention and need refactoring.

Please note that base *Test and target *TestCase suffixes should be customizable (users may choose different suffixes in phpunit.xml using <directory suffix="Test.php">).

For reference: sebastianbergmann/phpunit#4979

i.e it works fine on class like this

use PHPUnit\Framework\TestCase;

class DateTest extends TestCase
{
}

but it does not work on

  use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

  
  class BoostTest extends WebTestCase
  {

}

the issue seems to be that TestsNodeAnalyzer->isInTestClass does not consider them as test class

I'm trying to dig more in the issue

Note: I'm not sure if of any importance but my rector is installed in a different directory than my actual projet (i.e I have a common "rector" tool for all my projects , with its own composer.json )

Bug Report

Minimal PHP Code Causing Issue

See https://getrector.com/demo/3be3c0d9-dbea-4e1d-8c59-ea7fa84c4440

<?php

use PHPUnit\Framework\TestCase;

final class DemoFileTest extends TestCase
{
    /**
     * @var class-string<ExceptionInterface::class> $exception
     * @dataProvider exceptionsDataProvider
     **/
    public function testExceptionIsInstanceOfExceptionInterface(string $exception): void
    {
        self::assertTrue(is_a($exception, ExceptionInterface::class, true));
    }
    
    public static function exceptionsDataProvider(): iterable
    {
        yield from [
            ExceptionInterface::class => [ExceptionInterface::class]
        ];
    }
}

interface ExceptionInterface {
}

Responsible rules

• AssertTrueFalseToSpecificMethodRector

Expected Behavior

to remain unchanged.

Hi,

I noticed some @group … directives not being adapted and after a bit debugging I notice that in \Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\AnnotationWithValueToAttributeRector::refactor the call to getTagsByName returned DoctrineAnnotationTagValueNode instead of GenericTagValueNode.

But there is this check which makes it not pass:

if (!$desiredTagValueNode->value instanceof GenericTagValueNode) {
    continue;
}

As to why: well, I've a (namespaces) class \Project\Models\Group imported in some tests and I guess rector treats the class Group special with @group. However, in my case there's no relation here and since the default config in annotations-to-attributes.php uses the FQCN anyway, there's no clash.

What can I do to still have such @group being converted?

I'm talking about using this in CI, so I'm not looking for a one-time transformation here.

Thanks

Motivation?

• lazyness :)
• symfony/symfony#48668 (comment)

Inspiration:

• https://github.com/symfony/symfony/pull/49244/files#diff-1be27ccb568de8ba9ddeaa8bd720494dd529c3ed21121e4c9d06838eabf7729b
• https://github.com/symfony/symfony/pull/48980/files#diff-14bef5efeb6809c439378585a6768d4b04aa55521ea20527ae20c388157953ca

Bug Report

System error: "array_merge(): Argument rectorphp/rector#2 must be of type array, null given on line 104.

Minimal PHP Code Causing Issue

See https://getrector.com/demo/75a8fde7-5d55-4b3c-84a2-3df83076349c

<?php

use PHPUnit\Framework\TestCase;

final class DemoFileTest extends TestCase
{
    public function testThrowsBadMethodCallException(): void
    {
        try {
            throw new BadMethodCallException();
        } catch (BadMethodCallException $badMethodCallException) {
            self::assertSame($badMethodCallException::class, BadMethodCallException::class);
            return;
        }

        self::fail('Exception was not thrown');
    }
}

class BadMethodCallException extends \RuntimeException {
}

Responsible rules

• TryCatchToExpectExceptionRector

Expected Behavior

to run and remain unchanged.

Hello guys

I'm bumping phpunit to version 9 and I faced this issue in some places

$this->assertSame(15.28, $container->getUnsettledAmount());

//error:
//Failed asserting that 15.280000000000001 is identical to 15.28.

is there any rule to change

//from
$this->assertSame(15.28, $container->getUnsettledAmount());

//to
$this->assertEqualsWithDelta(15.28, $container->getUnsettledAmount(), 0.01);

I noticed a problem with how RemoveDataProviderTestPrefixRector that I think is related to how I'm using inheritance in tests for a project of mine.

I have a base class for tests with a test method that uses an illegally named data provider:

 /**
   * blah blah blah
   *
   * @dataProvider testExceptionsProvider
   */
  public function testExceptions($filename, $message) {

The rector rule correctly updates the annotation to read:

  /**
   * blah blah blah
   *
   * @dataProvider exceptionsProvider
   */
  public function testExceptions($filename, $message) {

However, the annotation in the base class is the only place that the data provider name is changed.

The rector rule seems to be getting "confused" because of how I'm relying on inheritance in these test classes. Specifically:

1. testExceptions() is only declared in the base class.
2. testExceptionsProvider() is not declared in the base test class. It is declared in each of the test classes that extend the base class.

For the record, I am seeing this on a Drupal project that I maintain: https://www.drupal.org/project/crossword/issues/3286830

Thanks!

There is a rule provided in phpunit90 rule set that converts TestListener usages to TestHook usages. This was done because phpunit deprecated TestListener and provided somewhat similar functionality under their TestHook system. This has a couple gotchas where I think maybe removing this from the rule set may be a good idea?

First is the TestListener service provides wildly different functionality to the TestHook system, For example TestListener provides the actual test object whereas the TestHook only provides a string representation. Unfortunately this means that there is a large part of functionality available that a simple method signature change can't ever address.

I think the more important issue though is that TestHook is not a system they recommend to use nor is it actually staying long term, a new event system is under development for phpunit 10. You can see some of the discussion around that here sebastianbergmann/phpunit#3390.

The main reason I suggest this change is that its a potentially breaking change rule since the interface isn't at all the same but its also not even a system that will be in use going forward. I can make a PR to do this but wanted to discuss first what your thoughts were on this?

Just for reference the rule I"m talking about.
https://github.com/rectorphp/rector-phpunit/blob/main/config/sets/phpunit90.php#L13

Given the following code:

<?php

use Mockery;
use Mockery\Adapter\Phpunit\MockeryTestCase;

class ExampleTest extends MockeryTestCase
{
    public function test_run(): void
    {
        $someMock = $this->getMock();

        // ...
    }

    private function getMock()
    {
        return Mockery::mock('dep')
            ->expects('run')
            ->with(1, 2)
            ->getMock();
    }
}

The GetMockRector rector will alter it to be $someMock = $this->createMock();, which is now invalid since in this context getMock points to a private method, not the PHPUnit mocking method.

Another context relates to Mockery exceptions. E.g.

<?php

use Mockery\Adapter\Phpunit\MockeryTestCase;
use Mockery\Exception\NoMatchingExpectationException;

abstract class CustomizedTestCase extends MockeryTestCase
{
    public function runTest(): void
    {
        try {
            parent::runTest();
        } catch (NoMatchingExpectationException $ex) {
            
            /** @var Mockery\MockInterface $failedMock */
            $failedMock = $ex->getMock();

            // ...
        }
    }
}

Would be updated to $failedMock = $ex->createMock();, which of course is invalid due to that method not existing on that exception type.

Hello

is there anyway to apply rules by phpunit version or just php version?

As I can see in the doc this #[DataProvider] attribute is available in phpunit 10 only

https://github.com/rectorphp/rector-phpunit/blob/main/rules/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector.php

9.x - https://docs.phpunit.de/en/9.6/writing-tests-for-phpunit.html#data-providers
10.x - https://docs.phpunit.de/en/10.4/writing-tests-for-phpunit.html#data-providers