Implement python 2 compatibility checks

We have been stuck on pylint <2 for a long time now because it dropped
some of the python 2 lints we rely on. We maintain compatibility with
python 2.6+ and 3.5+ and a lack of lints makes especially the former
much harder. Incompatibilities had already snuck in in the form of
implicit format specs. By implementing a custom checker we can make sure
this doesn't happen again.

5 files changed, 293 insertions, 0 deletions

diff --git a/.pylintrc b/.pylintrc
index b42432ca..3fdcbaa8 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -1,3 +1,7 @@
+[MASTER]
+init-hook='import sys; sys.path.append("tests/pylint")'
+load-plugins=py2_compat
+
 [BASIC]
 good-names=i,j,k,n,x,y,ex,Run,_,fm,ui,fg,bg
 bad-names=foo,baz,toto,tutu,tata
@@ -8,6 +12,7 @@ max-branches=16
 
 [FORMAT]
 max-line-length = 99
+enable=no-absolute-import,old-division
 disable=cyclic-import,duplicate-code,fixme,import-outside-toplevel,locally-disabled,locally-enabled,missing-docstring,no-else-break,no-else-continue,no-else-raise,no-else-return,redefined-variable-type,stop-iteration-return,useless-object-inheritance
 
 [TYPECHECK]
diff --git a/ranger/ext/safe_path.py b/ranger/ext/safe_path.py
index b172b577..b2e360ea 100644
--- a/ranger/ext/safe_path.py
+++ b/ranger/ext/safe_path.py
@@ -1,6 +1,7 @@
 # This file is part of ranger, the console file manager.
 # License: GNU GPL version 3, see the file "AUTHORS" for details.
 
+from __future__ import absolute_import
 import os
 
 SUFFIX = '_'
