Extend trailing-comma-tuple check to more complex assignments (#1721)

* Extend trailing-comma-tuple check to more complex assignments

The previous implementation was too conservative with looking for
previous tokens associated with assignment: It looked only
at the immediately previous token, causing 'a = (5),' to not be
caught.

Now The current implementation backtracks to the start of the line
to find an assignment substring.

Fixes issue #1713

Author: brycepg

CONTRIBUTORS.txt

@@ -141,3 +141,5 @@ Order doesn't matter (not that much, at least ;)
 * Ahirnish Pareek, 'keyword-arg-before-var-arg' check
 
 * Guillaume Peillex: contributor.
+
+* Bryce Guinta: contributor

ChangeLog

@@ -4,6 +4,11 @@ Pylint's ChangeLog
 What's New in Pylint 1.8?
 =========================
 
+    * `trailing-comma-tuple` refactor check now extends to assignment with
+       more than one element (such as lists)
+
+      Close #1713
+
     * Fixing u'' string in superfluous-parens message 
 
       Close #1420

pylint/checkers/refactoring.py

@@ -271,28 +271,10 @@ def process_tokens(self, tokens):
                 # tokens[index][2] is the actual position and also is
                 # reported by IronPython.
                 self._elifs.extend([tokens[index][2], tokens[index+1][2]])
-            elif six.PY3 and token.exact_type == tokenize.COMMA:
-                self._check_one_element_trailing_comma_tuple(tokens, token, index)
-
-    def _check_one_element_trailing_comma_tuple(self, tokens, token, index):
-        left_tokens = itertools.islice(tokens, index + 1, None)
-        same_line_remaining_tokens = list(itertools.takewhile(
-            lambda other_token, _token=token: other_token.start[0] == _token.start[0],
-            left_tokens
-        ))
-        is_last_element = all(
-            other_token.type in (tokenize.NEWLINE, tokenize.COMMENT)
-            for other_token in same_line_remaining_tokens
-        )
-
-        if not same_line_remaining_tokens or not is_last_element:
-            return
-
-        assign_token = tokens[index-2:index-1]
-        if assign_token and '=' in assign_token[0].string:
-            if self.linter.is_message_enabled('trailing-comma-tuple'):
-                self.add_message('trailing-comma-tuple',
-                                 line=token.start[0])
+            elif six.PY3 and is_trailing_comma(tokens, index):
+                if self.linter.is_message_enabled('trailing-comma-tuple'):
+                    self.add_message('trailing-comma-tuple',
+                                     line=token.start[0])
 
     def leave_module(self, _):
         self._init()
@@ -834,6 +816,46 @@ def visit_compare(self, node):
                     self.add_message('len-as-condition', node=node)
 
 
+def is_trailing_comma(tokens, index):
+    """Check if the given token is a trailing comma
+
+    :param tokens: Sequence of modules tokens
+    :type tokens: list[tokenize.TokenInfo]
+    :param int index: Index of token under check in tokens
+    :returns: True if the token is a comma which trails an expression
+    :rtype: bool
+    """
+    token = tokens[index]
+    if token.exact_type != tokenize.COMMA:
+        return False
+    # Must have remaining tokens on the same line such as NEWLINE
+    left_tokens = itertools.islice(tokens, index + 1, None)
+    same_line_remaining_tokens = list(itertools.takewhile(
+        lambda other_token, _token=token: other_token.start[0] == _token.start[0],
+        left_tokens
+    ))
+    # Note: If the newline is tokenize.NEWLINE and not tokenize.NL
+    # then the newline denotes the end of expression
+    is_last_element = all(
+        other_token.type in (tokenize.NEWLINE, tokenize.COMMENT)
+        for other_token in same_line_remaining_tokens
+    )
+    if not same_line_remaining_tokens or not is_last_element:
+        return False
+    def get_curline_index_start():
+        """Get the index denoting the start of the current line"""
+        for subindex, token in enumerate(reversed(tokens[:index])):
+            # See Lib/tokenize.py and Lib/token.py in cpython for more info
+            if token.type in (tokenize.NEWLINE, tokenize.NL):
+                return index - subindex
+        return 0
+    curline_start = get_curline_index_start()
+    for prevtoken in tokens[curline_start:index]:
+        if '=' in prevtoken.string:
+            return True
+    return False
+
+
 def register(linter):
     """Required method to auto register this checker."""
     linter.register_checker(RefactoringChecker(linter))

pylint/test/functional/trailing_comma_tuple.py

@@ -3,6 +3,7 @@
 AAA = 1, # [trailing-comma-tuple]
 BBB = "aaaa", # [trailing-comma-tuple]
 CCC="aaa", # [trailing-comma-tuple]
+FFF=['f'], # [trailing-comma-tuple]
 
 BBB = 1, 2
 CCC = (1, 2, 3)

pylint/test/functional/trailing_comma_tuple.txt

@@ -1,3 +1,4 @@
 trailing-comma-tuple:3::Disallow trailing comma tuple
 trailing-comma-tuple:4::Disallow trailing comma tuple
 trailing-comma-tuple:5::Disallow trailing comma tuple
+trailing-comma-tuple:6::Disallow trailing comma tuple