(stable) Promote 2015 Week 35

src/lint/linter/ArcanistPyLintLinter.php

@@ -130,10 +130,15 @@ final class ArcanistPyLintLinter extends ArcanistExternalLinter {
         continue;
       }
 
+      // NOTE: PyLint sometimes returns -1 as the character offset for a
+      // message. If it does, treat it as 0. See T9257.
+      $char = (int)$matches[1];
+      $char = max(0, $char);
+
       $message = id(new ArcanistLintMessage())
         ->setPath($path)
         ->setLine($matches[0])
-        ->setChar($matches[1])
+        ->setChar($char)
         ->setCode($matches[2])
         ->setSeverity($this->getLintMessageSeverity($matches[2]))
         ->setName(ucwords(str_replace('-', ' ', $matches[3])))

src/lint/linter/__tests__/pylint/negativechar.lint-test

@@ -0,0 +1,6 @@
+"""Docstring"""
+
+"""
+Useless string """
+~~~~~~~~~~
+warning:4:0 See T9257.

src/lint/linter/__tests__/xml/attr3.lint-test

@@ -1,8 +0,0 @@
-<!DOCTYPE doc [
-<!ELEMENT doc (#PCDATA)>
-<!ATTLIST doc a1 CDATA "v1">
-<!ATTLIST doc a1 CDATA "z1">
-]>
-<doc></doc>
-~~~~~~~~~~
-warning:4:28

src/lint/linter/__tests__/xml/attr4.lint-test

@@ -1,3 +0,0 @@
-<ROOT attr="XY"/>
-~~~~~~~~~~
-error:1:15

src/lint/linter/__tests__/xml/languages-7.lint-test

@@ -1,7 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<languages>
-    <lang>
-</languages>
-~~~~~~~~~~
-error:4:7
-error:5:1

src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php

@@ -327,7 +327,7 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule
 
     $ternaries = $root->selectDescendantsOfType('n_TERNARY_EXPRESSION');
     foreach ($ternaries as $ternary) {
-      $yes = $ternary->getChildByIndex(1);
+      $yes = $ternary->getChildByIndex(2);
       if ($yes->getTypeName() === 'n_EMPTY') {
         $this->raiseLintAtNode(
           $ternary,

src/repository/api/ArcanistGitAPI.php

@@ -51,6 +51,11 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return 'git';
   }
 
+  public function getGitVersion() {
+    list($stdout) = $this->execxLocal('--version');
+    return rtrim(str_replace('git version ', '', $stdout));
+  }
+
   public function getMetadataPath() {
     static $path = null;
     if ($path === null) {
@@ -482,35 +487,61 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return $stdout;
   }
 
-  public function getBranchName() {
-    // TODO: consider:
-    //
-    //    $ git rev-parse --abbrev-ref `git symbolic-ref HEAD`
-    //
-    // But that may fail if you're not on a branch.
-    list($stdout) = $this->execxLocal('branch --no-color');
-
-    // Assume that any branch beginning with '(' means 'no branch', or whatever
-    // 'no branch' is in the current locale.
-    $matches = null;
-    if (preg_match('/^\* ([^\(].*)$/m', $stdout, $matches)) {
-      return $matches[1];
+  private function getBranchNameFromRef($ref) {
+    $count = 0;
+    $branch = preg_replace('/^refs\/heads\//', '', $ref, 1, $count);
+    if ($count !== 1) {
+      return null;
     }
 
+    return $branch;
+  }
+
+  public function getBranchName() {
+    list($err, $stdout, $stderr) = $this->execManualLocal(
+      'symbolic-ref --quiet HEAD');
+
+    if ($err === 0) {
+      // We expect the branch name to come qualified with a refs/heads/ prefix.
+      // Verify this, and strip it.
+      $ref = rtrim($stdout);
+      $branch = $this->getBranchNameFromRef($ref);
+      if (!$branch) {
+        throw new Exception(
+          pht('Failed to parse %s output!', 'git symbolic-ref'));
+      }
+      return $branch;
+    } else if ($err === 1) {
+      // Exit status 1 with --quiet indicates that HEAD is detached.
       return null;
+    } else {
+      throw new Exception(
+        pht('Command %s failed: %s', 'git symbolic-ref', $stderr));
+    }
   }
 
   public function getRemoteURI() {
-    list($stdout) = $this->execxLocal('remote show -n origin');
-
-    $matches = null;
-    if (preg_match('/^\s*Fetch URL: (.*)$/m', $stdout, $matches)) {
-      return trim($matches[1]);
+    // "git ls-remote --get-url" is the appropriate plumbing to get the remote
+    // URI. "git config remote.origin.url", on the other hand, may not be as
+    // accurate (for example, it does not take into account possible URL
+    // rewriting rules set by the user through "url.<base>.insteadOf"). However,
+    // the --get-url flag requires git 1.7.5.
+    $version = $this->getGitVersion();
+    if (version_compare($version, '1.7.5', '>=')) {
+      list($stdout) = $this->execxLocal('ls-remote --get-url origin');
+    } else {
+      list($stdout) = $this->execxLocal('config remote.origin.url');
     }
 
+    $uri = rtrim($stdout);
+    // 'origin' is what ls-remote outputs if no origin remote URI exists
+    if (!$uri || $uri === 'origin') {
       return null;
     }
 
+    return $uri;
+  }
+
   public function getSourceControlPath() {
     // TODO: Try to get something useful here.
     return null;
@@ -780,37 +811,42 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
   }
 
   public function getBlame($path) {
-    // TODO: 'git blame' supports --porcelain and we should probably use it.
     list($stdout) = $this->execxLocal(
-      'blame --date=iso -w -M %s -- %s',
+      'blame --porcelain -w -M %s -- %s',
       $this->getBaseCommit(),
       $path);
 
+    // the --porcelain format prints at least one header line per source line,
+    // then the source line prefixed by a tab character
+    $blame_info = preg_split('/^\t.*\n/m', rtrim($stdout));
+
+    // commit info is not repeated in these headers, so cache it
+    $revision_data = array();
+
     $blame = array();
-    foreach (explode("\n", trim($stdout)) as $line) {
-      if (!strlen($line)) {
-        continue;
+    foreach ($blame_info as $line_info) {
+      $revision = substr($line_info, 0, 40);
+      $data = idx($revision_data, $revision, array());
+
+      if (empty($data)) {
+        $matches = array();
+        if (!preg_match('/^author (.*)$/m', $line_info, $matches)) {
+          throw new Exception(
+            pht(
+              'Unexpected output from %s: no author for commit %s',
+              'git blame',
+              $revision));
+        }
+        $data['author'] = $matches[1];
+        $data['from_first_commit'] = preg_match('/^boundary$/m', $line_info);
+        $revision_data[$revision] = $data;
       }
 
-      // lines predating a git repo's history are blamed to the oldest revision,
-      // with the commit hash prepended by a ^. we shouldn't count these lines
-      // as blaming to the oldest diff's unfortunate author
-      if ($line[0] == '^') {
-        continue;
+      // Ignore lines predating the git repository (on a boundary commit)
+      // rather than blaming them on the oldest diff's unfortunate author
+      if (!$data['from_first_commit']) {
+        $blame[] = array($data['author'], $revision);
       }
-
-      $matches = null;
-      $ok = preg_match(
-        '/^([0-9a-f]+)[^(]+?[(](.*?) +\d\d\d\d-\d\d-\d\d/',
-        $line,
-        $matches);
-      if (!$ok) {
-        throw new Exception(pht("Bad blame? `%s'", $line));
-      }
-      $revision = $matches[1];
-      $author = $matches[2];
-
-      $blame[] = array($author, $revision);
     }
 
     return $blame;
@@ -886,25 +922,22 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
    * @return list<dict<string, string>> Dictionary of branch information.
    */
   public function getAllBranches() {
-    list($branch_info) = $this->execxLocal(
-      'branch --no-color');
-    $lines = explode("\n", rtrim($branch_info));
+    list($ref_list) = $this->execxLocal(
+      'for-each-ref --format=%s refs/heads',
+      '%(refname)');
+    $refs = explode("\n", rtrim($ref_list));
 
+    $current = $this->getBranchName();
     $result = array();
-    foreach ($lines as $line) {
-
-      if (preg_match('@^[* ]+\(no branch|detached from \w+/\w+\)@', $line)) {
-        // This is indicating that the working copy is in a detached state;
-        // just ignore it.
-        continue;
-      }
-
-      list($current, $name) = preg_split('/\s+/', $line, 2);
+    foreach ($refs as $ref) {
+      $branch = $this->getBranchNameFromRef($ref);
+      if ($branch) {
         $result[] = array(
-        'current' => !empty($current),
-        'name'    => $name,
+          'current' => ($branch === $current),
+          'name'    => $branch,
         );
       }
+    }
 
     return $result;
   }
@@ -1134,11 +1167,28 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
           $commits[] = $merge_base;
 
           $head_branch_count = null;
+          $all_branch_names = ipull($this->getAllBranches(), 'name');
           foreach ($commits as $commit) {
+            // Ideally, we would use something like "for-each-ref --contains"
+            // to get a filtered list of branches ready for script consumption.
+            // Instead, try to get predictable output from "branch --contains".
             list($branches) = $this->execxLocal(
-              'branch --contains %s',
+              '-c column.ui=never -c color.ui=never branch --contains %s',
               $commit);
             $branches = array_filter(explode("\n", $branches));
+
+            // Filter the list, removing the "current" marker (*) and ignoring
+            // anything other than known branch names (mainly, any possible
+            // "detached HEAD" or "no branch" line).
+            foreach ($branches as $key => $branch) {
+              $branch = trim($branch, ' *');
+              if (in_array($branch, $all_branch_names)) {
+                $branches[$key] = $branch;
+              } else {
+                unset($branches[$key]);
+              }
+            }
+
             if ($head_branch_count === null) {
               // If this is the first commit, it's HEAD. Count how many
               // branches it is on; we want to include commits on the same
@@ -1147,9 +1197,6 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
               // for whatever reason.
               $head_branch_count = count($branches);
             } else if (count($branches) > $head_branch_count) {
-              foreach ($branches as $key => $branch) {
-                $branches[$key] = trim($branch, ' *');
-              }
               $branches = implode(', ', $branches);
               $this->setBaseCommitExplanation(
                 pht(

src/workflow/ArcanistLintersWorkflow.php

@@ -11,7 +11,7 @@ final class ArcanistLintersWorkflow extends ArcanistWorkflow {
 
   public function getCommandSynopses() {
     return phutil_console_format(<<<EOTEXT
-      **linters** [__options__]
+      **linters** [__options__] [__name__]
 EOTEXT
       );
   }
@@ -21,6 +21,8 @@ EOTEXT
           Supports: cli
           List the available and configured linters, with information about
           what they do and which versions are installed.
+
+          if __name__ is provided, the linter with that name will be displayed.
 EOTEXT
       ));
   }
@@ -30,6 +32,14 @@ EOTEXT
       'verbose' => array(
         'help' => pht('Show detailed information, including options.'),
       ),
+      'search' => array(
+        'param' => 'search',
+        'repeat' => true,
+        'help' => pht(
+          'Search for linters. Search is case-insensitive, and is performed'.
+          'against name and description of each linter.'),
+      ),
+      '*' => 'exact',
     );
   }
 
@@ -46,6 +56,164 @@ EOTEXT
       $built = array();
     }
 
+    $linter_info = $this->getLintersInfo($linters, $built);
+
+    $status_map = $this->getStatusMap();
+    $pad = '    ';
+
+    $color_map = array(
+      'configured' => 'green',
+      'available' => 'yellow',
+      'error' => 'red',
+    );
+
+    $is_verbose = $this->getArgument('verbose');
+    $exact = $this->getArgument('exact');
+    $search_terms = $this->getArgument('search');
+
+    if ($exact && $search_terms) {
+      throw new ArcanistUsageException(
+        'Specify either search expression or exact name');
+    }
+
+    if ($exact) {
+      $linter_info = $this->findExactNames($linter_info, $exact);
+      if (!$linter_info) {
+        $console->writeOut(
+          "%s\n",
+          pht(
+            'No match found. Try `%s %s` to search for a linter.',
+            'arc linters --search',
+            $exact[0]));
+        return;
+      }
+      $is_verbose = true;
+    }
+
+    if ($search_terms) {
+      $linter_info = $this->filterByNames($linter_info, $search_terms);
+    }
+
+
+    foreach ($linter_info as $key => $linter) {
+      $status = $linter['status'];
+      $color = $color_map[$status];
+      $text = $status_map[$status];
+      $print_tail = false;
+
+      $console->writeOut(
+        "<bg:".$color.">** %s **</bg> **%s** (%s)\n",
+        $text,
+        nonempty($linter['short'], '-'),
+        $linter['name']);
+
+      if ($linter['exception']) {
+        $console->writeOut(
+          "\n%s**%s**\n%s\n",
+          $pad,
+          get_class($linter['exception']),
+          phutil_console_wrap(
+            $linter['exception']->getMessage(),
+            strlen($pad)));
+        $print_tail = true;
+      }
+
+      if ($is_verbose) {
+        $version = $linter['version'];
+        $uri = $linter['uri'];
+        if ($version || $uri) {
+          $console->writeOut("\n");
+          $print_tail = true;
+        }
+
+        if ($version) {
+          $console->writeOut("%s%s **%s**\n", $pad, pht('Version'), $version);
+        }
+
+        if ($uri) {
+          $console->writeOut("%s__%s__\n", $pad, $linter['uri']);
+        }
+
+        $description = $linter['description'];
+        if ($description) {
+          $console->writeOut(
+            "\n%s\n",
+            phutil_console_wrap($linter['description'], strlen($pad)));
+          $print_tail = true;
+        }
+
+        $options = $linter['options'];
+        if ($options) {
+          $console->writeOut(
+            "\n%s**%s**\n\n",
+            $pad,
+            pht('Configuration Options'));
+
+          $last_option = last_key($options);
+          foreach ($options as $option => $option_spec) {
+            $console->writeOut(
+              "%s__%s__ (%s)\n",
+              $pad,
+              $option,
+              $option_spec['type']);
+
+            $console->writeOut(
+              "%s\n",
+              phutil_console_wrap(
+                $option_spec['help'],
+                strlen($pad) + 2));
+
+            if ($option != $last_option) {
+              $console->writeOut("\n");
+            }
+          }
+          $print_tail = true;
+        }
+
+        if ($print_tail) {
+          $console->writeOut("\n");
+        }
+      }
+    }
+
+    if (!$is_verbose) {
+      $console->writeOut(
+        "%s\n",
+        pht('(Run `%s` for more details.)', 'arc linters --verbose'));
+    }
+  }
+
+
+  /**
+   * Get human-readable linter statuses, padded to fixed width.
+   *
+   * @return map<string, string> Human-readable linter status names.
+   */
+  private function getStatusMap() {
+    $text_map = array(
+      'configured' => pht('CONFIGURED'),
+      'available'  => pht('AVAILABLE'),
+      'error'      => pht('ERROR'),
+    );
+
+    $sizes = array();
+    foreach ($text_map as $key => $string) {
+      $sizes[$key] = phutil_utf8_console_strlen($string);
+    }
+
+    $longest = max($sizes);
+    foreach ($text_map as $key => $string) {
+      if ($sizes[$key] < $longest) {
+        $text_map[$key] .= str_repeat(' ', $longest - $sizes[$key]);
+      }
+    }
+
+    $text_map['padding'] = str_repeat(' ', $longest);
+
+    return $text_map;
+  }
+
+  private function getLintersInfo(array $linters, array $built) {
     // Note that an engine can emit multiple linters of the same class to run
     // different rulesets on different groups of files, so these linters do not
     // necessarily have unique classes or types.
@@ -85,131 +253,40 @@ EOTEXT
       );
     }
 
-    $linter_info = isort($linter_info, 'short');
-
-    $status_map = $this->getStatusMap();
-    $pad = '    ';
-
-    $color_map = array(
-      'configured' => 'green',
-      'available' => 'yellow',
-      'error' => 'red',
-    );
-
-    foreach ($linter_info as $key => $linter) {
-      $status = $linter['status'];
-      $color = $color_map[$status];
-      $text = $status_map[$status];
-      $print_tail = false;
-
-      $console->writeOut(
-        "<bg:".$color.">** %s **</bg> **%s** (%s)\n",
-        $text,
-        nonempty($linter['short'], '-'),
-        $linter['name']);
-
-      if ($linter['exception']) {
-        $console->writeOut(
-          "\n%s**%s**\n%s\n",
-          $pad,
-          get_class($linter['exception']),
-          phutil_console_wrap(
-            $linter['exception']->getMessage(),
-            strlen($pad)));
-        $print_tail = true;
+    return isort($linter_info, 'short');
   }
 
-      $version = $linter['version'];
-      $uri = $linter['uri'];
-      if ($version || $uri) {
-        $console->writeOut("\n");
-        $print_tail = true;
-      }
-
-      if ($version) {
-        $console->writeOut("%s%s **%s**\n", $pad, pht('Version'), $version);
-      }
-
-      if ($uri) {
-        $console->writeOut("%s__%s__\n", $pad, $linter['uri']);
-      }
+  private function filterByNames(array $linters, array $search_terms) {
+    $filtered = array();
 
+    foreach ($linters as $key => $linter) {
+      $name = $linter['name'];
+      $short = $linter['short'];
       $description = $linter['description'];
-      if ($description) {
-        $console->writeOut(
-          "\n%s\n",
-          phutil_console_wrap($linter['description'], strlen($pad)));
-        $print_tail = true;
-      }
-
-      $options = $linter['options'];
-      if ($options && $this->getArgument('verbose')) {
-        $console->writeOut(
-          "\n%s**%s**\n\n",
-          $pad,
-          pht('Configuration Options'));
-
-        $last_option = last_key($options);
-        foreach ($options as $option => $option_spec) {
-          $console->writeOut(
-            "%s__%s__ (%s)\n",
-            $pad,
-            $option,
-            $option_spec['type']);
-
-          $console->writeOut(
-            "%s\n",
-            phutil_console_wrap(
-              $option_spec['help'],
-              strlen($pad) + 2));
-
-          if ($option != $last_option) {
-            $console->writeOut("\n");
+      foreach ($search_terms as $term) {
+        if (stripos($name, $term) !== false ||
+            stripos($short, $term) !== false ||
+            stripos($description, $term) !== false) {
+          $filtered[$key] = $linter;
         }
       }
-        $print_tail = true;
+    }
+    return $filtered;
   }
 
-      if ($print_tail) {
-        $console->writeOut("\n");
+  private function findExactNames(array $linters, array $names) {
+    $filtered = array();
+
+    foreach ($linters as $key => $linter) {
+      $name = $linter['name'];
+
+      foreach ($names as $term) {
+        if (strcasecmp($name, $term) == 0) {
+          $filtered[$key] = $linter;
         }
       }
-
-    if (!$this->getArgument('verbose')) {
-      $console->writeOut(
-        "%s\n",
-        pht('(Run `%s` for more details.)', 'arc linters --verbose'));
     }
-  }
-
-
-  /**
-   * Get human-readable linter statuses, padded to fixed width.
-   *
-   * @return map<string, string> Human-readable linter status names.
-   */
-  private function getStatusMap() {
-    $text_map = array(
-      'configured' => pht('CONFIGURED'),
-      'available'  => pht('AVAILABLE'),
-      'error'      => pht('ERROR'),
-    );
-
-    $sizes = array();
-    foreach ($text_map as $key => $string) {
-      $sizes[$key] = phutil_utf8_console_strlen($string);
-    }
-
-    $longest = max($sizes);
-    foreach ($text_map as $key => $string) {
-      if ($sizes[$key] < $longest) {
-        $text_map[$key] .= str_repeat(' ', $longest - $sizes[$key]);
-      }
-    }
-
-    $text_map['padding'] = str_repeat(' ', $longest);
-
-    return $text_map;
+    return $filtered;
   }
 
 }