diff --git a/tests/pylint/py2_compat.py b/tests/pylint/py2_compat.py
new file mode 100644
index 00000000..4c0d5817
--- /dev/null
+++ b/tests/pylint/py2_compat.py
@@ -0,0 +1,130 @@
+from __future__ import absolute_import
+
+import astroid
+
+from pylint.checkers import BaseChecker
+from pylint.interfaces import IAstroidChecker, HIGH
+
+from pylint.checkers import utils
+
+
+class Py2CompatibilityChecker(BaseChecker):
+    """Verify some simple properties of code compatible with both 2 and 3"""
+
+    __implements__ = IAstroidChecker
+
+    # The name defines a custom section of the config for this checker.
+    name = "py2-compat"
+    # The priority indicates the order that pylint will run the checkers.
+    priority = -1
+    # This class variable declares the messages (ie the warnings and errors)
+    # that the checker can emit.
+    msgs = {
+        # Each message has a code, a message that the user will see,
+        # a unique symbol that identifies the message,
+        # and a detailed help message
+        # that will be included in the documentation.
+        "E4200": ('Use explicit inheritance from "object"',
+            "old-style-class",
+            'Python 2 requires explicit inheritance from "object"'
+            ' for new-style classes.'),
+        # old-division
+        # "E4210": ('Use "//" for integer division or import from "__future__"',
+        #     "division-without-import",
+        #     'Python 2 might perform integer division unless you import'
+        #     ' "division" from "__future__".'),
+        # no-absolute-import
+        # "E4211": ('Always import "absolute_import" from "__future__"',
+        #     "old-no-absolute-import",
+        #     'Python 2 allows relative imports unless you import'
+        #     ' "absolute_import" from "__future__".'),
+        "E4212": ('Import "print_function" from "__future__"',
+            "print-without-import",
+            'Python 2 requires importing "print_function" from "__future__"'
+            ' to use the "print()" function syntax.'),
+        "E4220": ('Use explicit format spec numbering',
+            "implicit-format-spec",
+            'Python 2.6 does not support implicit format spec numbering "{}",'
+            ' use explicit numbering "{0}" or keywords "{key}".')
+    }
+    # This class variable declares the options
+    # that are configurable by the user.
+    options = ()
+
+    def visit_classdef(self, node):
+        """Make sure classes explicitly inherit from object"""
+        if not node.bases and node.type == 'class' and not node.metaclass():
+            # We use confidence HIGH here because this message should only ever
+            # be emitted for classes at the root of the inheritance hierarchy
+            self.add_message('old-style-class', node=node, confidence=HIGH)
+
+    def visit_call(self, node):
+        """Make sure "print_function" is imported if necessary"""
+        if (isinstance(node.func, astroid.nodes.Name)
+            and node.func.name == "print"
+           ):
+            if "print_function" in node.root().future_imports:
+                def previous(node):
+                    if node is not None:
+                        parent = node.parent
+                    previous = node.previous_sibling()
+                    if previous is None:
+                        return parent
+                    return previous
+
+                prev = previous(node)
+                while prev is not None:
+                    if (isinstance(prev, astroid.nodes.ImportFrom)
+                        and prev.modname == "__future__"
+                        and "print_function" in (name_alias[0] for name_alias in
+                            prev.names)
+                       ):
+                        return
+                    prev = previous(prev)
+
+                self.add_message("print-without-import", node=node,
+                    confidence=HIGH)
+            else:
+                self.add_message("print-without-import", node=node,
+                    confidence=HIGH)
+                    
+
+        func = utils.safe_infer(node.func)
+        if (
+            isinstance(func, astroid.BoundMethod)
+            and isinstance(func.bound, astroid.Instance)
+            and func.bound.name in ("str", "unicode", "bytes")
+        ):
+            if func.name == "format":
+                if isinstance(node.func, astroid.Attribute) and not isinstance(
+                    node.func.expr, astroid.Const
+                ):
+                    return
+                if node.starargs or node.kwargs:
+                    return
+                try:
+                    strnode = next(func.bound.infer())
+                except astroid.InferenceError:
+                    return
+                if not (isinstance(strnode, astroid.Const) and isinstance(
+                        strnode.value, str)
+                ):
+                    return
+                try:
+                    fields, num_args, manual_pos = (
+                        utils.parse_format_method_string(strnode.value)
+                    )
+                except utils.IncompleteFormatString:
+                    self.add_message("bad-format-string", node=node)
+                if num_args != 0:
+                    self.add_message("implicit-format-spec", node=node,
+                        confidence = HIGH)
+
+
+def register(linter):
+    """This required method auto registers the checker.
+
+    :param linter: The linter to register the checker to.
+    :type linter: pylint.lint.PyLinter
+    """
+    linter.register_checker(Py2CompatibilityChecker(linter))
diff --git a/tests/pylint/test_py2_compat.py b/tests/pylint/test_py2_compat.py
new file mode 100644
index 00000000..1d5c9486
--- /dev/null
+++ b/tests/pylint/test_py2_compat.py
@@ -0,0 +1,156 @@
+from __future__ import absolute_import
+
+import py2_compat
+
+import astroid
+import pylint.testutils
+
+class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = py2_compat.Py2CompatibilityChecker
+
+    def test_oldstyle_class(self):
+        oldstyle_class, from_old = astroid.extract_node("""
+        class OldStyle(): #@
+            pass
+
+        class FromOld(OldStyle): #@
+            pass
+        """)
+
+        with self.assertAddsMessages(
+            pylint.testutils.Message(
+                msg_id='old-style-class',
+                node=oldstyle_class,
+            ),
+        ):
+            self.checker.visit_classdef(oldstyle_class)
+
+        with self.assertNoMessages():
+            self.checker.visit_classdef(from_old)
+
+    def test_newstyle_class(self):
+        newstyle_class, from_new = astroid.extract_node("""
+        class NewStyle(object): #@
+            pass
+        class FromNew(NewStyle): #@
+            pass
+        """)
+
+        with self.assertNoMessages():
+            self.checker.visit_classdef(newstyle_class)
+            self.checker.visit_classdef(from_new)
+
+    def test_print_without_import(self):
+        print_function_call = astroid.extract_node("""
+        print("Print function call without importing print_function")
+        """)
+
+        with self.assertAddsMessages(
+            pylint.testutils.Message(
+                msg_id='print-without-import',
+                node=print_function_call,
+            ),
+        ):
+            self.checker.visit_call(print_function_call)
+
+    def test_print_with_import(self):
+        print_function_call = astroid.extract_node("""
+        from __future__ import print_function
+        print("Print function call with importing print_function") #@
+        """)
+
+        nested_print_function_call = astroid.extract_node("""
+        def f():
+            from __future__ import print_function
+            class A():
+                def m(self):
+                    print("Nested print with import in scope") #@
+        """)
+
+        with self.assertNoMessages():
+            self.checker.visit_call(print_function_call)
+            self.checker.visit_call(nested_print_function_call)
+
+    def test_print_late_import(self):
+        early_print_function_call = astroid.extract_node("""
+        print("Nested print with import in scope") #@
+        def f():
+            from __future__ import print_function
+            class A():
+                def m(self):
+                    pass
+        """)
+
+        with self.assertAddsMessages(
+            pylint.testutils.Message(
+                msg_id='print-without-import',
+                node=early_print_function_call,
+            ),
+        ):
+            self.checker.visit_call(early_print_function_call)
+
+    def test_implicit_format_spec(self):
+        implicit_format_spec = astroid.extract_node("""
+        "{}".format("implicit") #@
+        """)
+
+        with self.assertAddsMessages(
+            pylint.testutils.Message(
+                msg_id='implicit-format-spec',
+                node=implicit_format_spec,
+            ),
+        ):
+            self.checker.visit_call(implicit_format_spec)
+
+    ## These checks still exist as old-division and no-absolute-import
+    # def test_division_without_import(self):
+    #     division = astroid.extract_node("""
+    #     5/2
+    #     """)
+
+    #     with self.assertAddsMessages(
+    #         pylint.testutils.Message(
+    #             msg_id='division-without-import',
+    #             node=division,
+    #         ),
+    #     ):
+    #         self.checker.visit_XXX(division)
+
+    # def test_division_with_import(self):
+    #     division = astroid.extract_node("""
+    #     from __future__ import division
+    #     5/2 #@
+    #     """)
+
+    #     with self.assertNoMessages():
+    #         self.checker.visit_XXX(division)
+
+    # def test_absolute_import(self):
+    #     no_import = astroid.extract_node("""
+    #     import sys
+    #     """)
+
+    #     with self.assertAddsMessages(
+    #         pylint.testutils.Message(
+    #             msg_id='old-no-absolute-import',
+    #             node=no_import,
+    #         ),
+    #     ):
+    #         self.checker.visit_XXX(no_import)
+
+    #     only_import = astroid.extract_node("""
+    #     from __future__ import absolute_import
+    #     """)
+
+    #     first_import = astroid.extract_node("""
+    #     from __future__ import absolute_import, print_function
+    #     """)
+
+    #     second_import = astroid.extract_node("""
+    #     from __future__ import print_function, absolute_import
+    #     """)
+
+    #     with self.assertNoMessages():
+    #         self.checker.visit_XXX(only_import)
+    #         self.checker.visit_XXX(first_import)
+    #         self.checker.visit_XXX(second_import)
diff --git a/tests/ranger/core/test_main.py b/tests/ranger/core/test_main.py
index d992b8a7..fd209bf9 100644
--- a/tests/ranger/core/test_main.py
+++ b/tests/ranger/core/test_main.py
@@ -1,3 +1,4 @@
+from __future__ import absolute_import
 import collections
 import os