Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--set mysql-to-mysqli seems to be executing the PHP code it should be analyzing #3620

Closed
phpfui opened this issue Jun 30, 2020 · 9 comments
Closed

Comments

@phpfui
Copy link
Contributor

phpfui commented Jun 30, 2020

Bug Report

Subject Details
Rector version v0.7.41
Installed as composer dependency / php vendor\rector\rector\bin\rector

Running the mysql-to-mysqli set seems to run the code it is analyzing! It prints out things to the console (that the php should do if it were running) and will even stop execution of Rector with exit() (again, php would do this if the code were executed).

Minimal PHP Code Causing Issue

I am converting a large PHP 5.6 code base, so not sure exactly how to reproduce. Will update this if I can get something closer to a small example.

Expected Behaviour

Rector should not be executing code. It actually was finding unset array indices (which was good actually and I fixed). Very weird.

Running from Windows 10 with the following command:

php vendor\rector\rector\bin\rector process src\ajax --set mysql-to-mysqli --verbose

PHP is 7.4.7

I am more than happy to track this issue down and submit a PR, but not sure how to run Rector outside the phar file. Is there any documentation on debugging Rector?

Great tool by the way. Want to give back and make it even better!

@phpfui
Copy link
Contributor Author

phpfui commented Jun 30, 2020

I have figured out how to debug Rector. Hope to have a PR at some point when I figure out the problem.

@phpfui
Copy link
Contributor Author

phpfui commented Jun 30, 2020

So looks like the problem is the autoloader is including a file. But the file has plain PHP code in it as well as a class definition. So the include require does the standard PHP thing and executes the plain code. Will look into a work around on this, but real solution is to break the class out of the file.

@phpfui
Copy link
Contributor Author

phpfui commented Jul 2, 2020

I was able to solve this easily by knowing what file was being include. It would generally fail as soon as the file was included. In case anyone else has this problem, the easiest solution is to print the file being required by RobotLoader\RobotLoader on line 114.

Ideally --verbose would output this information, but the problem is RobotLoader has no indication a file was loaded. Ideally RobotLoader::tryLoad would return the file it loaded or an empty string if it did not load a file. But RobotLoader is a separate project and the PR would get involved.

Closing this. Hope someone else can find this useful.

@phpfui phpfui closed this as completed Jul 2, 2020
@TomasVotruba
Copy link
Member

I wonder if this will get solved with static reflections. See #3490

@phpfui
Copy link
Contributor Author

phpfui commented Jul 2, 2020

Not sure, basically you need to load a file. The would normally be a class and would not execute. But in my case (very old code with no autoloader and classes and raw php code mixed together), the require statement is causing PHP to read it in and execute it, since that is what PHP is supposed to do with a require statement.

If there is a commit I can test against, would be happy to so.

A solution may be to read the file by hand, then add it to the parser. I don't think there is a direct PHP to load but not execute a file. That would work as well.

@TomasVotruba
Copy link
Member

That's exactly case for static reflection. To analyse even functions without loading them.

If there is a commit I can test against, would be happy to so.

There is just issue now 😅

Maybe other way around. If you can provide faling test case, I can create a fix for it.

@phpfui
Copy link
Contributor Author

phpfui commented Jul 3, 2020 via email

@phpfui
Copy link
Contributor Author

phpfui commented Jul 7, 2020

Tomas,

I just ran my source through the latest Rector 0.7.46 and seems to have solved the problem. I also could not come up with a simple example, so I think we are done. I can reopen another issue with a small example if I run into it again.

@TomasVotruba
Copy link
Member

Great. I'm glad it works.

Thanks for reaching out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants