Issue 4044 - Added handling for __future__ unicode_literals import to check_quotes()

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
@@ skipping 266 matching lines @@
         substitute = DISCOURAGED_APIS.get(name)
         if substitute:
             self.errors.append((node, 'A301 use {}() instead of '
                                       '{}()'.format(substitute, name)))
 
     def visit_Call(self, 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)))
+        if isinstance(arg, ast.Lambda):
+            if len(node.args) == 2 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:
@@ skipping 64 matching lines @@
 
 
 def check_non_default_encoding(physical_line, line_number):
     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):
+def check_quotes(logical_line, tokens, previous_logical, checker_state):
     first_token = True
-    global is_unicode_literals
+
+    token_strings = [t[1] for t in tokens]
+    future_import = token_strings[:3] == ['from', '__future__', 'import']
+
+    if future_import and 'unicode_literals' in token_strings:
+        checker_state['has_unicode_literals'] = True
 
     for kind, token, start, end, _ in tokens:
         if kind == tokenize.INDENT or kind == tokenize.DEDENT:
             continue
 
-        if start[0] == 1:

[Vasily Kuznetsov, 2016/05/20 08:46:24]

This `if` as well as the following one don't need to be inside the loop.

-            is_unicode_literals = False
-
-        if logical_line == 'from __future__ import unicode_literals':

[Vasily Kuznetsov, 2016/05/20 08:46:25]

Actually this is still not completely right. It could be something like `from
__future__ import print_function, unicode_literals, division`, so to be more
robust you probably want to look at the tokens array and check if the first one
is 'from', the second one is '__future__', the third one is 'import' and there's
'unicode_literals' somewhere in the tail. I think this would be sufficiently
robust for most purposes.

-            is_unicode_literals = True
-
         if kind == tokenize.STRING:
-            match = re.search(r'^(u)?(b)?(r)?((""")?.*)$',
+            match = re.search(r'^([rub]*)([\'"]{1,3})(.*)\2$',
                               token, re.IGNORECASE | re.DOTALL)
-            (is_unicode, is_bytes, is_raw,
-             literal, has_doc_quotes) = match.groups()
+            prefixes, quote, text = match.groups()
+            prefixes = prefixes.lower()
+
+            if 'u' in prefixes:
+                yield (start, 'A112 use "from __future__ import '
+                              'unicode_literals" instead of '
+                              'prefixing literals with "u"')
 
             if first_token and re.search(r'^(?:(?:def|class)\s|$)',
                                          previous_logical):
-                if not has_doc_quotes:
+                if quote != '"""':
                     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)
-
+            elif start[0] != end[0]:
+                pass
+            elif 'r' in prefixes:
+                if quote != "'" and not (quote == '"' and "'" in text):
+                    yield (start, 'A110 use single quotes for raw string')
+            else:
+                prefix = ''
                 if sys.version_info[0] >= 3:
-                    if is_bytes:
-                        literal = 'b' + literal
-                elif is_unicode or is_unicode_literals:

[Sebastian Noack, 2016/05/20 13:48:01]

Perhaps we should generate a warning when using strings like u'' while using the
future import at the same time. Its redundant, and certainly bad practice.

[Vasily Kuznetsov, 2016/05/23 08:51:14]

Yes, this sounds like a good idea. Unfortunately in Python 2 `repr` returns
literals prefixed with `u` for unicode strings even if `unicode_literals` mode
is on. This means we will deviate from our recommendation to follow `repr` (in a
good and useful way, but still) and we probably need to reflect this in the
style guide. In any case.

It still makes sense to already implement it in `flake8-abp`. Perhaps same error
code: A110 with another message.

[Sebastian Noack, 2016/05/24 12:41:35]

Since compatibility with Python 3 is now mandatory in our coding style guide,
and since we also agreed that the future import is preferable over using strings
like u'' in Python 3, that effectively means that we shouldn't ever use strings
like u'' anymore, doesn't it? So I guess this should be the rule to be added to
the style guide an to be implemented here.

[Vasily Kuznetsov, 2016/05/24 13:02:42]

Yep. All exactly as you say. And it will actually simplify the logic here since
now we can just always complain about u'' literals. It seems that we still need
to detect `from __future__ import unicode_literals` though to make sure '\uXXXX'
escapes are handled correctly.

-                    literal = 'u' + literal
-
+                    if 'b' in prefixes:
+                        prefix = 'b'
+                else:
+                    u_literals = checker_state.get('has_unicode_literals')
+                    if 'u' in prefixes or u_literals and 'b' not in prefixes:
+                        prefix = 'u'
+
+                literal = '{0}{1}{2}{1}'.format(prefix, quote, text)
                 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__
 
 
@@ skipping 42 matching lines @@
                     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__