Handle 'too-few-format-args' or 'too-many-format-args' for format strings with both named and positional fields. Closes issue #286.

6 files changed, 145 insertions, 116 deletions

diff --git a/ChangeLog b/ChangeLog
index 0fca0ef..b0d0956 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,9 @@ ChangeLog for Pylint
 
     * 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.
+
 2014-07-26  --  1.3.0
 
     * Allow hanging continued indentation for implicitly concatenated
diff --git a/checkers/strings.py b/checkers/strings.py
index 6f6f3bb..11b2baa 100644
--- a/checkers/strings.py
+++ b/checkers/strings.py
@@ -345,6 +345,10 @@ class StringMethodsChecker(BaseChecker):
                              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 +360,19 @@ 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:
+                if named or any(True for field in named_fields if field == ''):
+                    # 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:
diff --git a/test/functional/string_formatting.py b/test/functional/string_formatting.py
new file mode 100644
index 0000000..69a9cb1
--- /dev/null
+++ b/test/functional/string_formatting.py
@@ -0,0 +1,95 @@
+"""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)

diff --git a/test/functional/string_formatting.txt b/test/functional/string_formatting.txt
new file mode 100644
index 0000000..6ec8944
--- /dev/null
+++ b/test/functional/string_formatting.txt
@@ -0,0 +1,31 @@
+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
diff --git a/test/input/func_string_format_py27.py b/test/input/func_string_format_py27.py
deleted file mode 100644
index e2bfe12..0000000
--- a/test/input/func_string_format_py27.py
+++ /dev/null
@@ -1,88 +0,0 @@
-"""test for Python 3 string formatting error

-"""

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

-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 """

-    print "{0} {1}".format(1, 2)

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

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

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

-    print "{pid}".format(pid=os.getpid())

-    print str("{}").format(2)

-    print "{0.missing.length}".format(ReturnYes())

-    print "{1.missing.length}".format(ReturnYes())

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

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

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

-    print "{a.test}".format(a=Custom())

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

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

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

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

-    print "{[0][0]}".format(["test"])

-    # these are skipped

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

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

-    print "{a}".format(a=Missing())

-

-def pprint_bad():

-    """Test string format """

-    print "{{}}".format(1)

-    print "{} {".format()

-    print "{} }".format()

-    print "{0} {}".format(1, 2)

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

-    print "{} {a}".format(1, 2)

-    print "{} {}".format(1)

-    print "{} {}".format(1, 2, 3)

-    print "{a} {b} {c}".format()

-    print "{} {}".format(a=1, b=2)

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

-    print "{0} {1} {a}".format(1, 2, 3)

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

-    print "{a.ids[3][400]}".format(a=Test())

-    print "{a.ids[3]['string']}".format(a=Test())

-    print "{[0][1]}".format(["a"])

-    print "{[0][0]}".format(((1, )))

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

-    print "{a[0]}".format(a=object)

-    print log("{}".format(2, "info"))

-    print "{0.missing}".format(2)

-    print "{0} {1} {2}".format(1, 2)

-    print "{0} {1}".format(1, 2, 3)

-

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

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

-    positive.

-    """

-    data = 'Hello John Doe {0[0]}'.format(args)

-    print data

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

diff --git a/test/messages/func_string_format_py27.txt b/test/messages/func_string_format_py27.txt
deleted file mode 100644
index c94e953..0000000
--- a/test/messages/func_string_format_py27.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-E: 58:pprint_bad: Too many arguments for format string

-E: 64:pprint_bad: Not enough arguments for format string

-E: 65:pprint_bad: Too many arguments for format string

-E: 67:pprint_bad: Not enough arguments for format string

-E: 77:pprint_bad: Too many arguments for format string

-E: 79:pprint_bad: Not enough arguments for format string

-E: 80:pprint_bad: Too many arguments for format string

-W: 59:pprint_bad: Invalid format string

-W: 60:pprint_bad: Invalid format string

-W: 61:pprint_bad: Format string contains both automatic field numbering and manual field specification

-W: 62:pprint_bad: Missing keyword argument 'b' for format string

-W: 62:pprint_bad: Unused format argument 'c'

-W: 63:pprint_bad: Missing keyword argument 'a' for format string

-W: 66:pprint_bad: Missing keyword argument 'a' for format string

-W: 66:pprint_bad: Missing keyword argument 'b' for format string

-W: 66:pprint_bad: Missing keyword argument 'c' for format string

-W: 68:pprint_bad: Missing keyword argument 'a' for format string

-W: 68:pprint_bad: Missing keyword argument 'b' for format string

-W: 69:pprint_bad: Missing keyword argument 'a' for format string

-W: 70:pprint_bad: Missing format attribute 'length' in format specifier 'a.ids.__len__.length'

-W: 71:pprint_bad: Using invalid lookup key 400 in format specifier 'a.ids[3][400]'

-W: 72:pprint_bad: Using invalid lookup key "'string'" in format specifier 'a.ids[3]["\'string\'"]'

-W: 73:pprint_bad: Using invalid lookup key 1 in format specifier '0[0][1]'

-W: 74:pprint_bad: Using invalid lookup key 0 in format specifier '0[0][0]'

-W: 75:pprint_bad: Missing keyword argument 'b' for format string

-W: 75:pprint_bad: Unused format argument 'a'

-W: 76:pprint_bad: Using invalid lookup key 0 in format specifier 'a[0]'

-W: 78:pprint_bad: Missing format attribute 'missing' in format specifier '0.missing'
\ No newline at end of file