From 6a871101331b9b6ea19958946cfd4f5845505d38 Mon Sep 17 00:00:00 2001 From: toonn Date: Sun, 28 Jun 2020 22:24:19 +0200 Subject: 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. --- .pylintrc | 5 ++ ranger/ext/safe_path.py | 1 + tests/pylint/py2_compat.py | 130 +++++++++++++++++++++++++++++++++ tests/pylint/test_py2_compat.py | 156 ++++++++++++++++++++++++++++++++++++++++ tests/ranger/core/test_main.py | 1 + 5 files changed, 293 insertions(+) create mode 100644 tests/pylint/py2_compat.py create mode 100644 tests/pylint/test_py2_compat.py 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 -- cgit 1.4.1-2-gfad0