Add a Python 3 checker for finding deprecated calls to `str.translate`. (#1188)

* Add a Python 3 checker for finding deprecated calls to `str.translate`.

This checker could possibly have some false positives, but after checking our codebase
and several other large codebases using this implementation, I did not find any false
positives.

* Add filter for types proven not to be strings

3 files changed, 124 insertions, 2 deletions

diff --git a/doc/whatsnew/2.0.rst b/doc/whatsnew/2.0.rst
index b10328a55..6b3cee835 100644
--- a/doc/whatsnew/2.0.rst
+++ b/doc/whatsnew/2.0.rst
@@ -410,6 +410,37 @@ New checkers
       "hello world!".upper()
 
 
+* A new Python 3 checker was added to warn about calling ``str.translate`` with the removed
+  ``deletechars`` parameter.  ``str.translate`` is frequently used as a way to remove characters
+  from a string.
+
+  .. code-block:: python
+
+      'hello world'.translate(None, 'low')
+
+  Unfortunately, there is not an idiomatic way of writing this call in a 2and3 compatible way.  If
+  this code is not in the critical path for your application and the use of ``translate`` was a
+  premature optimization, consider using ``re.sub`` instead:
+
+  .. code-block:: python
+
+      import re
+      chars_to_remove = re.compile('[low]')
+      chars_to_remove.sub('', 'hello world')
+
+  If this code is in your critical path and must be as fast as possible, consider declaring a
+  helper method that varies based upon Python version.
+
+  .. code-block:: python
+
+      if six.PY3:
+          def _remove_characters(text, deletechars):
+              return text.translate({ord(x): None for x in deletechars})
+      else:
+          def _remove_characters(text, deletechars):
+              return text.translate(None, deletechars)
+
+
 Other Changes
 =============
 
diff --git a/pylint/checkers/python3.py b/pylint/checkers/python3.py
index a82de4d33..2b91e82d6 100644
--- a/pylint/checkers/python3.py
+++ b/pylint/checkers/python3.py
@@ -389,6 +389,11 @@ class Python3Checker(checkers.BaseChecker):
                   'deprecated-string-function',
                   'Used when accessing a string function that has been deprecated in Python 3.',
                   {'maxversion': (3, 0)}),
+        'W1650': ('Using str.translate with deprecated deletechars parameters',
+                  'deprecated-str-translate-call',
+                  'Used when using the deprecated deletechars parameters from str.translate.  Use'
+                  're.sub to remove the desired characters ',
+                  {'maxversion': (3, 0)}),
     }
 
     _bad_builtins = frozenset([
@@ -594,22 +599,64 @@ class Python3Checker(checkers.BaseChecker):
                 self.add_message('using-cmp-argument', node=node)
                 return
 
+    @staticmethod
+    def _is_constant_string_or_name(node):
+        return ((isinstance(node, astroid.Const) and isinstance(node.value, six.string_types)) or
+                isinstance(node, astroid.Name))
+
+    @staticmethod
+    def _is_none(node):
+        return isinstance(node, astroid.Const) and node.value is None
+
+    @staticmethod
+    def _has_only_n_positional_args(node, number_of_args):
+        return len(node.args) == number_of_args and all(node.args) and not node.keywords
+
+    @staticmethod
+    def _could_be_string(inferred_types):
+        for inferred_type in inferred_types:
+            if not (inferred_type == astroid.Uninferable or (
+                    isinstance(inferred_type, astroid.Const) and
+                    isinstance(inferred_type.value, six.string_types))):
+                return False
+        return True
+
     def visit_call(self, node):
         self._check_cmp_argument(node)
 
         if isinstance(node.func, astroid.Attribute):
+            inferred_types = set()
             try:
                 for inferred_receiver in node.func.expr.infer():
+                    inferred_types.add(type(inferred_receiver))
                     if isinstance(inferred_receiver, astroid.Module):
                         self._warn_if_deprecated(node, inferred_receiver.name,
                                                  {node.func.attrname})
             except astroid.InferenceError:
                 pass
             if node.args:
-                if node.func.attrname in ('encode', 'decode'):
-                    if len(node.args) >= 1 and node.args[0]:
+                if self._could_be_string(inferred_types):
+                    if (node.func.attrname in ('encode', 'decode') and
+                            len(node.args) >= 1 and node.args[0]):
                         first_arg = node.args[0]
                         self._validate_encoding(first_arg, node)
+                    if (node.func.attrname == 'translate' and
+                            self._has_only_n_positional_args(node, 2) and
+                            self._is_none(node.args[0]) and
+                            self._is_constant_string_or_name(node.args[1])):
+                        # The above statement looking for calls of the form:
+                        #
+                        # foo.translate(None, 'abc123')
+                        #
+                        # or
+                        #
+                        # foo.translate(None, some_variable)
+                        #
+                        # This check is somewhat broad and _may_ have some false positives, but
+                        # after checking several large codebases it did not have any false
+                        # positives while finding several real issues.  This call pattern seems
+                        # rare enough that the trade off is worth it.
+                        self.add_message('deprecated-str-translate-call', node=node)
                 return
             if node.keywords:
                 return
diff --git a/pylint/test/unittest_checker_python3.py b/pylint/test/unittest_checker_python3.py
index 922bc4939..28a6d8052 100644
--- a/pylint/test/unittest_checker_python3.py
+++ b/pylint/test/unittest_checker_python3.py
@@ -647,6 +647,50 @@ class Python3CheckerTest(testutils.CheckerTestCase):
         with self.assertAddsMessages(absolute_import_message):
             self.checker.visit_importfrom(node)
 
+    @python2_only
+    def test_bad_str_translate_call_string_literal(self):
+        node = astroid.extract_node('''
+         foobar.translate(None, 'abc123') #@
+         ''')
+        message = testutils.Message('deprecated-str-translate-call', node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_call(node)
+
+    @python2_only
+    def test_bad_str_translate_call_variable(self):
+        node = astroid.extract_node('''
+         foobar.translate(None, foobar) #@
+         ''')
+        message = testutils.Message('deprecated-str-translate-call', node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_call(node)
+
+    @python2_only
+    def test_ok_str_translate_call_integer(self):
+        node = astroid.extract_node('''
+         foobar.translate(None, 33) #@
+         ''')
+        with self.assertNoMessages():
+            self.checker.visit_call(node)
+
+    @python2_only
+    def test_ok_str_translate_call_keyword(self):
+        node = astroid.extract_node('''
+         foobar.translate(None, 'foobar', raz=33) #@
+         ''')
+        with self.assertNoMessages():
+            self.checker.visit_call(node)
+
+    @python2_only
+    def test_ok_str_translate_call_not_str(self):
+        node = astroid.extract_node('''
+         foobar = {}
+         foobar.translate(None, 'foobar') #@
+         ''')
+        with self.assertNoMessages():
+            self.checker.visit_call(node)
+
+
 @python2_only
 class Python3TokenCheckerTest(testutils.CheckerTestCase):