Backport some changes from default to 1.3.

9 files changed, 299 insertions, 35 deletions

diff --git a/ChangeLog b/ChangeLog
index a3f7dd9..b6fdba2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,9 +7,27 @@ ChangeLog for Pylint
       Closes issue #285.
 
     * Fix a false positive with string formatting checker, when using
-      keyword argument packing. Closes issue #288.
+      keyword argument packing. Closes issue #288.    
 
-    * Proper handle class level scope for lambdas.
+    * Handle 'too-few-format-args' or 'too-many-format-args' for format
+      strings with both named and positional fields. Closes issue #286.
+
+    * Analyze only strings by the string format checker. Closes issue #287.
+
+    * Properly handle nested format string fields. Closes issue #294.
+    
+    * Properly handle unicode format strings for Python 2.
+      Closes issue #296.
+ 
+    * Fix a false positive with 'too-few-format-args', when the format
+      strings contains duplicate manual position arguments.
+      Closes issue #310.
+
+    * fixme regex handles comments without spaces after the hash.
+      Closes issue #311.
+
+    * Fix a crash encountered when looking for attribute docstrings.
+ 
 
 2014-07-26  --  1.3.0
 
diff --git a/checkers/base.py b/checkers/base.py
index c1600b5..469aeb8 100644
--- a/checkers/base.py
+++ b/checkers/base.py
@@ -555,7 +555,7 @@ functions, methods
                     pass
                 else:
                     sibling = expr.previous_sibling()
-                    if (sibling.scope() is scope and
+                    if (sibling is not None and sibling.scope() is scope and
                             isinstance(sibling, astroid.Assign)):
                         return
             self.add_message('pointless-string-statement', node=node)
diff --git a/checkers/misc.py b/checkers/misc.py
index b4738b0..b53f882 100644
--- a/checkers/misc.py
+++ b/checkers/misc.py
@@ -74,7 +74,7 @@ class EncodingChecker(BaseChecker):
         stream.seek(0)  # XXX may be removed with astroid > 0.23
         if self.config.notes:
             notes = re.compile(
-                r'.*?#\s+(%s)(:*\s*.+)' % "|".join(self.config.notes))
+                r'.*?#\s*(%s)(:*\s*.+)' % "|".join(self.config.notes))
         else:
             notes = None
         if module.file_encoding:
diff --git a/checkers/strings.py b/checkers/strings.py
index 6f6f3bb..0eda6a2 100644
--- a/checkers/strings.py
+++ b/checkers/strings.py
@@ -21,6 +21,10 @@
 import sys
 import tokenize
 import string
+try:
+    import numbers
+except ImportError:
+    numbers = None
 
 import astroid
 
@@ -122,7 +126,7 @@ if _PY3K:
 else:
     def _field_iterator_convertor(iterator):
         for is_attr, key in iterator:
-            if not isinstance(key, str):
+            if isinstance(key, numbers.Number):
                 yield is_attr, int(key)
             else:
                 yield is_attr, key
@@ -133,17 +137,13 @@ else:
         # the output
         return keyname, _field_iterator_convertor(fielditerator)
 
-def parse_format_method_string(format_string):
-    """
-    Parses a PEP 3101 format string, returning a tuple of
-    (keys, num_args, manual_pos_arg),
-    where keys is the set of mapping keys in the format string, num_args
-    is the number of arguments required by the format string and
-    manual_pos_arg is the number of arguments passed with the position.
+
+def collect_string_fields(format_string):
+    """ Given a format string, return an iterator
+    of all the valid format fields. It handles nested fields
+    as well.
     """
-    keys = []
-    num_args = 0
-    manual_pos_arg = 0
+
     formatter = string.Formatter()
     parseiterator = formatter.parse(format_string)
     try:
@@ -152,22 +152,40 @@ def parse_format_method_string(format_string):
                 # not a replacement format
                 continue
             name = result[1]
-            if name and str(name).isdigit():
-                manual_pos_arg += 1
-            elif name:
-                keyname, fielditerator = split_format_field_names(name)
-                if not isinstance(keyname, str):
-                    # In Python 2 it will return long which will lead
-                    # to different output between 2 and 3
-                    keyname = int(keyname)
-                keys.append((keyname, list(fielditerator)))
-            else:
-                num_args += 1
+            nested = result[2]
+            yield name
+            if nested:
+                for field in collect_string_fields(nested):
+                    yield field
     except ValueError:
         # probably the format string is invalid
         # should we check the argument of the ValueError?
         raise utils.IncompleteFormatString(format_string)
-    return keys, num_args, manual_pos_arg
+
+def parse_format_method_string(format_string):
+    """
+    Parses a PEP 3101 format string, returning a tuple of
+    (keys, num_args, manual_pos_arg),
+    where keys is the set of mapping keys in the format string, num_args
+    is the number of arguments required by the format string and
+    manual_pos_arg is the number of arguments passed with the position.
+    """
+    keys = []
+    num_args = 0
+    manual_pos_arg = set()
+    for name in collect_string_fields(format_string):
+        if name and str(name).isdigit():
+            manual_pos_arg.add(str(name))
+        elif name:
+            keyname, fielditerator = split_format_field_names(name)
+            if isinstance(keyname, numbers.Number):
+                # In Python 2 it will return long which will lead
+                # to different output between 2 and 3
+                keyname = int(keyname)
+            keys.append((keyname, list(fielditerator)))
+        else:
+            num_args += 1
+    return keys, num_args, len(manual_pos_arg)
 
 def get_args(callfunc):
     """ Get the arguments from the given `CallFunc` node.
@@ -223,7 +241,8 @@ class StringFormatChecker(BaseChecker):
                 utils.parse_format_string(format_string)
         except utils.UnsupportedFormatCharacter, e:
             c = format_string[e.index]
-            self.add_message('bad-format-character', node=node, args=(c, ord(c), e.index))
+            self.add_message('bad-format-character',
+                             node=node, args=(c, ord(c), e.index))
             return
         except utils.IncompleteFormatString:
             self.add_message('truncated-format-string', node=node)
@@ -246,7 +265,8 @@ class StringFormatChecker(BaseChecker):
                         if isinstance(key, basestring):
                             keys.add(key)
                         else:
-                            self.add_message('bad-format-string-key', node=node, args=key)
+                            self.add_message('bad-format-string-key',
+                                             node=node, args=key)
                     else:
                         # One of the keys was something other than a
                         # constant.  Since we can't tell what it is,
@@ -256,13 +276,16 @@ class StringFormatChecker(BaseChecker):
                 if not unknown_keys:
                     for key in required_keys:
                         if key not in keys:
-                            self.add_message('missing-format-string-key', node=node, args=key)
+                            self.add_message('missing-format-string-key',
+                                             node=node, args=key)
                 for key in keys:
                     if key not in required_keys:
-                        self.add_message('unused-format-string-key', node=node, args=key)
+                        self.add_message('unused-format-string-key',
+                                         node=node, args=key)
             elif isinstance(args, OTHER_NODES + (astroid.Tuple,)):
                 type_name = type(args).__name__
-                self.add_message('format-needs-mapping', node=node, args=type_name)
+                self.add_message('format-needs-mapping',
+                                 node=node, args=type_name)
             # else:
                 # The RHS of the format specifier is a name or
                 # expression.  It may be a mapping object, so
@@ -317,6 +340,13 @@ class StringMethodsChecker(BaseChecker):
 
     def _check_new_format(self, node, func):
         """ Check the new string formatting. """
+        # TODO: skip (for now) format nodes which don't have
+        #       an explicit string on the left side of the format operation.
+        #       We do this because our inference engine can't properly handle
+        #       redefinitions of the original string.
+        #       For more details, see issue 287.
+        if not isinstance(node.func.expr, astroid.Const):
+            return
         try:
             strnode = func.bound.infer().next()
         except astroid.InferenceError:
@@ -337,14 +367,18 @@ class StringMethodsChecker(BaseChecker):
             return
 
         manual_fields = set(field[0] for field in fields
-                            if isinstance(field[0], int))
+                            if isinstance(field[0], numbers.Number))
         named_fields = set(field[0] for field in fields
-                           if isinstance(field[0], str))
+                           if isinstance(field[0], basestring))
         if num_args and manual_pos:
             self.add_message('format-combined-specification',
                              node=node)
             return
 
+        check_args = False
+        # Consider "{[0]} {[1]}" as num_args.
+        num_args += sum(1 for field in named_fields
+                        if field == '')
         if named_fields:
             for field in named_fields:
                 if field not in named and field:
@@ -356,7 +390,21 @@ class StringMethodsChecker(BaseChecker):
                     self.add_message('unused-format-string-argument',
                                      node=node,
                                      args=(field, ))
