Fix too agressive logging-format-interpolation

Fixes #572

3 files changed, 37 insertions, 15 deletions

diff --git a/pylint/checkers/logging.py b/pylint/checkers/logging.py
index 486c52dbd..be031a931 100644
--- a/pylint/checkers/logging.py
+++ b/pylint/checkers/logging.py
@@ -7,6 +7,7 @@
 
 """checker for use of Python logging
 """
+import string
 
 import six
 
@@ -56,15 +57,15 @@ MSGS = {
     }
 
 
-CHECKED_CONVENIENCE_FUNCTIONS = set([
-    'critical', 'debug', 'error', 'exception', 'fatal', 'info', 'warn',
-    'warning'])
+CHECKED_CONVENIENCE_FUNCTIONS = {'critical', 'debug', 'error', 'exception',
+                                 'fatal', 'info', 'warn', 'warning'}
 
-def is_method_call(callfunc_node, types=(), methods=()):
-    """Determines if a CallFunc node represents a method call.
+
+def is_method_call(func, types=(), methods=()):
+    """Determines if a BoundMethod node represents a method call.
 
     Args:
-      callfunc_node (astroid.CallFunc): The CallFunc AST node to check.
+      func (astroid.BoundMethod): The BoundMethod AST node to check.
       types (Optional[String]): Optional sequence of caller type names to restrict check.
       methods (Optional[String]): Optional sequence of method names to restrict check.
 
@@ -72,16 +73,12 @@ def is_method_call(callfunc_node, types=(), methods=()):
       bool: true if the node represents a method call for the given type and
       method names, False otherwise.
     """
-    if not isinstance(callfunc_node, astroid.Call):
-        return False
-    func = utils.safe_infer(callfunc_node.func)
     return (isinstance(func, astroid.BoundMethod)
             and isinstance(func.bound, astroid.Instance)
             and (func.bound.name in types if types else True)
             and (func.name in methods if methods else True))
 
 
-
 class LoggingChecker(checkers.BaseChecker):
     """Checks use of the logging module."""
 
@@ -183,15 +180,18 @@ class LoggingChecker(checkers.BaseChecker):
         elif isinstance(node.args[format_pos], astroid.Const):
             self._check_format_string(node, format_pos)
 
-    def _check_call_func(self, callfunc_node):
+    def _check_call_func(self, node):
         """Checks that function call is not format_string.format().
 
         Args:
-          callfunc_node (astroid.node_classes.NodeNG):
+          node (astroid.node_classes.CallFunc):
             CallFunc AST node to be checked.
         """
-        if is_method_call(callfunc_node, ('str', 'unicode'), ('format',)):
-            self.add_message('logging-format-interpolation', node=callfunc_node)
+        func = utils.safe_infer(node.func)
+        types = ('str', 'unicode')
+        methods = ('format',)
+        if is_method_call(func, types, methods) and not is_complex_format_str(func.bound):
+            self.add_message('logging-format-interpolation', node=node)
 
     def _check_format_string(self, node, format_arg):
         """Checks that format string tokens match the supplied arguments.
@@ -232,6 +232,23 @@ class LoggingChecker(checkers.BaseChecker):
             self.add_message('logging-too-few-args', node=node)
 
 
+def is_complex_format_str(node):
+    """ Checks if node represents a string with complex formatting specs.
+
+    Args:
+        node (astroid.node_classes.NodeNG): AST node to check
+    Returns:
+        bool: True if inferred string uses complex formatting, False otherwise
+    """
+    inferred = utils.safe_infer(node)
+    if inferred is None or not isinstance(inferred.value, six.string_types):
+        return True
+    for _, _, format_spec, _ in string.Formatter().parse(inferred.value):
+        if format_spec:
+            return True
+    return False
+
+
 def _count_supplied_tokens(args):
     """Counts the number of tokens in an args list.
 
diff --git a/pylint/test/functional/logging_format_interpolation.py b/pylint/test/functional/logging_format_interpolation.py
index 0f8f64db3..cbfe87e17 100644
--- a/pylint/test/functional/logging_format_interpolation.py
+++ b/pylint/test/functional/logging_format_interpolation.py
@@ -8,6 +8,7 @@ except ImportError:
 # Muck up the names in an effort to confuse...
 import logging as renamed_logging
 import os as logging
+from uninferable import UNINFERABLE
 
 FORMAT_STR = '{0}, {1}'
 
@@ -16,6 +17,7 @@ renamed_logging.debug('{0}, {1}'.format(4, 5)) # [logging-format-interpolation]
 renamed_logging.log(renamed_logging.DEBUG, 'msg: {}'.format('Run!')) # [logging-format-interpolation]
 renamed_logging.debug(FORMAT_STR.format(4, 5)) # [logging-format-interpolation]
 renamed_logging.log(renamed_logging.DEBUG, FORMAT_STR.format(4, 5)) # [logging-format-interpolation]
+renamed_logging.info("Read {l} rows".format(l=123456789)) # [logging-format-interpolation]
 
 # Statements that should not be flagged:
 renamed_logging.debug(format(66, 'x'))
@@ -23,3 +25,5 @@ renamed_logging.debug(builtins.format(66, 'x'))
 renamed_logging.log(renamed_logging.DEBUG, 'msg: Run!'.upper())
 logging.debug('{0}, {1}'.format(4, 5))
 logging.log(logging.DEBUG, 'msg: {}'.format('Run!'))
+renamed_logging.info("Read {l:,d} rows".format(l=123456789))
+renamed_logging.info(UNINFERABLE.format(l=123456789))
diff --git a/pylint/test/functional/logging_format_interpolation.txt b/pylint/test/functional/logging_format_interpolation.txt
index 75281658a..36051253c 100644
--- a/pylint/test/functional/logging_format_interpolation.txt
+++ b/pylint/test/functional/logging_format_interpolation.txt
@@ -1,4 +1,5 @@
-logging-format-interpolation:15::Use % formatting in logging functions and pass the % parameters as arguments
 logging-format-interpolation:16::Use % formatting in logging functions and pass the % parameters as arguments
 logging-format-interpolation:17::Use % formatting in logging functions and pass the % parameters as arguments
 logging-format-interpolation:18::Use % formatting in logging functions and pass the % parameters as arguments
+logging-format-interpolation:19::Use % formatting in logging functions and pass the % parameters as arguments
+logging-format-interpolation:20::Use % formatting in logging functions and pass the % parameters as arguments