Remove direct NodeTravser::* where possible, use node visitor attribute key instead

[TomasVotruba]

No description provided.

[samsonasik]

beware of use in other rector-* packages as well.

[samsonasik]

There is ContextAnalyzer->isLeftPartOfAssign() service for this cases, see

rector-src/src/NodeNestingScope/ContextAnalyzer.php

Lines 49 to 60 in 82f5181

	public function isLeftPartOfAssign(Node $node): bool
	{
	if ($node->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) {
	return true;
	}
	if ($node->getAttribute(AttributeKey::IS_ASSIGN_REF_EXPR) === true) {
	return true;
	}
	return $node->getAttribute(AttributeKey::IS_ASSIGN_OP_VAR) === true;
	}

[samsonasik]

I created PR:

• [Transform] Use ContextAnalyzer->isLeftPartOfAssign() on ArrayDimFetchToMethodCallRector #7702

for it.

[TomasVotruba]

That's not necessary, as hiding simple attributes.

[calebdw]

I came here from the rector-laravel PR, could y'all please elaborate on why this is being done? I'm just curious why the direct NodeTraverser::* is bad and what advantages the attribute keys provide...

[TomasVotruba]

@calebdw Basically, we used it as a hack that was saying "don't use this rule further, if you found node X".
But node traverser doesn't understand this approach. So we had to mark all the nodes beyond node X with special attribute to be ignored by current rule. Took me while to understand this hack :)

It's like using a container to get a service to fetch a controller that has access to container to get service we want.

Instead, Rector rules should be able to work standalone as it is and have all the metadata to decide in the refactor() method. Change the node or return null 👍

[calebdw]

Ah, thanks for the explanation---I'll just need to learn to think about things differently and learn the various attribute keys!

[TomasVotruba]

@calebdw No worries, it's a mess :D That's why I'm reducing these constants to NodeVisitor::REMOVE_NODE.

[TomasVotruba]

@calebdw This article explains the way to add custom attribute keys if needed:
https://phpstan.org/blog/preprocessing-ast-for-custom-rules

[Orest-Divintari]

hi,

i have this case and i want to stop traversing if it finds a parameter

__construct (private $data = new Test())

before this deprecation this was enough

  public function getNodeTypes() : array
    {
        return [Param::class, New_::class];
    }

 if ($node instanceof Param) {
            return NodeTraverser::DONT_TRAVERSE_CHILDREN;
        }

i tried the NodeVisitor::DONT_TRAVERSE_CHILDREN instead but that does not seem to work , i tried other constants of that file too

is there any other alternative to this ? or does it need a custom solution ?

@samsonasik

[samsonasik]

@Orest-Divintari you can create custom visitor for that, for example:

<?php

declare(strict_types=1);

namespace My\Rector\Visitor;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Param;
use PhpParser\NodeVisitorAbstract;
use Rector\Contract\PhpParser\DecoratingNodeVisitorInterface;
use Rector\PhpParser\NodeTraverser\SimpleNodeTraverser;

final class IsInParamDefaultVisitor extends NodeVisitorAbstract implements DecoratingNodeVisitorInterface
{
    public const IS_IN_PARAM_DEFAULT = 'is_in_param_default;

    public function enterNode(Node $node)
    {
        if (! $node instanceof Param) {
            return null;
        }

       if (! $node->default instanceof Expr) {
             return null;
       }

        SimpleNodeTraverser::decorateWithAttributeValue($param->default, self::IS_IN_PARAM_DEFAULT, true);
        return null;
    }
}

Then, register to rector config:

<?php

use Rector\Config\RectorConfig;
use My\Rector\Visitor\HelloVisitor;

return RectorConfig::configure()
    // other existing config ...
    ->registerDecoratingNodeVisitor(\My\Rector\Visitor\IsInParamDefaultVisitor::class);

Then, on your custom rector rule, check it:

if ($node->getAttribute(\My\Rector\Visitor\IsInParamDefaultVisitor::IS_IN_PARAM_DEFAULT) === true) {
      return null;
}

[Orest-Divintari]

@Orest-Divintari you can create custom visitor for that, for example:

<?php



declare(strict_types=1);



namespace My\Rector\Visitor;



use PhpParser\Node;

use PhpParser\Node\Expr;

use PhpParser\Node\Param;

use PhpParser\NodeVisitorAbstract;

use Rector\Contract\PhpParser\DecoratingNodeVisitorInterface;

use Rector\PhpParser\NodeTraverser\SimpleNodeTraverser;



final class IsInParamDefaultVisitor extends NodeVisitorAbstract implements DecoratingNodeVisitorInterface

{

    public const IS_IN_PARAM_DEFAULT = 'is_in_param_default;



    public function enterNode(Node $node)

    {

        if (! $node instanceof Param) {

            return null;

        }



       if (! $node->default instanceof Expr) {

             return null;

       }



        SimpleNodeTraverser::decorateWithAttributeValue($param->default, self::IS_IN_PARAM_DEFAULT, true);

        return null;

    }

}

Then, register to rector config:

<?php



use Rector\Config\RectorConfig;

use My\Rector\Visitor\HelloVisitor;



return RectorConfig::configure()

    // other existing config ...

    ->registerDecoratingNodeVisitor(\My\Rector\Visitor\IsInParamDefaultVisitor::class);

Then, on your custom rector rule, check it:

if ($node->getAttribute(\My\Rector\Visitor\IsInParamDefaultVisitor::IS_IN_PARAM_DEFAULT) === true) {

      return null;

}

Thank you !