+            # num_args can be 0 if manual_pos is not.
+            num_args = num_args or manual_pos
+            if positional or num_args:
+                empty = any(True for field in named_fields
+                            if field == '')
+                if named or empty:
+                    # Verify the required number of positional arguments
+                    # only if the .format got at least one keyword argument.
+                    # This means that the format strings accepts both
+                    # positional and named fields and we should warn
+                    # when one of the them is missing or is extra.
+                    check_args = True
         else:
+            check_args = True
+        if check_args:
             # num_args can be 0 if manual_pos is not.
             num_args = num_args or manual_pos
             if positional > num_args:
@@ -384,7 +432,7 @@ class StringMethodsChecker(BaseChecker):
                 # to 0. It will not be present in `named`, so use the value
                 # 0 for it.
                 key = 0
-            if isinstance(key, int):
+            if isinstance(key, numbers.Number):
                 try:
                     argname = utils.get_argument_from_call(node, key)
                 except utils.NoSuchArgumentError:
diff --git a/test/functional/string_formatting.py b/test/functional/string_formatting.py
new file mode 100644
index 0000000..594c870
--- /dev/null
+++ b/test/functional/string_formatting.py
@@ -0,0 +1,121 @@
+"""test for Python 3 string formatting error

+"""

+# pylint: disable=too-few-public-methods, import-error, unused-argument, star-args, line-too-long

+import os

+from missing import Missing

+

+__revision__ = 1

+

+class Custom(object):

+    """ Has a __getattr__ """

+    def __getattr__(self):

+        return self

+

+class Test(object):

+    """ test format attribute access """

+    custom = Custom()

+    ids = [1, 2, 3, [4, 5, 6]]

+

+class Getitem(object):

+    """ test custom getitem for lookup access """

+    def __getitem__(self, index):

+        return 42

+

+class ReturnYes(object):

+    """ can't be properly infered """

+    missing = Missing()

+

+def log(message, message_type="error"):

+    """ Test """

+    return message

+

+def print_good():

+    """ Good format strings """

+    "{0} {1}".format(1, 2)

+    "{0!r:20}".format("Hello")

+    "{!r:20}".format("Hello")

+    "{a!r:20}".format(a="Hello")

+    "{pid}".format(pid=os.getpid())

+    str("{}").format(2)

+    "{0.missing.length}".format(ReturnYes())

+    "{1.missing.length}".format(ReturnYes())

+    "{a.ids[3][1]}".format(a=Test())

+    "{a[0][0]}".format(a=[[1]])

+    "{[0][0]}".format({0: {0: 1}})

+    "{a.test}".format(a=Custom())

+    "{a.__len__}".format(a=[])

+    "{a.ids.__len__}".format(a=Test())

+    "{a[0]}".format(a=Getitem())

+    "{a[0][0]}".format(a=[Getitem()])

+    "{[0][0]}".format(["test"])

+    # these are skipped

+    "{0} {1}".format(*[1, 2])

+    "{a} {b}".format(**{'a': 1, 'b': 2})

+    "{a}".format(a=Missing())

+

+def pprint_bad():

+    """Test string format """

+    "{{}}".format(1) # [too-many-format-args]

+    "{} {".format() # [bad-format-string]

+    "{} }".format() # [bad-format-string]

+    "{0} {}".format(1, 2) # [format-combined-specification]

+    # +1: [missing-format-argument-key, unused-format-string-argument]

+    "{a} {b}".format(a=1, c=2)

+    "{} {a}".format(1, 2) # [missing-format-argument-key]

+    "{} {}".format(1) # [too-few-format-args]

+    "{} {}".format(1, 2, 3) # [too-many-format-args]

+    # +1: [missing-format-argument-key,missing-format-argument-key,missing-format-argument-key]

+    "{a} {b} {c}".format()

+    "{} {}".format(a=1, b=2) # [too-few-format-args]

+    # +1: [missing-format-argument-key, missing-format-argument-key]

+    "{a} {b}".format(1, 2)

+    "{0} {1} {a}".format(1, 2, 3) # [missing-format-argument-key]

+    # +1: [missing-format-attribute]

+    "{a.ids.__len__.length}".format(a=Test())

+    "{a.ids[3][400]}".format(a=Test()) # [invalid-format-index]

+    "{a.ids[3]['string']}".format(a=Test()) # [invalid-format-index]

