Refactor PARENT_NODE away from JoinStringConcatRector (#3947)

rectorphp · May 24, 2023 · eef0e5d · eef0e5d
1 parent c050b66
commit eef0e5d
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 71 deletions.
diff --git a/...JoinStringConcatRector/Fixture/skip_when_one_of_the_lines_becomes_longer_than_120.php.inc b/...JoinStringConcatRector/Fixture/skip_when_one_of_the_lines_becomes_longer_than_120.php.inc
diff --git a/rules/CodeQuality/Rector/Concat/JoinStringConcatRector.php b/rules/CodeQuality/Rector/Concat/JoinStringConcatRector.php
@@ -10,7 +10,6 @@
 use PhpParser\Node\Scalar\String_;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\Util\StringUtils;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -31,8 +30,6 @@ final class JoinStringConcatRector extends AbstractRector
      */
     private const ASCII_REGEX = '#[^\x00-\x7F]#';
 
-    private bool $nodeReplacementIsRestricted = false;
-
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition(
@@ -76,73 +73,35 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        $this->nodeReplacementIsRestricted = false;
-
-        if (! $this->isTopMostConcatNode($node)) {
-            return null;
-        }
-
-        $joinedNode = $this->joinConcatIfStrings($node);
-        if (! $joinedNode instanceof String_) {
+        if (! $node->left instanceof String_) {
             return null;
         }
 
-        if ($this->nodeReplacementIsRestricted) {
+        if (! $node->right instanceof String_) {
             return null;
         }
 
-        return $joinedNode;
+        return $this->joinConcatIfStrings($node->left, $node->right);
     }
 
-    private function isTopMostConcatNode(Concat $concat): bool
+    private function joinConcatIfStrings(String_ $leftString, String_ $rightString): ?String_
     {
-        $parentNode = $concat->getAttribute(AttributeKey::PARENT_NODE);
-        return ! $parentNode instanceof Concat;
-    }
-
-    private function joinConcatIfStrings(Concat $node): Concat | String_
-    {
-        $concat = clone $node;
-
-        if ($concat->left instanceof Concat) {
-            $concat->left = $this->joinConcatIfStrings($concat->left);
-        }
-
-        if ($concat->right instanceof Concat) {
-            $concat->right = $this->joinConcatIfStrings($concat->right);
-        }
-
-        if (! $concat->left instanceof String_) {
-            return $node;
-        }
-
-        if (! $concat->right instanceof String_) {
-            return $node;
-        }
-
-        $leftValue = $concat->left->value;
-        $rightValue = $concat->right->value;
+        $leftValue = $leftString->value;
+        $rightValue = $rightString->value;
 
-        if ($leftValue === "\n") {
-            $this->nodeReplacementIsRestricted = true;
-            return $node;
-        }
-
-        if ($rightValue === "\n") {
-            $this->nodeReplacementIsRestricted = true;
-            return $node;
+        if ($leftValue === "\n" || $rightValue === "\n") {
+            return null;
         }
 
-        $resultString = new String_($leftValue . $rightValue);
-        if (StringUtils::isMatch($resultString->value, self::ASCII_REGEX)) {
-            return $node;
+        $joinedStringValue = $leftValue . $rightValue;
+        if (StringUtils::isMatch($joinedStringValue, self::ASCII_REGEX)) {
+            return null;
         }
 
-        if (Strings::length($resultString->value) >= self::LINE_BREAK_POINT) {
-            $this->nodeReplacementIsRestricted = true;
-            return $node;
+        if (Strings::length($joinedStringValue) >= self::LINE_BREAK_POINT) {
+            return null;
         }
 
-        return $resultString;
+        return new String_($joinedStringValue);
     }
 }