From d1db9a72b552151613a918e3d49fa72433387a68 Mon Sep 17 00:00:00 2001
From: Jon Parise <jon@pinterest.com>
Date: Fri, 24 Mar 2017 08:26:03 -0700
Subject: [PATCH] Improve the flake8 line-matching regex

Summary:
Because the character offset group is optional, the logic for dealing
with the previous index-based match object was more complex than it
needs to be. Switching to named groups makes this clearer.

As an example of how this was causing a problem, when the character
group *was* present (at index 3), it was being appending to the linter's
name in the call to ->setName(), resulting in confusing linter output.
We may have also been setting the character offset to the error code's
string, too, which is further nonsense.

Because we reliably capture the flake8 error code, I don't think there's
a need to append it to the linter's name at all now, so I removed that
part (so it's always just `flake8`), but it's not a problem to restore.

Lastly, I hoisted `$regex` out of the loop because it's a constant and
de-indenting it gave me enough room to continuing writing it all on one
line.

Test Plan: Tested primarily with flake8 3.3.0

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17552
---
 src/lint/linter/ArcanistFlake8Linter.php | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php
index 19d57432..37fb45e1 100644
--- a/src/lint/linter/ArcanistFlake8Linter.php
+++ b/src/lint/linter/ArcanistFlake8Linter.php
@@ -51,12 +51,14 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter {
   protected function parseLinterOutput($path, $err, $stdout, $stderr) {
     $lines = phutil_split_lines($stdout, false);
 
+    // stdin:2: W802 undefined name 'foo'  # pyflakes
+    // stdin:3:1: E302 expected 2 blank lines, found 1  # pep8
+    $regexp =
+      '/^(?:.*?):(?P<line>\d+):(?:(?P<char>\d+):)? (?P<code>\S+) (?P<msg>.*)$/';
+
     $messages = array();
     foreach ($lines as $line) {
       $matches = null;
-      // stdin:2: W802 undefined name 'foo'  # pyflakes
-      // stdin:3:1: E302 expected 2 blank lines, found 1  # pep8
-      $regexp = '/^(.*?):(\d+):(?:(\d+):)? (\S+) (.*)$/';
       if (!preg_match($regexp, $line, $matches)) {
         continue;
       }
@@ -66,14 +68,14 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter {
 
       $message = new ArcanistLintMessage();
       $message->setPath($path);
-      $message->setLine($matches[2]);
-      if (!empty($matches[3])) {
-        $message->setChar($matches[3]);
+      $message->setLine($matches['line']);
+      if (!empty($matches['char'])) {
+        $message->setChar($matches['char']);
       }
-      $message->setCode($matches[4]);
-      $message->setName($this->getLinterName().' '.$matches[3]);
-      $message->setDescription($matches[5]);
-      $message->setSeverity($this->getLintMessageSeverity($matches[4]));
+      $message->setCode($matches['code']);
+      $message->setName($this->getLinterName().' '.$matches['code']);
+      $message->setDescription($matches['msg']);
+      $message->setSeverity($this->getLintMessageSeverity($matches['code']));
 
       $messages[] = $message;
     }