Rector: Suggestions for new Rectors

[phpfui]

Feature Request

I am starting to write Rectors to transform my code into modern PHP. I think others may find them useful and want to contribute, but looking for namespaces and naming suggestions.

Here are some of them that I need and am creating. Let me know if you think they would be useful and suggest namespaces/names.

AddTopOfFileIncludeStatement

My source does not use a controller, so all URLs are filename.php type things. Plain PHP with write statements. There is no common include so no easy way add autoloading, session management, etc. This rector would allow you to add a include statement after the first PHP open tag in a file, but would skip any file that starts with a class definition.

RemoveIncludes

The above source includes the file with the class definition right before a new ClassName() statement. This simply nukes all include / require statements. This is run first, then AddTopOfFileIncludeStatement

GetClassNames

This creates a CSV formatted file with the class name and file that contains the class. The above code does not use namespaces, but does have some sort of directory structure that could be namespaces. This would not modify the code, but simply generate the CSV file for analysis.

NamespaceClasses

With the above CVS file, classes can be assigned to namespaces. This rector would namespace any class and add namespaces to any new statements for classes used. Would be configured in rector.yaml. This may need to be two steps, one to namespace the class itself, and another to add namespace on new statement.

BreakoutClasses

Above source is riddled with classed defined in the source then used in same file. This will break out the class into a new file following the namespace. rector.yaml would include root paths if needed.

Let me know your feedback on these. Some of these may already exist, just don't see anything like them from what I can tell.

[TomasVotruba]

Hi,

let's start slowly with 1 rule. I like RemoveIncludes :+1:

You can send PR with that rule in Legacy namespace - Rector\Legacy\Rector\Include_\RemoveIncludeRector

After we merge it, we'll pick next low handing fruit.

[phpfui]

Sounds good to me. Low hanging fruit is the tastiest. Do you have an example of a good PR to follow?

[TomasVotruba]

Do you have an example of a good PR to follow?

Great question! This week I really liked this simple PR: https://github.com/rectorphp/rector/pull/3669

There is helper command, that generated most of the boiler plate code for you: https://github.com/rectorphp/rector/blob/master/docs/rector_recipe.md

[phpfui]

I have AddTopOfFileIncludeStatement rector working. Would you like a PR for that? If so, suggest an name. Maybe RectorLegacy\Rector\Include_\AppTopOfFileIncludeRector?

[TomasVotruba]

I can't imagine what this rules does. Could share a small snippet here in comments?

-before
+after

Also for other rules. It's easier to evalues exact PHP code rather than text.

[phpfui]

The so the problem is on my (inherited) legacy site, there is no common include file. Each class used needed it's own include. Which is why I needed the remove all includes rector. Now that that is done, I need to add back one common include to all files that are not class definitions. Classes will autoload once I get one common include that will set up the autoloader. It would all an include file before the first executable PHP statement that is not related to a class file (uses, namespace, define and class would not count as executable code).

So the before / after code would look like this:

~~~php
-before

outputHeader ($menu = 'Contact');
if (isset($_GET['user']))
$contact = $_GET['user'];
if (isset($_POST['email']))
$contact = $_POST['user'];

+after
include 'common.php';

outputHeader ($menu = 'Contact');
if (isset($_GET['user']))
$contact = $_GET['user'];
if (isset($_POST['email']))
$contact = $_POST['user'];
~~~

Completely understand not wanting to add it. Just thinking others might find it helpful, but not sure.

[TomasVotruba]

I see. I worked with such leagcy, so I'm aware of the need.

Well, we can add such rule. It would require configuration for:

• name of the file that should be included [required]
  
  
  
  • just to let people choose, also it should support nesting, not just common.php ,but include/common.php
  
  
• name pattern to name files [optional]
  
  
  
  • sometimes only e.g. *Presenter.php file needs this include, while files in include/*.php do not
  
  

What do you think?

[phpfui]

For sure the name of the file to include.

I think the pattern match is also a good idea and I agree optional.

A good name?

[TomasVotruba]

AddTopOfFileIncludeStatement seems good!

Maybe just AddTopIncludeRector, file and statements are obvious words

[phpfui]

Great.  Coming up.  Thinking multiple pattern matching, or multiple excludes.  Will figure something out.

[phpfui]

Tomas,

Any others of these you want to add? Maybe BreakoutClasses or NamespaceClasses exists under different names?

I am going to GetClassNames for myself now. I have a bunch of classes with no namespaces, but reasonable file structure. Thinking it would dump classname,namespace,file lines into a file but not do any mods to files.

Then I could do breakout and finally namespace.

Also, when do expect the next release?

[TomasVotruba]

• BreakoutClasses is already implemented https://github.com/rectorphp/rector/blob/master/docs/rector_rules_overview.md#multipleclassfiletopsr4classesrector, right?

• NamespaceClasses - this might be already implemented too... but I'm not sure I understand the goal of this rule. Could you show me small example of PHP code in diff? before/after?

I see you're basically going for PSR-4 in the end, am I right?

You might find this useful: From Class Mess to PSR-4 Step by Step With Confidence

[phpfui]

Yes, PSR-0 with no composer used on deploy. All source files checked in, but not in vendor directory. Just source, nothing else.

I will probably add Rector to http://www.phpfui.com, which is my documentation library example site. It uses this approach. I just need to add dynamic page loading, which I should have in the next few weeks. Right now it reloads the entire menu for each page load, which is a bit wasteful. But really nice to sell fully linked docs to all libraries.

Thanks for the link. I will investigate more.

[TomasVotruba]

If you ever have an idea for a new Rector, make an issues with diff code and state where it helps.
If we confirm that it might be useful for wider community, you can follow up with PR :+1:

Closing as answered.