rector: Problems with rector and pestphp when installed together.

Hi @TomasVotruba and @nunomaduro,

Disclaimer: To be honest I donโ€™t know if this belongs into the rector/rector repository or the pestphp/pest repository. Iโ€™m just hoping that we can reach a solution on this topic.

I have a problem with a project when using both rector and pest. The problem has to do with load-order as I see it.

My requirements atm:

pestphp/pest 1.2.1
rector/ 0.11.4

The Rector checks if function expects already exists in the global namespace, if not creating it [1]. Pest is creating in, in the global namespace [2].

This leads to this error when running my rector command:

PHP Fatal error:  Cannot redeclare expect() (previously declared in /home/runner/work/rector-pest-problem/rector-pest-problem/vendor/rector/rector/vendor/scoper-autoload.php:170) in /home/runner/work/rector-pest-problem/rector-pest-problem/vendor/pestphp/pest-plugin-expectations/src/Autoload.php on line 20

https://github.com/rectorphp/rector/blob/568d9ef7f59ba1105c17da7100e397ed1a4417c2/vendor/scoper-autoload.php#L168

if (!function_exists('expect')) {
    function expect() {
        return \RectorPrefix20210517\expect(...func_get_args());
    }
}

https://github.com/pestphp/pest-plugin-expectations/blob/master/src/Autoload.php#L20 2)

function expect($value = null)
{
    if (func_num_args() === 0) {
        return new Extendable(Expectation::class);
    }

    return new Expectation($value);
}

I have created a small demo repository for this propose: https://github.com/tomasnorre/rector-pest-problem

Where you can also see the CI run: https://github.com/tomasnorre/rector-pest-problem/runs/2610735473?check_suite_focus=true#step:7:4

How to reproduce:

$ git clone https://github.com/tomasnorre/rector-pest-problem
$ cd rector-pest-problem
$ composer install 
$ ./vendor/bin/rector 

Best Tomas

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 19 (19 by maintainers)

Commits related to this issue

Most upvoted comments

Just for reference, Prโ€™s has been opened in Pest also to wrap all global functions in function_exists statements. ๐Ÿ‘

@TomasVotruba Oh wauw ๐Ÿ˜ฎ

Yeah that kinda makes sense, but sounds really scary

What about one that wraps global functions in function_exists? ๐Ÿ˜„

That would actually overpower the other functions, that would fatal error. Imagine 2 packages with same-named function:

// package A

if (! function_exists('me_first')) {
    function me_first(string $value)
    {
    }
}


// package B

if (! function_exists('me_first')) {
    function me_first(int $value)
    {
    }
}



// your code
me_first(10);


// code of package A
me_first('hey');

What will happen? Depends on OS, file name, package name and sorting algorithm of composer that will prefer one or the other. Most likely ๐Ÿ’ฅ ๐Ÿ˜„


This code smell actually shut down Rector ecosystem ~2 weeks ago. php-parser and PHP_CodeSniffer emulated T_ENUM tokens in 2 different ways:

if (defined('T_ENUM') === false) {
    define('T_ENUM', 5005);
}


if (defined('T_ENUM') === false) {
    define('T_ENUM', 'php_cs_enum');
}

Integer was placed into string type declatarion and code crashed. I had a full day of work to figure this out and patch it ๐Ÿ˜ƒ

@TomasVotruba I am on it. Reason found. Gonna try to fix it.

No problem. Maybe we could close one of them.