Browse Source

ProcessLinter - compatibility with Symfony 3.3

Dariusz Ruminski 7 years ago
parent
commit
d0a13a052f
2 changed files with 35 additions and 47 deletions
  1. 7 12
      src/Linter/ProcessLinter.php
  2. 28 35
      tests/Linter/ProcessLinterTest.php

+ 7 - 12
src/Linter/ProcessLinter.php

@@ -16,7 +16,7 @@ use PhpCsFixer\FileRemoval;
 use Symfony\Component\Filesystem\Exception\IOException;
 use Symfony\Component\Process\PhpExecutableFinder;
 use Symfony\Component\Process\Process;
-use Symfony\Component\Process\ProcessUtils;
+use Symfony\Component\Process\ProcessBuilder;
 
 /**
  * Handle PHP code linting process.
@@ -125,7 +125,7 @@ final class ProcessLinter implements LinterInterface
             return $this->createProcessForSource(file_get_contents($path));
         }
 
-        $process = new Process($this->prepareCommand($path));
+        $process = $this->prepareProcess($path);
         $process->setTimeout(null);
         $process->start();
 
@@ -154,22 +154,17 @@ final class ProcessLinter implements LinterInterface
     }
 
     /**
-     * Prepare command that will lint a file.
-     *
      * @param string $path
      *
-     * @return string
+     * @return Process
      */
-    private function prepareCommand($path)
+    private function prepareProcess($path)
     {
-        $executable = ProcessUtils::escapeArgument($this->executable);
-
+        $arguments = array('-l', $path);
         if (defined('HHVM_VERSION')) {
-            $executable .= ' --php';
+            array_unshift($arguments, '--php');
         }
 
-        $path = ProcessUtils::escapeArgument($path);
-
-        return sprintf('%s -l %s', $executable, $path);
+        return ProcessBuilder::create($arguments)->setPrefix($this->executable)->getProcess();
     }
 }

+ 28 - 35
tests/Linter/ProcessLinterTest.php

@@ -14,7 +14,6 @@ namespace PhpCsFixer\Tests\Linter;
 
 use PhpCsFixer\Linter\ProcessLinter;
 use PhpCsFixer\Test\AccessibleObject;
-use Symfony\Component\Process\ProcessUtils;
 
 /**
  * @author Dariusz Rumiński <dariusz.ruminski@gmail.com>
@@ -31,20 +30,41 @@ final class ProcessLinterTest extends AbstractLinterTestCase
         $this->assertTrue($this->createLinter()->isAsync());
     }
 
-    public function testPrepareCommandOnPhp()
+    /**
+     * @param string $executable
+     * @param string $file
+     * @param string $expected
+     *
+     * @testWith ["php", "foo.php", "'php' '-l' 'foo.php'"]
+     *           ["C:\\Program Files\\php\\php.exe", "foo bar\\baz.php", "'C:\\Program Files\\php\\php.exe' '-l' 'foo bar\\baz.php'"]
+     * @requires OS Linux
+     */
+    public function testPrepareCommandOnPhpOnLinux($executable, $file, $expected)
     {
         if (defined('HHVM_VERSION')) {
             $this->markTestSkipped('Skip tests for PHP compiler when running on HHVM compiler.');
         }
 
         $this->assertSame(
-            $this->fixEscape('"php" -l "foo.php"'),
-            AccessibleObject::create(new ProcessLinter('php'))->prepareCommand('foo.php')
+            $expected,
+            AccessibleObject::create(new ProcessLinter($executable))->prepareProcess($file)->getCommandLine()
         );
+    }
 
+    /**
+     * @param string $executable
+     * @param string $file
+     * @param string $expected
+     *
+     * @testWith ["php", "foo.php", "php -l foo.php"]
+     *           ["C:\\Program Files\\php\\php.exe", "foo bar\\baz.php", "\"C:\\Program Files\\php\\php.exe\" -l \"foo bar\\baz.php\""]
+     * @requires OS Win
+     */
+    public function testPrepareCommandOnPhpOnWindows($executable, $file, $expected)
+    {
         $this->assertSame(
-            $this->fixEscape('"C:\Program Files\php\php.exe" -l "foo bar\baz.php"'),
-            AccessibleObject::create(new ProcessLinter('C:\Program Files\php\php.exe'))->prepareCommand('foo bar\baz.php')
+            $expected,
+            AccessibleObject::create(new ProcessLinter($executable))->prepareProcess($file)->getCommandLine()
         );
     }
 
@@ -55,8 +75,8 @@ final class ProcessLinterTest extends AbstractLinterTestCase
         }
 
         $this->assertSame(
-            $this->fixEscape('"hhvm" --php -l "foo.php"'),
-            AccessibleObject::create(new ProcessLinter('hhvm'))->prepareCommand('foo.php')
+            "'hhvm' '--php' '-l' 'foo.php'",
+            AccessibleObject::create(new ProcessLinter('hhvm'))->prepareProcess('foo.php')->getCommandLine()
         );
     }
 
@@ -67,31 +87,4 @@ final class ProcessLinterTest extends AbstractLinterTestCase
     {
         return new ProcessLinter();
     }
-
-    /**
-     * Fix escaping character.
-     *
-     * Escape character may be different on various environments.
-     * This method change used escape character into character that is default
-     * for environment.
-     *
-     * @param string $value          value to be fixed
-     * @param string $usedEscapeChar used escape char, may be only ' or "
-     *
-     * @return string
-     */
-    private function fixEscape($value, $usedEscapeChar = '"')
-    {
-        static $escapeChar = null;
-
-        if (null === $escapeChar) {
-            $escapeChar = substr(ProcessUtils::escapeArgument('x'), 0, 1);
-        }
-
-        if ($usedEscapeChar === $escapeChar) {
-            return $value;
-        }
-
-        return str_replace($usedEscapeChar, $escapeChar, $value);
-    }
 }