Repository: PHP-CS-Fixer/PHP-CS-Fixer (php)

Issue:
`no_useless_concat_operator` rule may break code
## Bug report

<!--
  Before reporting a bug, make sure PHP CS Fixer is up-to-date.

  Check that the issue is NOT regarded to an applied rule by using the `-vvv` option (will list applied rules) on a minimal code snippet.
-->

### Description

`no_useless_concat_operator` rule may break code when variables and whitespace are mixed in a string enclosed by double colons.

### Runtime version

```
PHP CS Fixer 3.48.0 Small Changes by Fabien Potencier, Dariusz Ruminski and contributors.
PHP runtime: 8.2.5
```

### Used command

```bash
$ vendor/bin/php-cs-fixer fix example.php -vvv
PHP CS Fixer 3.48.0 Small Changes by Fabien Potencier, Dariusz Ruminski and contributors.
PHP runtime: 8.2.5
Loaded config default from "/test/.php-cs-fixer.dist.php".
Using cache file ".php-cs-fixer.cache".
Paths from configuration file have been overridden by paths provided as command arguments.
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

1) example.php (no_useless_concat_operator)

Fixed 1 of 1 files in 0.006 seconds, 14.000 MB memory used
```

### Configuration file

```php
<?php

$config = new PhpCsFixer\Config();

return $config
    ->setRules([
        'no_useless_concat_operator' => true,
    ]);
```

### Code snippet that reproduces the problem

```php
<?php

$param1 = 'Hello';
$param2 = 'World!';

return "$param1" . "ooo　$param2";
```

is fixed to

```php
<?php

$param1 = 'Hello';
$param2 = 'World!';

return "$param1ooo　$param2";
```


Pull request:
fix: `no_useless_concat_operator` - do not break variable
Fixes #7826

Patch:
diff --git a/src/Fixer/Operator/NoUselessConcatOperatorFixer.php b/src/Fixer/Operator/NoUselessConcatOperatorFixer.php
index 6d9629b5e..79c35e7cd 100644
--- a/src/Fixer/Operator/NoUselessConcatOperatorFixer.php
+++ b/src/Fixer/Operator/NoUselessConcatOperatorFixer.php
@@ -26,6 +26,13 @@
 use PhpCsFixer\Tokenizer\Token;
 use PhpCsFixer\Tokenizer\Tokens;
 
