Browse Source

ExplicitStringVariableFixer - fix case of 2 variables next to each other

Filippo Tessarotto 6 years ago
parent
commit
88374e7869

+ 50 - 24
src/Fixer/StringNotation/ExplicitStringVariableFixer.php

@@ -45,6 +45,7 @@ EOT
                 ."\n".'- PHP manual marks `"$var"` syntax as implicit and `"${var}"` syntax as explicit: explicit code should always be preferred'
                 ."\n".'- Explicit syntax allows word concatenation inside strings, e.g. `"${var}IsAVar"`, implicit doesn\'t'
                 ."\n".'- Explicit syntax is easier to detect for IDE/editors and therefore has colors/hightlight with higher contrast, which is easier to read'
+            ."\n".'Backtick operator is skipped because it is harder to handle; you can use `backtick_to_shell_exec` fixer to normalize backticks to strings'
         );
     }
 
@@ -61,9 +62,16 @@ EOT
      */
     protected function applyFix(\SplFileInfo $file, Tokens $tokens)
     {
+        $backtickStarted = false;
         for ($index = \count($tokens) - 1; $index > 0; --$index) {
             $token = $tokens[$index];
-            if (!$token->isGivenKind(T_VARIABLE)) {
+            if ($token->equals('`')) {
+                $backtickStarted = !$backtickStarted;
+
+                continue;
+            }
+
+            if ($backtickStarted || !$token->isGivenKind(T_VARIABLE)) {
                 continue;
             }
 
@@ -72,40 +80,58 @@ EOT
                 continue;
             }
 
+            $distinctVariableIndex = $index;
             $variableTokens = [
-                $index => $token,
+                $distinctVariableIndex => [
+                    'tokens' => [$index => $token],
+                    'firstVariableTokenIndex' => $index,
+                    'lastVariableTokenIndex' => $index,
+                ],
             ];
-            $firstVariableTokenIndex = $index;
-            $lastVariableTokenIndex = $index;
 
             $nextIndex = $index + 1;
             while (!$this->isStringPartToken($tokens[$nextIndex])) {
-                $variableTokens[$nextIndex] = $tokens[$nextIndex];
-                $lastVariableTokenIndex = $nextIndex;
+                if ($tokens[$nextIndex]->isGivenKind(T_VARIABLE)) {
+                    $distinctVariableIndex = $nextIndex;
+                    $variableTokens[$distinctVariableIndex] = [
+                        'tokens' => [$nextIndex => $tokens[$nextIndex]],
+                        'firstVariableTokenIndex' => $nextIndex,
+                        'lastVariableTokenIndex' => $nextIndex,
+                    ];
+                } else {
+                    $variableTokens[$distinctVariableIndex]['tokens'][$nextIndex] = $tokens[$nextIndex];
+                    $variableTokens[$distinctVariableIndex]['lastVariableTokenIndex'] = $nextIndex;
+                }
+
                 ++$nextIndex;
             }
+            krsort($variableTokens, \SORT_NUMERIC);
 
-            if (1 === \count($variableTokens)) {
-                $tokens->overrideRange($index, $index, [
-                    new Token([T_DOLLAR_OPEN_CURLY_BRACES, '${']),
-                    new Token([T_STRING_VARNAME, substr($token->getContent(), 1)]),
-                    new Token([CT::T_DOLLAR_CLOSE_CURLY_BRACES, '}']),
-                ]);
-            } else {
-                foreach ($variableTokens as $variablePartIndex => $variablePartToken) {
-                    if ($variablePartToken->isGivenKind(T_NUM_STRING)) {
-                        $tokens[$variablePartIndex] = new Token([T_LNUMBER, $variablePartToken->getContent()]);
-
-                        continue;
-                    }
+            foreach ($variableTokens as $distinctVariableSet) {
+                if (1 === \count($distinctVariableSet['tokens'])) {
+                    $singleVariableIndex = key($distinctVariableSet['tokens']);
+                    $singleVariableToken = current($distinctVariableSet['tokens']);
+                    $tokens->overrideRange($singleVariableIndex, $singleVariableIndex, [
+                        new Token([T_DOLLAR_OPEN_CURLY_BRACES, '${']),
+                        new Token([T_STRING_VARNAME, substr($singleVariableToken->getContent(), 1)]),
+                        new Token([CT::T_DOLLAR_CLOSE_CURLY_BRACES, '}']),
+                    ]);
+                } else {
+                    foreach ($distinctVariableSet['tokens'] as $variablePartIndex => $variablePartToken) {
+                        if ($variablePartToken->isGivenKind(T_NUM_STRING)) {
+                            $tokens[$variablePartIndex] = new Token([T_LNUMBER, $variablePartToken->getContent()]);
+
+                            continue;
+                        }
 
-                    if ($variablePartToken->isGivenKind(T_STRING) && $tokens[$variablePartIndex + 1]->equals(']')) {
-                        $tokens[$variablePartIndex] = new Token([T_CONSTANT_ENCAPSED_STRING, "'".$variablePartToken->getContent()."'"]);
+                        if ($variablePartToken->isGivenKind(T_STRING) && $tokens[$variablePartIndex + 1]->equals(']')) {
+                            $tokens[$variablePartIndex] = new Token([T_CONSTANT_ENCAPSED_STRING, "'".$variablePartToken->getContent()."'"]);
+                        }
                     }
-                }
 
-                $tokens->insertAt($lastVariableTokenIndex + 1, new Token([CT::T_CURLY_CLOSE, '}']));
-                $tokens->insertAt($firstVariableTokenIndex, new Token([T_CURLY_OPEN, '{']));
+                    $tokens->insertAt($distinctVariableSet['lastVariableTokenIndex'] + 1, new Token([CT::T_CURLY_CLOSE, '}']));
+                    $tokens->insertAt($distinctVariableSet['firstVariableTokenIndex'], new Token([T_CURLY_OPEN, '{']));
+                }
             }
         }
     }

+ 1 - 0
tests/AutoReview/FixerFactoryTest.php

@@ -64,6 +64,7 @@ final class FixerFactoryTest extends TestCase
             [$fixers['array_syntax'], $fixers['binary_operator_spaces']],
             [$fixers['array_syntax'], $fixers['ternary_operator_spaces']],
             [$fixers['backtick_to_shell_exec'], $fixers['escape_implicit_backslashes']],
+            [$fixers['backtick_to_shell_exec'], $fixers['explicit_string_variable']],
             [$fixers['blank_line_after_opening_tag'], $fixers['no_blank_lines_before_namespace']],
             [$fixers['braces'], $fixers['array_indentation']],
             [$fixers['braces'], $fixers['method_chaining_indentation']],

+ 20 - 1
tests/Fixer/StringNotation/ExplicitStringVariableFixerTest.php

@@ -52,7 +52,7 @@ final class ExplicitStringVariableFixerTest extends AbstractFixerTestCase
                 '<?php $a = "My name is $name!";',
             ],
             [
-                '<?php "My name is {$james$bond}!";',
+                '<?php "My name is ${james}${bond}!";',
                 '<?php "My name is $james$bond!";',
             ],
             [
@@ -133,6 +133,10 @@ EOF;
                 '<?php $a = "Complex array chaining not allowed {$array[1]}[2][MY_CONSTANT] text";',
                 '<?php $a = "Complex array chaining not allowed $array[1][2][MY_CONSTANT] text";',
             ],
+            [
+                '<?php $a = "Concatenation: ${james}${bond}{$object->property}{$array[1]}!";',
+                '<?php $a = "Concatenation: $james$bond$object->property$array[1]!";',
+            ],
             [
                 '<?php $a = "{$a->b} start";',
                 '<?php $a = "$a->b start";',
@@ -181,6 +185,21 @@ EOF;
                 '<?php $a = B"end {$a[1]}";',
                 '<?php $a = B"end $a[1]";',
             ],
+            [
+                '<?php $a = "*{$a[0]}{$b[1]}X{$c[2]}{$d[3]}";',
+                '<?php $a = "*$a[0]$b[1]X$c[2]$d[3]";',
+            ],
+            [
+                '<?php $a = `echo $foo`;',
+            ],
+            [
+                '<?php $a = "My name is ${name}!"; $a = `echo $foo`; $a = "{$a->b} start";',
+                '<?php $a = "My name is $name!"; $a = `echo $foo`; $a = "$a->b start";',
+            ],
+            [
+                '<?php $mobileNumberVisible = "***-***-{$last4Digits[0]}{$last4Digits[1]}-{$last4Digits[2]}{$last4Digits[3]}";',
+                '<?php $mobileNumberVisible = "***-***-$last4Digits[0]$last4Digits[1]-$last4Digits[2]$last4Digits[3]";',
+            ],
         ];
     }
 

+ 11 - 0
tests/Fixtures/Integration/priority/backtick_to_shell_exec,explicit_string_variable.test

@@ -0,0 +1,11 @@
+--TEST--
+Integration of fixers: backtick_to_shell_exec,explicit_string_variable.
+--RULESET--
+{"backtick_to_shell_exec": true, "explicit_string_variable": true}
+--EXPECT--
+<?php
+$a = shell_exec("echo ${foo}");
+
+--INPUT--
+<?php
+$a = `echo $foo`;