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 359 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):
     first_token = True
-    is_unicode_literals = False
+    global is_unicode_literals
 
     for kind, token, start, end, _ in tokens:
         if kind == tokenize.INDENT or kind == tokenize.DEDENT:
             continue
 
-        if token is 'unicode_literals':
+        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)?((""")?.*)$',
                               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 (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
-                elif is_unicode:
-                    literal = 'u' + literal
-                elif not is_unicode_literals:
+                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 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 44 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__