+    "{[0][1]}".format(["a"]) # [invalid-format-index]

+    "{[0][0]}".format(((1, ))) # [invalid-format-index]

+    # +1: [missing-format-argument-key, unused-format-string-argument]

+    "{b[0]}".format(a=23)

+    "{a[0]}".format(a=object) # [invalid-format-index]

+    log("{}".format(2, "info")) # [too-many-format-args]

+    "{0.missing}".format(2) # [missing-format-attribute]

+    "{0} {1} {2}".format(1, 2) # [too-few-format-args]

+    "{0} {1}".format(1, 2, 3) # [too-many-format-args]

+    "{0} {a}".format(a=4) # [too-few-format-args]

+    "{[0]} {}".format([4]) # [too-few-format-args]

+    "{[0]} {}".format([4], 5, 6) # [too-many-format-args]

+

+def good_issue288(*args, **kwargs):

+    """ Test that using kwargs does not emit a false

+    positive.

+    """

+    'Hello John Doe {0[0]}'.format(args)

+    'Hello {0[name]}'.format(kwargs)

+

+def good_issue287():

+    """ Test that the string format checker skips

+    format nodes which don't have a string as a parent

+    (but a subscript, name etc).

+    """

+    name = 'qwerty'

+    ret = {'comment': ''}

+    ret['comment'] = 'MySQL grant {0} is set to be revoked'

+    ret['comment'] = ret['comment'].format(name)

+    return ret, name

+

+def nested_issue294():

+    """ Test nested format fields. """

+    '{0:>{1}}'.format(42, 24)

+    '{0:{a[1]}} {a}'.format(1, a=[1, 2])

+    '{:>{}}'.format(42, 24)

+    '{0:>{1}}'.format(42) # [too-few-format-args]

+    '{0:>{1}}'.format(42, 24, 54) # [too-many-format-args]

+    '{0:{a[1]}}'.format(1) # [missing-format-argument-key]

+    '{0:{a.x}}'.format(1, a=2) # [missing-format-attribute]

+

+def issue310():

+    """ Test a regression using duplicate manual position arguments. """

+    '{0} {1} {0}'.format(1, 2)

+    '{0} {1} {0}'.format(1) # [too-few-format-args]

diff --git a/test/functional/string_formatting.txt b/test/functional/string_formatting.txt
new file mode 100644
index 0000000..5f27835
--- /dev/null
+++ b/test/functional/string_formatting.txt
@@ -0,0 +1,36 @@
+too-many-format-args:58:pprint_bad:Too many arguments for format string
+bad-format-string:59:pprint_bad:Invalid format string
+bad-format-string:60:pprint_bad:Invalid format string
+format-combined-specification:61:pprint_bad:Format string contains both automatic field numbering and manual field specification
+missing-format-argument-key:63:pprint_bad:Missing keyword argument 'b' for format string
+unused-format-string-argument:63:pprint_bad:Unused format argument 'c'
+missing-format-argument-key:64:pprint_bad:Missing keyword argument 'a' for format string
+too-few-format-args:65:pprint_bad:Not enough arguments for format string
+too-many-format-args:66:pprint_bad:Too many arguments for format string
+missing-format-argument-key:68:pprint_bad:Missing keyword argument 'a' for format string
+missing-format-argument-key:68:pprint_bad:Missing keyword argument 'b' for format string
+missing-format-argument-key:68:pprint_bad:Missing keyword argument 'c' for format string
+too-few-format-args:69:pprint_bad:Not enough arguments for format string
+missing-format-argument-key:71:pprint_bad:Missing keyword argument 'a' for format string
+missing-format-argument-key:71:pprint_bad:Missing keyword argument 'b' for format string
+missing-format-argument-key:72:pprint_bad:Missing keyword argument 'a' for format string
+missing-format-attribute:74:pprint_bad:Missing format attribute 'length' in format specifier 'a.ids.__len__.length'
+invalid-format-index:75:pprint_bad:Using invalid lookup key 400 in format specifier 'a.ids[3][400]'
+invalid-format-index:76:pprint_bad:Using invalid lookup key "'string'" in format specifier 'a.ids[3]["\'string\'"]'
+invalid-format-index:77:pprint_bad:Using invalid lookup key 1 in format specifier '0[0][1]'
+invalid-format-index:78:pprint_bad:Using invalid lookup key 0 in format specifier '0[0][0]'
+missing-format-argument-key:80:pprint_bad:Missing keyword argument 'b' for format string
+unused-format-string-argument:80:pprint_bad:Unused format argument 'a'
+invalid-format-index:81:pprint_bad:Using invalid lookup key 0 in format specifier 'a[0]'
+too-many-format-args:82:pprint_bad:Too many arguments for format string
+missing-format-attribute:83:pprint_bad:Missing format attribute 'missing' in format specifier '0.missing'
+too-few-format-args:84:pprint_bad:Not enough arguments for format string
+too-many-format-args:85:pprint_bad:Too many arguments for format string
+too-few-format-args:86:pprint_bad:Not enough arguments for format string
+too-few-format-args:87:pprint_bad:Not enough arguments for format string
+too-many-format-args:88:pprint_bad:Too many arguments for format string
+too-few-format-args:113:nested_issue294:Not enough arguments for format string
+too-many-format-args:114:nested_issue294:Too many arguments for format string
+missing-format-argument-key:115:nested_issue294:Missing keyword argument 'a' for format string
+missing-format-attribute:116:nested_issue294:Missing format attribute 'x' in format specifier 'a.x'
+too-few-format-args:121:issue310:Not enough arguments for format string
diff --git a/test/functional/string_formatting_py27.py b/test/functional/string_formatting_py27.py
new file mode 100644
index 0000000..d6aeae7
--- /dev/null
+++ b/test/functional/string_formatting_py27.py
@@ -0,0 +1,23 @@
+"""test for Python 2 string formatting error

+"""

