Custom rule "foo {$var} bar" => "foo bar"

[AngelinCalu]

Given the following custom rule:

<?php

declare (strict_types=1);
namespace Rector\Basic\Rector\EncapsedStringPart;

final class RemoveGivenVariableRector extends \Rector\Core\Rector\AbstractRector
{
    public function getRuleDefinition() : \Symplify\RuleDocGenerator\ValueObject\RuleDefinition
    {
        return new \Symplify\RuleDocGenerator\ValueObject\RuleDefinition('"foo {$var} bar" => "foo bar"', [new \Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample('$test = "foo {$var} bar";', '$test = "foo bar";')]);
    }

    public function getNodeTypes() : array
    {
        return array(\PhpParser\Node\Scalar\Encapsed::class);
    }

    public function refactor(\PhpParser\Node $node) : ?\PhpParser\Node
    {
        $updated = false;
        foreach ($node->parts as $part) {
            if ($part instanceof \PhpParser\Node\Expr\Variable && $part->name == 'var') {
                $this->removeNode($part);
                $updated = true;
            }
        }

        if ($updated) {
            return $node;
        }
    }
}

Here are the expected vs. actual results 👇

-$test = "foo bar";
+$test = "foo } bar";

Is there a better way to do this?
What's the proper way to get rid of the closing curly bracket?

[TomasVotruba]

Hi, in this case it's not legal to remove node that is part of bigger one. Imagine 2-part concat, where one is removed:

-return $hi . $name;
+return $hi . ;

Instead, the return node should be new String_(...) .

[AngelinCalu]

Apparently, this will not work in Rector, but only if I'd use PHP-Parser directly.

in PHP-Parser:

<?php

require_once "./vendor/autoload.php";

$code = <<<'CODE'
<?php

$test = "foo {$var} bar";
$test = "foo ${var} bar";
CODE;

$parser = (new PhpParser\ParserFactory)->create(PhpParser\ParserFactory::PREFER_PHP7);
try {
    $ast = $parser->parse($code);
} catch (PhpParser\Error $error) {
    echo "Parse error: {$error->getMessage()}\n";
    return;
}

$traverser = new PhpParser\NodeTraverser();
$traverser->addVisitor(new class extends PhpParser\NodeVisitorAbstract {
    public function enterNode(PhpParser\Node $node) {
        if ($node instanceof \PhpParser\Node\Scalar\Encapsed) {
            $updated = false;
            foreach ($node->parts as &$part) {
                if ($part instanceof \PhpParser\Node\Expr\Variable && $part->name == 'var') {
                    $part = new PhpParser\Node\Scalar\EncapsedStringPart('');
                    $updated = true;
                }
            }
    
            return ($updated) ? $node : null;
        }
    }
});

$ast = $traverser->traverse($ast);

$prettyPrinter = new PhpParser\PrettyPrinter\Standard;
echo $prettyPrinter->prettyPrintFile($ast) . PHP_EOL;

returns:

<?php

$test = "foo  bar";
$test = "foo  bar";

In Rector:

<?php

declare (strict_types=1);
namespace Rector\Basic\Rector\EncapsedStringPart;

final class RemoveGivenVariableRector extends \Rector\Core\Rector\AbstractRector
{
    public function getRuleDefinition() : \Symplify\RuleDocGenerator\ValueObject\RuleDefinition
    {
        return new \Symplify\RuleDocGenerator\ValueObject\RuleDefinition('"foo {$var} bar" => "foo bar"', [new \Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample('$test = "foo {$var} bar";', '$test = "foo bar";')]);
    }

    public function getNodeTypes() : array
    {
        return array(\PhpParser\Node\Scalar\Encapsed::class);
    }

    public function refactor(\PhpParser\Node $node) : ?\PhpParser\Node
    {
        $updated = false;
        foreach ($node->parts as &$part) {
            if ($part instanceof \PhpParser\Node\Expr\Variable && $part->name == 'var') {
                $part = new \PhpParser\Node\Scalar\EncapsedStringPart('');
                $updated = true;
            }
        }

        if ($updated) {
            return $node;
        }
    }
}

for the same input as above, Rector will produce the following (Expected / Actual) result:

-$test = "foo  bar";
-$test = "foo  bar";
+$test = "foo {``} bar";
+$test = "foo `` bar";

Maybe Rector is missing some prettyPrinter features?

[TomasVotruba]

You're still returning the Encapsed_ node.

The returned node must be String_

[AngelinCalu]

The returned node cannot be String_ as it might contain dozens of other variables.

In my tests I also have these scenarios which would fail:

$test = "foo {$var} bar {$baz} continues ... id='$id' ";
$test = "foo ${var} bar {$baz} continues ... id='$id' ";

[TomasVotruba]

Yes, that's what I explained there: #7075 (reply in thread)

• In case of 1 string value, the returned node must be String_
• In case of 1+ EncapsedPart + 1+ string value, returned node must be Encapsed

It's best practise to create a new node in both case.

[AngelinCalu]

Okay, we can assume all my returns are Encapsed as in reality I don't have any case which would return just String_, as I said previously, this was just a way of simplifying the actual cases. The actual problem is that:

$test = "foo {$var} bar {$baz} continues ... ";
$test = "foo ${var} bar {$baz} continues ... ";

Would be returned as follows:

$test = "foo {``} bar {$baz} continues ...  ";
$test = "foo `` bar {$baz} continues ...  ";

when replacing the $var with an empty EncapsedStringPart. If I remove the node part I will be left with a closing curly bracket.