Browse Source

minor #2812 ProcessLinter - compatibility with Symfony 3.3 (keradus)

This PR was squashed before being merged into the 2.2 branch (closes #2812).

Discussion
----------

ProcessLinter - compatibility with Symfony 3.3

Sadly, 3.3 introduced new deprecation (#21474).

One can not operate with `Process` directly to have same code on 3.2 and 3.3 that is not triggering the deprecation on 3.3 (as to avoid triggering deprecation one must to pass array to Process, which is not possible on 3.2). For that, I go behind `ProcessBuilder`, which has same interface between 3.2 and 3.3...

Commits
-------

d0a13a05 ProcessLinter - compatibility with Symfony 3.3
Dariusz Ruminski 7 years ago
parent
commit
ecc5b49e80
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);
-    }
 }