Skip to content

Screen file command improvements#2159

Merged
TomasVotruba merged 2 commits intomasterfrom
screen-file
Oct 14, 2019
Merged

Screen file command improvements#2159
TomasVotruba merged 2 commits intomasterfrom
screen-file

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Oct 14, 2019

vendor/bin/rector screen-file SomeFile.php

Input file

<?php

declare(strict_types=1);

namespace App;

final class ScreenSample
{
    public function run($value)
    {
        $value = is_string($value) ? 5 : $value;
        return $value;
    }
}

Output helper

<?php
// node_type: "Declare_"
declare(strict_types=1);
// node_type: "Namespace_"
// name: "App"
namespace App;

// node_type: "Class_"
// class_name: "App\ScreenSample"
final class ScreenSample
{
    // node_type: "ClassMethod"
    public function run($value)
    {
        // node_type: "Assign"
        // assign_var: 
        //   * node_type: "Variable"
        //   * variable_name: "value"
        //   * variable_types: "mixed"
        
        // assign_expr: 
        //   * node_type: "Ternary"
        $value = is_string(
            // node_type: "Arg"
            $value
        ) ? 5 : $value;
        // node_type: "Return_"
        // returned_node: 
        //   * node_type: "Variable"
        //   * variable_name: "value"
        //   * variable_types: "mixed"
        return $value;
    }
}

@TomasVotruba
Copy link
Copy Markdown
Member Author

cc @DaveLiddament @gnutix What is missing to be useful here?

@gnutix
Copy link
Copy Markdown
Contributor

gnutix commented Oct 14, 2019

Are comments / docblocks "explained" too ?

@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Oct 14, 2019

Could you show me some example what would help you?

@gnutix
Copy link
Copy Markdown
Contributor

gnutix commented Oct 14, 2019

Hmm.. maybe I spoke too soon. Regarding the type of information that this command provides, I'm not sure it could/should say anything about comments.

Is the purpose of this command solely to help people code/contribute Rectors ? If so, I think it might help if the command returned code examples on how to check for these types or fetch some information (like how to get data for a Node, or how to get a comment/docblock, stuff like that). But maybe that's too much and not the scope of this PR (and I guess most of this is related to Nikic PHP Parser right ? so maybe a link to https://github.com/nikic/PHP-Parser#documentation might "just work" as well ?

@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Oct 14, 2019

Is the purpose of this command solely to help people code/contribute Rectors ?

Yes! We used it at workshop, so people don't have to learn/remember how php-parser, PHPStan and Rector works (way too much info for anyone). And it was great, saved use like 40 minutes of talking about (booring) theory.

You just run the command, look at the line you want to change and see it's MethodCall of type A - half job done.

how to get data for a Node

At the moment simple dump($node) in Rector shows the most important stuff. Docs is also one of the sources, but again too complicated. One programmre wants to change method, another extra function etc.

@gnutix
Copy link
Copy Markdown
Contributor

gnutix commented Oct 14, 2019

This is definitely a very nice first step indeed. I guess we'll see down the road if there are more improvements that could be done. Dumping a $node for example provides quite a huuuuuuge tree of information, most of them probably never useful to write a simple check or change. But let's start with that already. :)

@TomasVotruba
Copy link
Copy Markdown
Member Author

This is definitely a very nice first step indeed. I guess we'll see down the road if there are more improvements that could be done.

Agreed, I'll put it out on trainings and start to collect feedback what is relevant and what not.

I guess we'll see down the road if there are more improvements that could be done. Dumping a $node for example provides quite a huuuuuuge tree of information, most of them probably never useful to write a simple check or change. But let's start with that already. :)

Agreed, thinking of that, there might be useful function dump_node() which would dump only what is useful.

Thanks for feedback

@TomasVotruba TomasVotruba merged commit fb41b38 into master Oct 14, 2019
@TomasVotruba TomasVotruba deleted the screen-file branch October 14, 2019 18:21
TomasVotruba added a commit that referenced this pull request Apr 25, 2022
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

Successfully merging this pull request may close these issues.

2 participants