Allow ArcanistExternalLinter flags to be specified as an array.

Summary: Personally, I prefer to specify command lines flags as an array rather than a string.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aran, epriestley, Korvin, chad

Differential Revision: https://secure.phabricator.com/D8387

src/lint/linter/ArcanistCSSLintLinter.php

@@ -19,13 +19,13 @@ final class ArcanistCSSLintLinter extends ArcanistExternalLinter {
   }
 
   public function getMandatoryFlags() {
-    return '--format=lint-xml';
+    return array('--format=lint-xml');
   }
 
   public function getDefaultFlags() {
     // TODO: Deprecation warning.
     $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.csslint.options');
+    return $config->getConfigFromAnySource('lint.csslint.options', array());
   }
 
   public function getDefaultBinary() {

src/lint/linter/ArcanistExternalLinter.php

@@ -117,11 +117,11 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
    * Flags which are not mandatory should be provided in
    * @{method:getDefaultFlags} instead.
    *
-   * @return string|null  Mandatory flags, like `"--format=xml"`.
+   * @return list<string>  Mandatory flags, like `"--format=xml"`.
    * @task bin
    */
   protected function getMandatoryFlags() {
-    return null;
+    return array();
   }
 
 
@@ -133,11 +133,11 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
    *
    * Default flags can be overridden with @{method:setFlags}.
    *
-   * @return string|null  Overridable default flags.
+   * @return list<string>  Overridable default flags.
    * @task bin
    */
   protected function getDefaultFlags() {
-    return null;
+    return array();
   }
 
 
@@ -145,12 +145,12 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
    * Override default flags with custom flags. If not overridden, flags provided
    * by @{method:getDefaultFlags} are used.
    *
-   * @param string New flags.
+   * @param list<string> New flags.
    * @return this
    * @task bin
    */
   final public function setFlags($flags) {
-    $this->flags = $flags;
+    $this->flags = (array) $flags;
     return $this;
   }
 
@@ -348,21 +348,32 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
    * Get the composed flags for the executable, including both mandatory and
    * configured flags.
    *
-   * @return string Composed flags.
+   * @return list<string> Composed flags.
    * @task exec
    */
   protected function getCommandFlags() {
-    return csprintf(
+    $mandatory_flags = $this->getMandatoryFlags();
-      '%C %C',
+    if (!is_array($mandatory_flags)) {
-      $this->getMandatoryFlags(),
+      phutil_deprecated(
-      coalesce($this->flags, $this->getDefaultFlags()));
+        'String support for flags.', 'You should use list<string> instead.');
+      $mandatory_flags = (array) $mandatory_flags;
+    }
+
+    $flags = nonempty($this->flags, $this->getDefaultFlags());
+    if (!is_array($flags)) {
+      phutil_deprecated(
+        'String support for flags.', 'You should use list<string> instead.');
+      $flags = (array) $flags;
+    }
+
+    return array_merge($mandatory_flags, $flags);
   }
 
 
   protected function buildFutures(array $paths) {
     $executable = $this->getExecutableCommand();
 
-    $bin = csprintf('%C %C', $executable, $this->getCommandFlags());
+    $bin = csprintf('%C %Ls', $executable, $this->getCommandFlags());
 
     $futures = array();
     foreach ($paths as $path) {
@@ -410,7 +421,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
   public function getLinterConfigurationOptions() {
     $options = array(
       'bin' => 'optional string | list<string>',
-      'flags' => 'optional string',
+      'flags' => 'optional list<string>',
     );
 
     if ($this->shouldUseInterpreter()) {
@@ -465,9 +476,13 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
         throw new Exception(
           pht('None of the configured binaries can be located.'));
       case 'flags':
-        if (strlen($value)) {
+        if (!is_array($value)) {
-          $this->setFlags($value);
+          phutil_deprecated(
+            'String support for flags.',
+            'You should use list<string> instead.');
+          $value = (array) $value;
         }
+        $this->setFlags($value);
         return;
     }
 

src/lint/linter/ArcanistFlake8Linter.php

@@ -19,7 +19,7 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter {
   public function getDefaultFlags() {
     // TODO: Deprecated.
     $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.flake8.options', '');
+    return $config->getConfigFromAnySource('lint.flake8.options', array());
   }
 
   public function getDefaultBinary() {

src/lint/linter/ArcanistPEP8Linter.php

@@ -17,7 +17,7 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter {
 
   public function getCacheVersion() {
     list($stdout) = execx('%C --version', $this->getExecutableCommand());
-    return $stdout.$this->getCommandFlags();
+    return $stdout.implode(' ', $this->getCommandFlags());
   }
 
   public function getDefaultFlags() {
@@ -25,7 +25,7 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter {
     $config = $this->getEngine()->getConfigurationManager();
     return $config->getConfigFromAnySource(
       'lint.pep8.options',
-      $this->getConfig('options'));
+      $this->getConfig('options', array()));
   }
 
   public function shouldUseInterpreter() {

src/lint/linter/ArcanistPhpcsLinter.php

@@ -26,7 +26,7 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
   }
 
   public function getMandatoryFlags() {
-    return '--report=xml';
+    return array('--report=xml');
   }
 
   public function getInstallInstructions() {
@@ -38,10 +38,16 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
 
     $config = $this->getEngine()->getConfigurationManager();
 
-    $options = $config->getConfigFromAnySource('lint.phpcs.options');
+    $options = $config->getConfigFromAnySource('lint.phpcs.options', array());
 
     $standard = $config->getConfigFromAnySource('lint.phpcs.standard');
-    $options .= !empty($standard) ? ' --standard=' . $standard : '';
+    if (!empty($standard)) {
+      if (is_array($options)) {
+        $options[] = '--standard='.$standard;
+      } else {
+        $options .= ' --standard='.$standard;
+      }
+    }
 
     return $options;
   }

src/lint/linter/ArcanistRubyLinter.php

@@ -41,7 +41,7 @@ final class ArcanistRubyLinter extends ArcanistExternalLinter {
   protected function getMandatoryFlags() {
     // -w: turn on warnings
     // -c: check syntax
-    return '-w -c';
+    return array('-w', '-c');
   }
 
   protected function getDefaultMessageSeverity($code) {