Noissue - Added flake8 extension accounting for our coding style and some other stuff

flake8-abp/flake8_abp.py

 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import ast
 import re
 import tokenize
 import sys
-
-__version__ = '0.1'

[Sebastian Noack, 2016/04/21 19:02:12]

It would be kinda cool if we could inject the version based on the mercurial
revision as a build step, given that we probably won't have releases for this
peace of code, and that it can only be used with flake8 after installing it via
setup.py, anyway.

However, I already spend more time than intended on this. But if you know from
the top of your head how to do that, or if you'd volunteer to look into that
later, that would be cool. ;)

-
-DEPRECATED_APIS = {
-    ('re', 'match'): 'A101 use re.search() instead re.match()',
-    ('codecs', 'open'): 'A102 use io.open() instead codecs.open()',
+import collections
+
+try:
+    import builtins
+except ImportError:
+    import __builtin__ as builtins
+
+import pkg_resources
+
+try:
+    ascii
+except NameError:
+    ascii = repr
+
+__version__ = pkg_resources.get_distribution('flake8-abp').version
+
+DISCOURAGED_APIS = {
+    're.match': 're.search',
+    'codecs.open': 'io.open',
 }
 
-BAILOUT = (ast.Return, ast.Raise, ast.Continue, ast.Break)
+ESSENTIAL_BUILTINS = set(dir(builtins)) - {'apply', 'buffer', 'coerce',
+                                           'intern', 'file'}
+
+LEAVE_BLOCK = (ast.Return, ast.Raise, ast.Continue, ast.Break)
+VOLATILE = object()
+
+
+def evaluate(node):
+    try:
+        return eval(compile(ast.Expression(node), '', 'eval'), {})
+    except Exception:
+        return VOLATILE
+
+
+def is_const(node):
+    return evaluate(node) is not VOLATILE
+
+
+def get_identifier(node):
+    if isinstance(node, ast.Name):
+        return node.id
+    if isinstance(node, ast.Attribute) and isinstance(node.value, ast.Name):
+        return '{}.{}'.format(node.value.id, node.attr)
+
+
+def get_statement(node):
+    return type(node).__name__.lower()
 
 
 class TreeVisitor(ast.NodeVisitor):
+    Scope = collections.namedtuple('Scope', ['node', 'names', 'globals'])
+
     def __init__(self):
         self.errors = []
-        self.stack = []
-
-    def _visit_block(self, nodes, mandatory=False, docstring=False):
+        self.scope_stack = []
+
+    def _visit_block(self, nodes, block_required=False,
+                     nodes_required=True, docstring=False,
+                     can_have_unused_expr=False):
         pass_node = None
-        bailed = False
+        has_non_pass = False
+        leave_node = None
         dead_code = False
 
-        for node in nodes:
+        for i, node in enumerate(nodes):
             if isinstance(node, ast.Pass):
                 pass_node = node
-
-            if bailed and not dead_code:
+            else:
+                has_non_pass = True
+
+            if leave_node and not dead_code:
                 dead_code = True
-                self.errors.append((node, 'A151 dead code after '
-                                          'return/raise/continue/break'))
-
-            if isinstance(node, BAILOUT):
-                bailed = True
-
-            if not isinstance(node, ast.Expr):
+                statement = get_statement(leave_node)
+                self.errors.append((node, 'A202 dead code after '
+                                          '{}'.format(statement)))
+
+            if isinstance(node, LEAVE_BLOCK):
+                leave_node = node
+
+            if can_have_unused_expr or not isinstance(node, ast.Expr):
+                continue
+            if docstring and i == 0 and isinstance(node.value, ast.Str):
                 continue
             if isinstance(node.value, (ast.Call, ast.Yield)):
                 continue
-            if docstring and node is nodes[0] and isinstance(node.value, ast.Str):
-                continue
-
-            self.errors.append((node, 'A152 unused expression'))
+
+            self.errors.append((node, 'A203 unused expression'))
 
         if pass_node:
-            if len(nodes) > 1:
-                self.errors.append((pass_node, 'A153 redundant pass statement'))
-
-            if not mandatory and all(isinstance(n, ast.Pass) for n in nodes):
-                self.errors.append((pass_node, 'A154 empty block'))
-
-    def _visit_block_node(self, node, **kwargs):
-        self._visit_block(node.body, **kwargs)
-        if hasattr(node, 'orelse'):
-            self._visit_block(node.orelse)
-        if hasattr(node, 'finalbody'):
-            self._visit_block(node.finalbody)
-        self.generic_visit(node)
-
-    visit_Try = visit_TryExcept = visit_TryFinally = _visit_block_node
-
-    visit_ExceptHandler = visit_While = \
-        lambda self, node: self._visit_block_node(node, mandatory=True)
-
-    visit_Module = visit_ClassDef = \
-        lambda self, node: self._visit_block_node(node, mandatory=True,
-                                                        docstring=True)
-
-    def visit_Attribute(self, node):
-        if isinstance(node.ctx, ast.Load) and isinstance(node.value, ast.Name):
-            error = DEPRECATED_APIS.get((node.value.id, node.attr))
-            if error:
-                self.errors.append((node, error))
-        self.generic_visit(node)
-
-    def visit_ImportFrom(self, node):
-        for alias in node.names:
-            error = DEPRECATED_APIS.get((node.module, alias.name))
-            if error:
-                self.errors.append((node, error))
+            if not nodes_required or len(nodes) > 1:
+                self.errors.append((pass_node, 'A204 redundant '
+                                               'pass statement'))
+
+            if not block_required and not has_non_pass:
+                self.errors.append((pass_node, 'A205 empty block'))
+
+    def _check_redundant_else(self, node, handlers, clause):
+        if not node.orelse:
+            return
+
+        for handler in handlers:
+            for child in handler.body:
+                if isinstance(child, LEAVE_BLOCK):
+                    leave_node = child
+                    break
+            else:
+                return
+
+        statement = get_statement(leave_node)
+        self.errors.append((node.orelse[0],
+                            'A206 Extraneous else statement after {} '
+                            'in {}-clause'.format(statement, clause)))
+
+    def visit_If(self, node):
+        self._visit_block(node.body, block_required=bool(node.orelse))
+        self._visit_block(node.orelse)
+        self._check_redundant_else(node, [node], 'if')
+        self.generic_visit(node)
+
+    def visit_Try(self, node):
+        self._visit_block(node.body, can_have_unused_expr=bool(node.handlers))
+        self._visit_block(node.orelse)
+        self._visit_block(node.finalbody)
+        self._check_redundant_else(node, node.handlers, 'except')
+        self.generic_visit(node)
+
+    def visit_TryExcept(self, node):
+        self._visit_block(node.body, can_have_unused_expr=True)
+        self._visit_block(node.orelse)
+        self._check_redundant_else(node, node.handlers, 'except')
+        self.generic_visit(node)
+
+    def visit_TryFinally(self, node):
+        self._visit_block(node.body)
+        self._visit_block(node.finalbody)
+        self.generic_visit(node)
+
+    def visit_ExceptHandler(self, node):
+        self._visit_block(node.body, block_required=True)
+        self.generic_visit(node)
+
+    def _visit_stored_name(self, node, name):
+        scope = self.scope_stack[-1]
+        scope.names.add(name)
+
+        if name in ESSENTIAL_BUILTINS and isinstance(scope.node,
+                                                     ast.FunctionDef):
+            self.errors.append((node, 'A302 redefined built-in ' + name))
+
+    def visit_Name(self, node):
+        if isinstance(node.ctx, (ast.Store, ast.Param)):
+            self._visit_stored_name(node, node.id)
+
+    def visit_arg(self, node):
+        self._visit_stored_name(node, node.arg)
+
+    def _visit_with_scope(self, node):
+        scope = self.Scope(node, names=set(), globals=[])
+        self.scope_stack.append(scope)
+        self.generic_visit(node)
+        del self.scope_stack[-1]
+        return scope
+
+    def visit_Module(self, node):
+        self._visit_block(node.body, block_required=True,
+                          nodes_required=False, docstring=True)
+        self._visit_with_scope(node)
+
+    def visit_FunctionDef(self, node):
+        self._visit_stored_name(node, node.name)
+        self._visit_block(node.body, block_required=True, docstring=True)
+
+        scope = self._visit_with_scope(node)
+        global_names = set()
+
+        for declaration in scope.globals:
+            for name in declaration.names:
+                if name not in scope.names or name in global_names:
+                    statement = get_statement(declaration)
+                    self.errors.append((declaration,
+                                        'A201 redundant {} declaration for '
+                                        '{}'.format(statement, name)))
+                else:
+                    global_names.add(name)
+
+    visit_ClassDef = visit_FunctionDef
+
+    def visit_Global(self, node):
+        scope = self.scope_stack[-1]
+        scope.globals.append(node)
+
+        if isinstance(scope.node, ast.Module):
+            statement = get_statement(node)
+            self.errors.append((node, 'A201 {} declaration on '
+                                      'top-level'.format(statement)))
+
+    visit_Nonlocal = visit_Global
+
+    def _visit_iter(self, node):
+        if isinstance(node, (ast.Tuple, ast.Set)):
+            self.errors.append((node, 'A101 use lists for data '
+                                      'that have order'))
+
+    def visit_comprehension(self, node):
+        self._visit_iter(node.iter)
+        self.generic_visit(node)
+
+    def visit_For(self, node):
+        self._visit_iter(node.iter)
+        self._visit_block(node.body, block_required=True)
+        self._visit_block(node.orelse)
+        self.generic_visit(node)
+
+    def visit_While(self, node):
+        self._visit_block(node.body, block_required=True)
+        self._visit_block(node.orelse)
+        self.generic_visit(node)
 
     def visit_BinOp(self, node):
         if isinstance(node.op, ast.Mod) and isinstance(node.left, ast.Str):
-            self.errors.append((node, 'A111 use format() instead % operator '
-                                      'for string formatting'))
+            self.errors.append((node, 'A107 use format() instead of '
+                                      '% operator for string formatting'))
 
         multi_addition = (isinstance(node.op, ast.Add) and
                           isinstance(node.left, ast.BinOp) and
                           isinstance(node.left.op, ast.Add))
         if multi_addition and (isinstance(node.left.left, ast.Str) or
                                isinstance(node.left.right, ast.Str) or
                                isinstance(node.right, ast.Str)):
-            self.errors.append((node, 'A112 use format() instead + operator '
-                                      'when concatenating >2 strings'))
-
-        self.generic_visit(node)
-
-    def visit_comprehension(self, node):
-        if isinstance(node.iter, (ast.Tuple, ast.Set, ast.Dict)):
-            self.errors.append((node.iter, 'A121 use lists for data '
-                                           'that have order'))
-        self.generic_visit(node)
-
-    def visit_For(self, node):
-        self._visit_block(node.body, mandatory=True)
-        self.visit_comprehension(node)
+            self.errors.append((node, 'A108 use format() instead of '
+                                      '+ operator when concatenating '
+                                      'more than two strings'))
+
+        self.generic_visit(node)
+
+    def visit_Compare(self, node):
+        left = node.left
+        single = len(node.ops) == 1
+
+        for op, right in zip(node.ops, node.comparators):
+            membership = isinstance(op, (ast.In, ast.NotIn))
+            symmetric = isinstance(op, (ast.Eq, ast.NotEq, ast.Is, ast.IsNot))
+
+            if membership and isinstance(right, (ast.Tuple, ast.List)):
+                self.errors.append((right, 'A102 use sets for distinct '
+                                           'unordered data'))
+
+            consts_first = single and not membership or symmetric
+            if consts_first and is_const(left) and not is_const(right):
+                self.errors.append((left, 'A103 yoda condition'))
+
+            left = right
+
+        self.generic_visit(node)
+
+    def _check_deprecated(self, node, name):
+        substitute = DISCOURAGED_APIS.get(name)
+        if substitute:
+            self.errors.append((node, 'A301 use {}() instead of '
+                                      '{}()'.format(substitute, name)))
 
     def visit_Call(self, node):
-        func = node.func
-        if isinstance(func, ast.Name) and func.id in {'filter', 'map'}:
-            if len(node.args) > 0 and isinstance(node.args[0], ast.Lambda):
-                self.errors.append((node, 'A131 use a comprehension '
-                                          'instead calling {}() with '
-                                          'lambda function'.format(func.id)))
-        self.generic_visit(node)
-
-    def visit_FunctionDef(self, node):
-        self._visit_block(node.body, mandatory=True, docstring=True)
-
-        self.stack.append((set(), []))
-        self.generic_visit(node)
-        targets, globals = self.stack.pop()
-
-        for var in globals:
-            if any(name not in targets for name in var.names):
-                self.errors.append((var, 'A141 redundant global/nonlocal '
-                                         'declaration'))
-
-    def visit_Name(self, node):
-        if self.stack and isinstance(node.ctx, ast.Store):
-            self.stack[-1][0].add(node.id)
-
-    def visit_Global(self, node):
-        if self.stack:
-            self.stack[-1][1].append(node)
-        else:
-            self.errors.append((node, 'A141 global/nonlocal declaration '
-                                      'on top-level'))
-
-    visit_Nonlocal = visit_Global
-
-    def visit_If(self, node):
-        has_else = bool(node.orelse)
-
-        if has_else and any(isinstance(m, BAILOUT) for n in node.body):
-            self.errors.append((node, 'A159 redundant else statement after '
-                                      'return/raise/continue/break '
-                                      'in if-clause'))
-
-        self._visit_block(node.body, mandatory=has_else)
-        self._visit_block(node.orelse)
-        self.generic_visit(node)
+        func = get_identifier(node.func)
+        arg = next(iter(node.args), None)
+        redundant_literal = False
+
+        if isinstance(arg, ast.Lambda) and func in {'map', 'filter',
+                                                    'imap', 'ifilter',
+                                                    'itertools.imap',
+                                                    'itertools.ifilter'}:
+            self.errors.append((node, 'A104 use a comprehension '
+                                      'instead of calling {}() with '
+                                      'lambda function'.format(func)))
+        elif isinstance(arg, (ast.List, ast.Tuple)):
+            if func == 'dict':
+                redundant_literal = all(isinstance(elt, (ast.Tuple, ast.List))
+                                        for elt in arg.elts)
+            else:
+                redundant_literal = func in {'list', 'set', 'tuple'}
+        elif isinstance(arg, (ast.ListComp, ast.GeneratorExp)):
+            if func == 'dict':
+                redundant_literal = isinstance(arg.elt, (ast.Tuple, ast.List))
+            else:
+                redundant_literal = func in {'list', 'set'}
+
+        if redundant_literal:
+            self.errors.append((node, 'A105 use a {0} literal or '
+                                      'comprehension instead of calling '
+                                      '{0}()'.format(func)))
+
+        self._check_deprecated(node, func)
+        self.generic_visit(node)
+
+    def visit_Import(self, node):
+        for alias in node.names:
+            self._visit_stored_name(node, alias.asname or alias.name)
+
+            if hasattr(node, 'module'):
+                self._check_deprecated(node, '{}.{}'.format(node.module,
+                                                            alias.name))
+
+    visit_ImportFrom = visit_Import
+
+    def visit_Assign(self, node):
+        if isinstance(node.value, ast.BinOp) and len(node.targets) == 1:
+            target = node.targets[0]
+            left_is_target = (isinstance(target, ast.Name) and
+                              isinstance(node.value.left, ast.Name) and
+                              target.id == node.value.left.id)
+            if left_is_target:
+                self.errors.append((node, 'A106 use augment assignment, '
+                                          'e.g. x += y instead x = x + y'))
+        self.generic_visit(node)
+
+    def _visit_hash_keys(self, nodes, what):
+        keys = []
+        for node in nodes:
+            key = evaluate(node)
+            if key is VOLATILE:
+                continue
+
+            if key in keys:
+                self.errors.append((node, 'A207 duplicate ' + what))
+                continue
+
+            keys.append(key)
+
+    def visit_Dict(self, node):
+        self._visit_hash_keys(node.keys, 'key in dict')
+
+    def visit_Set(self, node):
+        self._visit_hash_keys(node.elts, 'item in set')
 
 
 class ASTChecker(object):
     name = 'abp'
     version = __version__
 
     def __init__(self, tree, filename):
         self.tree = tree
 
     def run(self):
         visitor = TreeVisitor()
         visitor.visit(self.tree)
 
         for node, error in visitor.errors:
             yield (node.lineno, node.col_offset, error, type(self))
 
 
 def check_non_default_encoding(physical_line, line_number):
-    if (line_number <= 2 and re.search(r'^\s*#.*coding[:=]', physical_line)):
-        return (0, 'A201 non-default file encoding')
+    if line_number <= 2 and re.search(r'^\s*#.*coding[:=]', physical_line):
+        return (0, 'A303 non-default file encoding')
 
 check_non_default_encoding.name = 'abp-non-default-encoding'
 check_non_default_encoding.version = __version__
 
 
 def check_quotes(logical_line, tokens, previous_logical):
     first_token = True
-    offset = 0
 
     for kind, token, start, end, _ in tokens:
-        if kind == tokenize.INDENT:
-            offset = end[1]
+        if kind == tokenize.INDENT or kind == tokenize.DEDENT:
             continue
 
         if kind == tokenize.STRING:
-            pos = start[1] - offset
             match = re.search(r'^(u)?(b)?(r)?((""")?.*)$',
                               token, re.IGNORECASE | re.DOTALL)
             (is_unicode, is_bytes, is_raw,
              literal, has_doc_quotes) = match.groups()
 
             if first_token and re.search(r'^(?:(?:def|class)\s|$)',
                                          previous_logical):
                 if not has_doc_quotes:
-                    yield (pos, 'A301 use triple double quotes for docstrings')
-                if is_unicode or is_bytes or is_raw:
-                    yield (pos, "A302 don't use u, b or for doc strings")
+                    yield (start, 'A109 use triple double '
+                                  'quotes for docstrings')
+                elif is_unicode or is_bytes or is_raw:
+                    yield (start, "A109 don't use u'', b'' "
+                                  "or r'' for doc strings")
             elif start[0] == end[0]:
                 if is_raw:
                     literal = re.sub(r'\\(?!{})'.format(literal[0]),
                                      '\\\\\\\\', literal)
 
                 if sys.version_info[0] >= 3:
                     if is_bytes:
                         literal = 'b' + literal
-                    else:
-                        literal = re.sub(r'(?<!\\)\\x(?!a[0d])([a-f][0-9a-f])',
-                                         lambda m: chr(int(m.group(1), 16)),
-                                         literal)
                 elif is_unicode:
                     literal = 'u' + literal
 
-                if repr(eval(literal)) != literal:
-                    yield (pos, "A311 string literal doesn't match repr()")
+                if ascii(eval(literal)) != literal:
+                    yield (start, "A110 string literal doesn't match"
+                                  '{}()'.format(ascii.__name__))
 
         first_token = False
 
 check_quotes.name = 'abp-quotes'
 check_quotes.version = __version__
+
+
+def check_redundant_parenthesis(logical_line, tokens):
+    start_line = tokens[0][2][0]
+    level = 0
+    statement = None
+
+    for i, (kind, token, _, end, _) in enumerate(tokens):
+        if kind == tokenize.INDENT or kind == tokenize.DEDENT:
+            continue
+
+        if statement is None:
+            # logical line doesn't start with an if, elif or while statement
+            if kind != tokenize.NAME or token not in {'if', 'elif', 'while'}:
+                break
+
+            # expression doesn't start with parenthesis
+            next_token = tokens[i + 1]
+            if next_token[:2] != (tokenize.OP, '('):
+                break
+
+            # expression is empty tuple
+            if tokens[i + 2][:2] == (tokenize.OP, ')'):
+                break
+
+            statement = token
+            pos = next_token[2]
+            continue
+
+        # expression ends on a different line, parenthesis are necessary
+        if end[0] > start_line:
+            break
+
+        if kind == tokenize.OP:
+            if token == ',':
+                # expression is non-empty tuple
+                if level == 1:
+                    break
+            elif token == '(':
+                level += 1
+            elif token == ')':
+                level -= 1
+                if level == 0:
+                    # outer parenthesis closed before end of expression
+                    if tokens[i + 1][:2] != (tokenize.OP, ':'):
+                        break
+
+                    return [(pos, 'A111 redundant parenthesis for {} '
+                                  'statement'.format(statement))]
+
+    return []
+
+check_redundant_parenthesis.name = 'abp-redundant-parenthesis'
+check_redundant_parenthesis.version = __version__