[DeadCode] Requiring phpstorm-stubs leads to Multiple definitions exist for class...

[atierant]

Question

Hi. I have a question about this commit bd909ab3
Can you tell me why phpstorm-stubs are not in require-dev ?

Requiring rector in my require-dev project part leads me to "multiple definitions exist" problem. Thank you !

[TomasVotruba]

Hi, good job with commit reference! 👍 It makes easier for me to find out why it was added.

Does the rule work locally when you remove the "phpstorm-stubs" package in your project? If so, please send PR for your proposal.

[atierant]

I'll check what I can do. I don't directly have "phpstorm-stubs" requirement in my project.
Of course, if I delete the jetbrains folder in vendors, the dead-code inspection fails.

me@mymachine:/mnt/c/GitSources/RectorRequirement$ rm -rf vendor/jetbrains
me@mymachine:/mnt/c/GitSources/RectorRequirement$ composer rector
> vendor/bin/rector process src --set dead-code --dry-run
PHP Warning:  require(/mnt/c/GitSources/RectorRequirement/vendor/composer/../jetbrains/phpstorm-stubs/PhpStormStubsMap.php): failed to open stream: No such file or directory in /mnt/c/GitSources/RectorRequirement/vendor/composer/au
toload_real.php on line 66
PHP Fatal error:  require(): Failed opening required '/mnt/c/GitSources/RectorRequirement/vendor/composer/../jetbrains/phpstorm-stubs/PhpStormStubsMap.php' (include_path='.:/usr/share/php') in /mnt/c/GitSources/RectorRequirement/ve
ndor/composer/autoload_real.php on line 66
Script vendor/bin/rector process src --set dead-code --dry-run handling the rector event returned with error code 255

I forked rector to check if I can make a proposal.

My tests : https://github.com/atierant/rector-phpstorm-stubs-requirements

[TomasVotruba]

What if you remove it from Rector's composer.json autoload and rebuild?

[atierant]

Simply like this ? e37c430

I updated my test project (master branch) according to the fork to point out the diff.

me@mymachine:/mnt/c/GitSources/RectorRequirement$ composer rector
> vendor/bin/rector process src --set dead-code --dry-run
Rector dev-phpstorm-requirement@e37c430

 0/3 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%
 2/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░]  66%
 3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

1 file with changes
===================

1) src/Inspect.php

    ---------- begin diff ----------
--- Original
+++ New
@@ -17,7 +17,7 @@

     public function run()
     {
-        $this->runWithDefault([]);
+        $this->runWithDefault();
         $card = self::runWithStaticDefault([]);
     }
    ----------- end diff -----------

Applied rectors:

 * Rector\DeadCode\Rector\MethodCall\RemoveDefaultArgumentValueRector


 [OK] Rector is done! 1 changed files

Script vendor/bin/rector process src --set dead-code --dry-run handling the rector event returned with error code 1
me@mymachine:/mnt/c/GitSources/RectorRequirement$

[TomasVotruba]

Simply like this ? e37c430

That's also a way.

Looks good 👍

[atierant]

The script seems to output in error anyway.
Script vendor/bin/rector process src --set dead-code --dry-run handling the rector event returned with error code 1
May be a bad php config on my computer, so I launched a CI on my branch to check : https://travis-ci.org/atierant/rector/builds/576815247

[TomasVotruba]

Try running it with: --debug

The dependency should not be removed completely, just moved to autoload-dev, because tests work with it.

[atierant]

Ok for the error, just a segfault in one of my php modules, nothing to do with rector.
When I run it on another clean env, everything looks fine.
P.R. is here: #1901
Thank you for the lead ;)

[gnutix]

@TomasVotruba Now the stubs are located inside Rector's code in stubs/, which again leads to PhpStorm's multiple definition exist for class %s warnings. What would be needed so that we could we add a /stubs export-ignore to .gitattributes to skip the stubs when installing Rector from Composer ? I tried to remove the folder locally and got one error at the end of the execution :

 [ERROR] Could not process                                                      
         "src/MyBundle.php" 
         file, due to:                                                          
         "File or directory                                                     
         'vendor/rector/rector/src/Stubs/../../stubs' not found.".                      

So I guess it would need some changes.

[TomasVotruba]

Well, without stubs some rules won't work.
E.g. phpunit_framework_testcase that is needed to upgrade some phpunit versions with ease

It would require to check every stubs manually per rule, which is crazy.

Another idea is to run some script on Rector install, which would remove existing classes in stubs

[gnutix]

Wouldn't it make sense for a Rector not to work if the real classes are missing from the project ? That would tell the user that including this Rector makes no sense, wouldn't it ?

[TomasVotruba]

Logically and yes, form user experince that means:

• find out why PHPUnit_Framework_TestCase is missing
• find how PHPUnit loads it
• learn about composer.json and require-dev section
• learn about files section
• find out where is PHPUnit_Framework_TestCase located
  • if nowhere, figure out they have to create it manually
  • figure out how to autoloadit it
  • figure out composer dump-autoload command, what it does

I had to do this 5 days ago, took me 15 minutes of life (and I wrote this :D )

Most people will get to step 1 and close it frustration