Improve the reporting for dangerous-default-value.

The new version takes in consideration the fact that a couple of builtins,
dict, tuple, set and list can be inferred to their AST syntax node, which messed up
the reporting for dangerous-default-value until now.

4 files changed, 41 insertions, 13 deletions

diff --git a/checkers/base.py b/checkers/base.py
index 604685a..943f65e 100644
--- a/checkers/base.py
+++ b/checkers/base.py
@@ -676,7 +676,13 @@ functions, methods
         variable names, max locals
         """
         self.stats[node.is_method() and 'method' or 'function'] += 1
+        self._check_dangerous_default(node)
+
+    def _check_dangerous_default(self, node):
         # check for dangerous default values as arguments
+        is_iterable = lambda n: isinstance(n, (astroid.List,
+                                               astroid.Set,
+                                               astroid.Dict))
         for default in node.args.defaults:
             try:
                 value = next(default.infer())
@@ -685,24 +691,30 @@ functions, methods
 
             if (isinstance(value, astroid.Instance) and
                     value.qname() in DEFAULT_ARGUMENT_SYMBOLS):
-                is_infered_builtin = isinstance(
-                    value,
-                    (astroid.List, astroid.Tuple, astroid.Set))
+
                 if value is default:
                     msg = DEFAULT_ARGUMENT_SYMBOLS[value.qname()]
-                elif type(value) is astroid.Instance or is_infered_builtin:
-                    if isinstance(default, astroid.CallFunc):
-                        # this argument is direct call to list() or dict() etc
+                elif type(value) is astroid.Instance or is_iterable(value):
+                    # We are here in the following situation(s):
+                    #   * a dict/set/list/tuple call which wasn't inferred
+                    #     to a syntax node ({}, () etc.). This can happen
+                    #     when the arguments are invalid or unknown to
+                    #     the inference.
+                    #   * a variable from somewhere else, which turns out to be a list
+                    #     or a dict.
+                    if is_iterable(default):
+                        msg = value.pytype()
+                    elif isinstance(default, astroid.CallFunc):
                         msg = '%s() (%s)' % (value.name, value.qname())
                     else:
-                        # this argument is a variable from somewhere else which turns
-                        # out to be a list or dict
                         msg = '%s (%s)' % (default.as_string(), value.qname())
                 else:
                     # this argument is a name
                     msg = '%s (%s)' % (default.as_string(),
                                        DEFAULT_ARGUMENT_SYMBOLS[value.qname()])
-                self.add_message('dangerous-default-value', node=node, args=(msg,))
+                self.add_message('dangerous-default-value',
+                                 node=node,
+                                 args=(msg, ))
 
     @check_messages('unreachable', 'lost-exception')
     def visit_return(self, node):
diff --git a/test/input/func_dangerous_default.py b/test/input/func_dangerous_default.py
index 768f70f..dcf7380 100644
--- a/test/input/func_dangerous_default.py
+++ b/test/input/func_dangerous_default.py
@@ -62,3 +62,15 @@ OINK = {
 def function13(value=OINK):
     """dictionaries with items should not output item values in error message"""
     print value
+
+def function14(value=dict([(1, 2), (1, 2, 3)])):
+    """a dictionary which will not be inferred to a syntax AST, but to an
+    astroid.Instance.
+    """
+    return value
+
+INVALID_DICT = dict([(1, 2), (1, 2, 3)])
+
+def function15(value=INVALID_DICT):
+    """The same situation as function14."""
+    return value
diff --git a/test/messages/func_dangerous_default.txt b/test/messages/func_dangerous_default.txt
index ddf94f4..8980b7c 100644
--- a/test/messages/func_dangerous_default.txt
+++ b/test/messages/func_dangerous_default.txt
@@ -1,5 +1,5 @@
 W:  7:function1: Dangerous default value [] as argument
-W: 11:function2: Dangerous default value HEHE ({}) as argument
+W: 11:function2: Dangerous default value HEHE (__builtin__.dict) as argument
 W: 19:function4: Dangerous default value set() (__builtin__.set) as argument
 W: 29:function6: Dangerous default value GLOBAL_SET (__builtin__.set) as argument
 W: 33:function7: Dangerous default value dict() (__builtin__.dict) as argument
@@ -8,4 +8,6 @@ W: 41:function9: Dangerous default value [] as argument
 W: 45:function10: Dangerous default value {} as argument
 W: 49:function11: Dangerous default value list() (__builtin__.list) as argument
 W: 53:function12: Dangerous default value dict() (__builtin__.dict) as argument
-W: 62:function13: Dangerous default value OINK ({}) as argument
+W: 62:function13: Dangerous default value OINK (__builtin__.dict) as argument
+W: 66:function14: Dangerous default value dict() (__builtin__.dict) as argument
+W: 74:function15: Dangerous default value INVALID_DICT (__builtin__.dict) as argument
diff --git a/test/messages/func_dangerous_default_py30.txt b/test/messages/func_dangerous_default_py30.txt
index 1a6dbf0..eb05c87 100644
--- a/test/messages/func_dangerous_default_py30.txt
+++ b/test/messages/func_dangerous_default_py30.txt
@@ -1,5 +1,5 @@
 W:  7:function1: Dangerous default value [] as argument
-W: 11:function2: Dangerous default value HEHE ({}) as argument
+W: 11:function2: Dangerous default value HEHE (builtins.dict) as argument
 W: 19:function4: Dangerous default value set() (builtins.set) as argument
 W: 29:function6: Dangerous default value GLOBAL_SET (builtins.set) as argument
 W: 33:function7: Dangerous default value dict() (builtins.dict) as argument
@@ -8,4 +8,6 @@ W: 41:function9: Dangerous default value [] as argument
 W: 45:function10: Dangerous default value {} as argument
 W: 49:function11: Dangerous default value list() (builtins.list) as argument
 W: 53:function12: Dangerous default value dict() (builtins.dict) as argument
-W: 62:function13: Dangerous default value OINK ({}) as argument
+W: 62:function13: Dangerous default value OINK (builtins.dict) as argument
+W: 66:function14: Dangerous default value dict() (builtins.dict) as argument
+W: 74:function15: Dangerous default value INVALID_DICT (builtins.dict) as argument