+from __future__ import unicode_literals

+# pylint: disable=line-too-long

+__revision__ = 1

+

+def pprint_bad():

+    """Test string format """

+    "{{}}".format(1) # [too-many-format-args]

+    "{} {".format() # [bad-format-string]

+    "{} }".format() # [bad-format-string]

+    "{0} {}".format(1, 2) # [format-combined-specification]

+    # +1: [missing-format-argument-key, unused-format-string-argument]

+    "{a} {b}".format(a=1, c=2)

+    "{} {a}".format(1, 2) # [missing-format-argument-key]

+    "{} {}".format(1) # [too-few-format-args]

+    "{} {}".format(1, 2, 3) # [too-many-format-args]

+    # +1: [missing-format-argument-key,missing-format-argument-key,missing-format-argument-key]

+    "{a} {b} {c}".format()

+    "{} {}".format(a=1, b=2) # [too-few-format-args]

+    # +1: [missing-format-argument-key, missing-format-argument-key]

+    "{a} {b}".format(1, 2)

+

diff --git a/test/functional/string_formatting_py27.rc b/test/functional/string_formatting_py27.rc
new file mode 100644
index 0000000..80170b7
--- /dev/null
+++ b/test/functional/string_formatting_py27.rc
@@ -0,0 +1,3 @@
+[testoptions]
+min_pyver=2.7

+max_pyver=3.0
\ No newline at end of file
diff --git a/test/functional/string_formatting_py27.txt b/test/functional/string_formatting_py27.txt
new file mode 100644
index 0000000..47f21de
--- /dev/null
+++ b/test/functional/string_formatting_py27.txt
@@ -0,0 +1,15 @@
+too-many-format-args:9:pprint_bad:Too many arguments for format string
+bad-format-string:10:pprint_bad:Invalid format string
+bad-format-string:11:pprint_bad:Invalid format string
+format-combined-specification:12:pprint_bad:Format string contains both automatic field numbering and manual field specification
+missing-format-argument-key:14:pprint_bad:Missing keyword argument u'b' for format string
+unused-format-string-argument:14:pprint_bad:Unused format argument 'c'
+missing-format-argument-key:15:pprint_bad:Missing keyword argument u'a' for format string
+too-few-format-args:16:pprint_bad:Not enough arguments for format string
+too-many-format-args:17:pprint_bad:Too many arguments for format string
+missing-format-argument-key:19:pprint_bad:Missing keyword argument u'a' for format string
+missing-format-argument-key:19:pprint_bad:Missing keyword argument u'b' for format string
+missing-format-argument-key:19:pprint_bad:Missing keyword argument u'c' for format string
+too-few-format-args:20:pprint_bad:Not enough arguments for format string
+missing-format-argument-key:22:pprint_bad:Missing keyword argument u'a' for format string
+missing-format-argument-key:22:pprint_bad:Missing keyword argument u'b' for format string