Add a new recommendation checker, 'consider-iterating-dictionary'

This error is emitted when a dictionary is iterated through .keys(), instead
of iterating the dictionary directly, as in 'for key in dictionary:'.

Close #699

4 files changed, 64 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index b5ea0164c..617442a4b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,11 @@ ChangeLog for Pylint
 
 --
 
+    * Add a new recommendation checker, 'consider-iterating-dictionary', which is emitted
+      which is emitted when a dictionary is iterated through .keys().
+
+      Close #699
+
     * Use the configparser backport for Python 2
 
       This fixes a problem we were having with comments inside values, which is fixed
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index 38e4b0c0a..36f3bd42b 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -46,7 +46,8 @@ from pylint.checkers.utils import (
     error_of_type,
     unimplemented_abstract_methods,
     has_known_bases,
-    safe_infer
+    safe_infer,
+    is_comprehension
     )
 from pylint.reporters.ureports.nodes import Table
 
@@ -1510,6 +1511,11 @@ class RecommandationChecker(_BasicChecker):
                       'Emitted when code that iterates with range and len is '
                       'encountered. Such code can be simplified by using the '
                       'enumerate builtin.'),
+            'C0201': ('Consider iterating the dictionary directly instead of calling .keys()',
+                      'consider-iterating-dictionary',
+                      'Emitted when the keys of a dictionary are iterated through the .keys() '
+                      'method. It is enough to just iterate through the dictionary itself, as '
+                      'in "for key in dictionary".'),
            }
 
     @staticmethod
@@ -1519,6 +1525,25 @@ class RecommandationChecker(_BasicChecker):
             return False
         return is_builtin_object(inferred) and inferred.name == function
 
+    @check_messages('consider-iterating-dictionary')
+    def visit_call(self, node):
+        inferred = safe_infer(node.func)
+        if inferred in (astroid.Uninferable, None):
+            return
+
+        if not isinstance(inferred, astroid.BoundMethod):
+            return
+        if not isinstance(inferred.bound, astroid.Dict) or inferred.name != 'keys':
+            return
+
+        # Check if the statement is what we're expecting to have.
+        statement = node.statement()
+        if isinstance(statement, astroid.Expr):
+            statement = statement.value
+
+        if isinstance(statement, astroid.For) or is_comprehension(statement):
+            self.add_message('consider-iterating-dictionary', node=node)
+
     @check_messages('consider-using-enumerate')
     def visit_for(self, node):
         """Emit a convention whenever range and len are used for indexing."""
diff --git a/pylint/test/functional/consider_iterating_dictionary.py b/pylint/test/functional/consider_iterating_dictionary.py
new file mode 100644
index 000000000..f5b95728a
--- /dev/null
+++ b/pylint/test/functional/consider_iterating_dictionary.py
@@ -0,0 +1,28 @@
+# pylint: disable=missing-docstring, expression-not-assigned, too-few-public-methods, no-member, import-error, no-self-use
+
+from unknown import Unknown
+
+
+class CustomClass(object):
+    def keys(self):
+        return []
+
+for key in Unknown().keys():
+    pass
+for key in Unknown.keys():
+    pass
+for key in dict.keys():
+    pass
+for key in {}.values():
+    pass
+for key in {}.key():
+    pass
+for key in CustomClass().keys():
+    pass
+
+[key for key in {}.keys()] # [consider-iterating-dictionary]
+(key for key in {}.keys()) # [consider-iterating-dictionary]
+{key for key in {}.keys()} # [consider-iterating-dictionary]
+{key: key for key in {}.keys()} # [consider-iterating-dictionary]
+for key in {}.keys(): # [consider-iterating-dictionary]
+    pass
diff --git a/pylint/test/functional/consider_iterating_dictionary.txt b/pylint/test/functional/consider_iterating_dictionary.txt
new file mode 100644
index 000000000..b862cbbb7
--- /dev/null
+++ b/pylint/test/functional/consider_iterating_dictionary.txt
@@ -0,0 +1,5 @@
+consider-iterating-dictionary:23::Consider iterating the dictionary directly instead of calling .keys()
+consider-iterating-dictionary:24::Consider iterating the dictionary directly instead of calling .keys()
+consider-iterating-dictionary:25::Consider iterating the dictionary directly instead of calling .keys()
+consider-iterating-dictionary:26::Consider iterating the dictionary directly instead of calling .keys()
+consider-iterating-dictionary:27::Consider iterating the dictionary directly instead of calling .keys()