+/**
+ * @phpstan-type _ConcatOperandType array{
+ *     start: int,
+ *     end: int,
+ *     type: self::STR_*,
+ * }
+ */
 final class NoUselessConcatOperatorFixer extends AbstractFixer implements ConfigurableFixerInterface
 {
     private const STR_DOUBLE_QUOTE = 0;
@@ -47,7 +54,7 @@ public function getDefinition(): FixerDefinitionInterface
      * {@inheritdoc}
      *
      * Must run before DateTimeCreateFromFormatCallFixer, EregToPregFixer, PhpUnitDedicateAssertInternalTypeFixer, RegularCallableCallFixer, SetTypeToCastFixer.
-     * Must run after NoBinaryStringFixer, SingleQuoteFixer.
+     * Must run after ExplicitStringVariableFixer, NoBinaryStringFixer, SingleQuoteFixer.
      */
     public function getPriority(): int
     {
@@ -105,16 +112,8 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
     }
 
     /**
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $firstOperand
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $secondOperand
+     * @param _ConcatOperandType $firstOperand
+     * @param _ConcatOperandType $secondOperand
      */
     private function fixConcatOperation(Tokens $tokens, array $firstOperand, int $concatIndex, array $secondOperand): void
     {
@@ -130,6 +129,10 @@ private function fixConcatOperation(Tokens $tokens, array $firstOperand, int $co
         }
 
         if (self::STR_DOUBLE_QUOTE_VAR === $firstOperand['type'] && self::STR_DOUBLE_QUOTE_VAR === $secondOperand['type']) {
+            if ($this->operandsCanNotBeMerged($tokens, $firstOperand, $secondOperand)) {
+                return;
+            }
+
             $this->mergeConstantEscapedStringVarOperands($tokens, $firstOperand, $concatIndex, $secondOperand);
 
             return;
@@ -146,6 +149,10 @@ private function fixConcatOperation(Tokens $tokens, array $firstOperand, int $co
             [$operand1, $operand2] = $operandPair;
 
             if (self::STR_DOUBLE_QUOTE_VAR === $operand1['type'] && self::STR_DOUBLE_QUOTE === $operand2['type']) {
+                if ($this->operandsCanNotBeMerged($tokens, $operand1, $operand2)) {
+                    return;
+                }
+
                 $this->mergeConstantEscapedStringVarOperands($tokens, $firstOperand, $concatIndex, $secondOperand);
 
                 return;
@@ -169,6 +176,10 @@ private function fixConcatOperation(Tokens $tokens, array $firstOperand, int $co
                 $operantContent = $tokens[$operand2['start']]->getContent();
 
                 if ($this->isSimpleQuotedStringContent($operantContent)) {
+                    if ($this->operandsCanNotBeMerged($tokens, $operand1, $operand2)) {
+                        return;
+                    }
+
                     $this->mergeConstantEscapedStringVarOperands($tokens, $firstOperand, $concatIndex, $secondOperand);
                 }
 
@@ -180,11 +191,7 @@ private function fixConcatOperation(Tokens $tokens, array $firstOperand, int $co
     /**
      * @param -1|1 $direction
      *
-     * @return null|array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * }
+     * @return null|_ConcatOperandType
      */
     private function getConcatOperandType(Tokens $tokens, int $index, int $direction): ?array
     {
@@ -216,16 +223,8 @@ private function getConcatOperandType(Tokens $tokens, int $index, int $direction
     }
 
     /**
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $firstOperand
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $secondOperand
+     * @param _ConcatOperandType $firstOperand
+     * @param _ConcatOperandType $secondOperand
      */
     private function mergeConstantEscapedStringOperands(
         Tokens $tokens,
@@ -249,16 +248,8 @@ private function mergeConstantEscapedStringOperands(
     }
 
     /**
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $firstOperand
-     * @param array{
-     *     start: int,
-     *     end: int,
-     *     type: self::STR_*,
-     * } $secondOperand
+     * @param _ConcatOperandType $firstOperand
+     * @param _ConcatOperandType $secondOperand
      */
     private function mergeConstantEscapedStringVarOperands(
         Tokens $tokens,
@@ -266,7 +257,7 @@ private function mergeConstantEscapedStringVarOperands(
         int $concatOperatorIndex,
         array $secondOperand
     ): void {
-        // build uo the new content
+        // build up the new content
         $newContent = '';
 
         foreach ([$firstOperand, $secondOperand] as $operant) {
@@ -336,4 +327,36 @@ private function containsLinebreak(Tokens $tokens, int $startIndex, int $endInde
 
         return false;
     }
+
+    /**
+     * @param _ConcatOperandType $firstOperand
+     * @param _ConcatOperandType $secondOperand
+     */
+    private function operandsCanNotBeMerged(Tokens $tokens, array $firstOperand, array $secondOperand): bool
+    {
+        // If the first operand does not end with a variable, no variables would be broken by concatenation.
+        if (self::STR_DOUBLE_QUOTE_VAR !== $firstOperand['type']) {
+            return false;
+        }
+        if (!$tokens[$firstOperand['end'] - 1]->isGivenKind(T_VARIABLE)) {
+            return false;
+        }
+
+        $allowedPatternsForSecondOperand = [
+            '/^\s.*/', // e.g. " foo", ' bar', " $baz"
+            '/^-(?!\>)/', // e.g. "-foo", '-bar', "-$baz"
+        ];
+
+        // If the first operand ends with a variable, the second operand should match one of the allowed patterns.
+        // Otherwise, the concatenation can break a variable in the first operand.
+        foreach ($allowedPatternsForSecondOperand as $allowedPattern) {
+            $secondOperandInnerContent = substr($tokens->generatePartialCode($secondOperand['start'], $secondOperand['end']), 1, -1);
+
+            if (Preg::match($allowedPattern, $secondOperandInnerContent)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }
diff --git a/src/Fixer/StringNotation/ExplicitStringVariableFixer.php b/src/Fixer/StringNotation/ExplicitStringVariableFixer.php
index ca0c741f9..5194a72e8 100644
--- a/src/Fixer/StringNotation/ExplicitStringVariableFixer.php
+++ b/src/Fixer/StringNotation/ExplicitStringVariableFixer.php
@@ -52,11 +52,12 @@ public function getDefinition(): FixerDefinitionInterface
     /**
      * {@inheritdoc}
      *
+     * Must run before NoUselessConcatOperatorFixer.
      * Must run after BacktickToShellExecFixer.
      */
     public function getPriority(): int
     {
-        return 0;
+        return 6;
     }
 
     public function isCandidate(Tokens $tokens): bool


Test patch:
diff --git a/tests/AutoReview/FixerFactoryTest.php b/tests/AutoReview/FixerFactoryTest.php
index 3d789112a..f9188f337 100644
--- a/tests/AutoReview/FixerFactoryTest.php
+++ b/tests/AutoReview/FixerFactoryTest.php
@@ -442,6 +442,9 @@ private static function getFixersPriorityGraph(): array
                 'heredoc_to_nowdoc',
                 'single_quote',
             ],
+            'explicit_string_variable' => [
+                'no_useless_concat_operator',
+            ],
             'final_class' => [
                 'protected_to_private',
                 'self_static_accessor',
diff --git a/tests/Fixer/Operator/NoUselessConcatOperatorFixerTest.php b/tests/Fixer/Operator/NoUselessConcatOperatorFixerTest.php
index 7e9a27adc..cee941425 100644
--- a/tests/Fixer/Operator/NoUselessConcatOperatorFixerTest.php
+++ b/tests/Fixer/Operator/NoUselessConcatOperatorFixerTest.php
@@ -136,6 +136,53 @@ public static function provideFixCases(): iterable
             ',
         ];
 
+        yield 'do not fix if the execution result would be different' => [
+            '<?php
+                echo "abc $d" . "[$e]";
+                echo "abc $d" . "[3]";
+                echo "abc $d" . \'[3]\';
+                echo "abc $d" . "->e";
+                echo "abc $d" . \'->e\';
+                echo "abc $d" . "->$e";
+                echo "abc $d" . "?->e";
+                echo "abc $d" . "?->$e";
+                echo "abc $d" . \'?->e\';
+            ',
+        ];
+
+        yield 'do not fix if variables would be broken' => [
+            '<?php
+                echo "abc $d" . "e $f";
+                echo "abc $d" . "e";
+                echo "abc $d" . \'e\';
+                echo "abc $d" . "😃"; // with emoji
+                echo "私の名前は$name" . "です"; // multibyte characters (Japanese)
+            ',
+        ];
+
+        yield 'fix if variables would not be broken' => [
+            '<?php
+                echo "$a b";
+                echo "$a bcde";
+                echo "abc ${d}e";
+                echo "abc $d-efg";
+                echo "$a bcdef";
+                echo "abc ${d}ef";
+                echo "abc $d-efgh";
+                echo "abc $d-$e";
+            ',
+            '<?php
+                echo "$a" . " b";
+                echo "$a bc" . "de";
+                echo "abc ${d}" . "e";
+                echo "abc $d" . "-efg";
+                echo "$a bc" . \'def\';
+                echo "abc ${d}" . \'ef\';
+                echo "abc $d" . \'-efgh\';
+                echo "abc $d" . "-$e";
+            ',
+        ];
+
         yield 'single quote concat single quote but with line break after' => [
             "<?php \$fh = 'x'. // some comment
 'y';",
diff --git a/tests/Fixtures/Integration/priority/explicit_string_variable,no_useless_concat_operator.test b/tests/Fixtures/Integration/priority/explicit_string_variable,no_useless_concat_operator.test
new file mode 100644
index 000000000..e0c5821ac
--- /dev/null
+++ b/tests/Fixtures/Integration/priority/explicit_string_variable,no_useless_concat_operator.test
@@ -0,0 +1,13 @@
+--TEST--
+Integration of fixers: explicit_string_variable,no_useless_concat_operator.
+--RULESET--
+{"no_useless_concat_operator": true, "explicit_string_variable": true}
+--EXPECT--
+<?php
+
+$foo = "bar {$baz}qux";
+
+--INPUT--
+<?php
+
+$foo = "bar $baz" . "qux";