Skip to content

[Core] Decorate all below nodes only on usage of DONT_TRAVERSE_CHILDREN or DONT_TRAVERSE_CURRENT_AND_CHILDREN#4191

Closed
samsonasik wants to merge 18 commits intomainfrom
use-dont-traverse-children
Closed

[Core] Decorate all below nodes only on usage of DONT_TRAVERSE_CHILDREN or DONT_TRAVERSE_CURRENT_AND_CHILDREN#4191
samsonasik wants to merge 18 commits intomainfrom
use-dont-traverse-children

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik samsonasik commented Jun 12, 2023

Instead of using STOP_TRAVERSAL or custom attribute, the DONT_TRAVERSE_CHILDREN or DONT_TRAVERSE_CURRENT_AND_CHILDREN with decorate all nodes below it.

This only filter only types that:

  1. registered in getNodesTypes() method
  2. different with current node type, as already decorated above

Ref rectorphp/rector#7983 (comment)

If it accepted, I will apply on other rules.

@samsonasik samsonasik force-pushed the use-dont-traverse-children branch from 3fb5961 to aea06c8 Compare June 12, 2023 10:00
@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@samsonasik
Copy link
Copy Markdown
Member Author

It seems will buggy on target node already changed by other rule, I willa dd failing test case.

@samsonasik samsonasik marked this pull request as draft June 12, 2023 12:53
@samsonasik
Copy link
Copy Markdown
Member Author

@samsonasik
Copy link
Copy Markdown
Member Author

Add new add skipped_by_rector_rule attribute seems the solution.

@samsonasik samsonasik marked this pull request as ready for review June 12, 2023 13:15
@samsonasik
Copy link
Copy Markdown
Member Author

It seems not working if node just reprinted:

        if ($node->value === 'Rector\Tests\Php55\Rector\String_\StringClassNameToClassConstantRector\Source\SomeUser') {
            $node->value = 'Rector\Config\RectorConfig';
            $node->setAttribute(AttributeKey::ORIGINAL_NODE, null);
            return $node;

It got:

1) Rector\Core\Tests\Issues\StringClassNameConstantDefaultValue\StringClassNameConstantDefaultValueTest::test with data set #0
Failed on fixture file "in_array_constant_with_undefined_variable.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 final class InArrayConstantWithUndefinedVariable
 {
     const SKIP_TYPES = [
-       'Rector\Config\RectorConfig'
+       \Rector\Config\RectorConfig::class
     ];

@samsonasik samsonasik marked this pull request as draft June 12, 2023 13:22
@samsonasik
Copy link
Copy Markdown
Member Author

@samsonasik
Copy link
Copy Markdown
Member Author

It seems traversing can't scan a node that doesn't have origNode attribute via reprint:

$node->setAttribute(AttributeKey::ORIGINAL_NODE, null);

so it doesn't have the information, that's why it still apply the changes if below node is reprinted.

@samsonasik
Copy link
Copy Markdown
Member Author

closing for now as too many conflict and requires rework

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