Merge branch 'pylint'

19 files changed, 66 insertions, 14 deletions

diff --git a/.github/workflows/pylint.yml b/.github/workflows/pylint.yml
index eac9aa5c..c15f8f64 100644
--- a/.github/workflows/pylint.yml
+++ b/.github/workflows/pylint.yml
@@ -27,6 +27,6 @@ jobs:
       run: |
         python -m pip install --upgrade pip
         pip install -r requirements.txt
-    - name: Lint with pylint
+    - name: Lint with pylint, test with pytest
       run: |
-        make test_pylint
+        make test_pylint test_pytest
diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml
index b96d6812..dc41318e 100644
--- a/.github/workflows/python.yml
+++ b/.github/workflows/python.yml
@@ -31,4 +31,4 @@ jobs:
         pip install -r requirements.txt
     - name: Flake8 and test
       run: |
-        make test_flake8 test_pytest test_doctest test_other
+        make test_flake8 test_doctest test_other
diff --git a/.pylintrc b/.pylintrc
index 994ddf62..9b4ad466 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -13,7 +13,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,raise-missing-from,redefined-variable-type,stop-iteration-return,super-with-arguments,useless-object-inheritance
+disable=consider-using-f-string,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,raise-missing-from,redefined-variable-type,stop-iteration-return,super-with-arguments,useless-object-inheritance
 
 [TYPECHECK]
 ignored-classes=ranger.core.actions.Actions
diff --git a/doc/tools/convert_papermode_to_metadata.py b/doc/tools/convert_papermode_to_metadata.py
index 0a1b21ce..58371193 100755
--- a/doc/tools/convert_papermode_to_metadata.py
+++ b/doc/tools/convert_papermode_to_metadata.py
@@ -39,6 +39,7 @@ def replace(source, target):
     result = {}
 
     # Read the input file and convert it to a dictionary
+    # pylint: disable=unspecified-encoding
     with open(".paperinfo", "r") as infile:
         reader = csv.reader(infile, skipinitialspace=True)
         for lineno, row in enumerate(reader):
@@ -61,6 +62,7 @@ def replace(source, target):
     if result:
         # There's no way to specify encoding in 2.x even though in this case we
         # could choose to write in UTF-8.
+        # pylint: disable=unspecified-encoding
         with open(".metadata.json", "w") as outfile:
             json.dump(result, outfile, indent=2)
     else:
diff --git a/examples/plugin_ipc.py b/examples/plugin_ipc.py
index da6a19b0..0e7de3bb 100644
--- a/examples/plugin_ipc.py
+++ b/examples/plugin_ipc.py
@@ -32,6 +32,7 @@ def hook_init(fm):
             while True:
                 # The IPC encoding depends on the system locale so we can't
                 # guess here.
+                # pylint: disable=unspecified-encoding
                 with open(filepath, 'r') as fifo:
                     line = fifo.read()
                     fm.execute_console(line.strip())
diff --git a/ranger/config/.pylintrc b/ranger/config/.pylintrc
index 316bf189..509c6f31 100644
--- a/ranger/config/.pylintrc
+++ b/ranger/config/.pylintrc
@@ -5,4 +5,4 @@ class-rgx=[a-z][a-z0-9_]{1,30}$
 [FORMAT]
 max-line-length = 99
 max-module-lines=3000
-disable=duplicate-code,fixme,import-outside-toplevel,locally-disabled,locally-enabled,missing-docstring,no-else-return,super-with-arguments
+disable=consider-using-f-string,duplicate-code,fixme,import-outside-toplevel,locally-disabled,locally-enabled,missing-docstring,no-else-return,super-with-arguments
diff --git a/ranger/container/file.py b/ranger/container/file.py
index 9477abe7..4cc29887 100644
--- a/ranger/container/file.py
+++ b/ranger/container/file.py
@@ -86,7 +86,7 @@ class File(FileSystemObject):
             return True
         if PREVIEW_BLACKLIST.search(self.basename):
             return False
-        if self.path == '/dev/core' or self.path == '/proc/kcore':
+        if self.path in ('/dev/core', '/proc/kcore'):
             return False
         if self.is_binary():
             return False
diff --git a/ranger/container/tags.py b/ranger/container/tags.py
index e1ceec6f..4d4e6c59 100644
--- a/ranger/container/tags.py
+++ b/ranger/container/tags.py
@@ -123,6 +123,7 @@ class Tags(FileManagerAware):
             elif path.startswith(path_old + sep):
                 pnew = path_new + path[len(path_old):]
             if pnew:
+                # pylint: disable=unnecessary-dict-index-lookup
                 del self.tags[path]
                 self.tags[pnew] = tag
                 changed = True
diff --git a/ranger/core/actions.py b/ranger/core/actions.py
index 6cbc9ef7..a552e328 100644
--- a/ranger/core/actions.py
+++ b/ranger/core/actions.py
@@ -372,6 +372,7 @@ class Actions(  # pylint: disable=too-many-instance-attributes,too-many-public-m
         """
         filename = os.path.expanduser(filename)
         LOG.debug("Sourcing config file '%s'", filename)
+        # pylint: disable=unspecified-encoding
         with open(filename, 'r', encoding="utf-8") as fobj:
             for line in fobj:
                 line = line.strip(" \r\n")
@@ -1053,6 +1054,7 @@ class Actions(  # pylint: disable=too-many-instance-attributes,too-many-public-m
                 # XXX: properly determine file's encoding
                 # Disable the lint because the preview is read outside the
                 # local scope.
+                # pylint: disable=consider-using-with
                 return codecs.open(path, 'r', errors='ignore')
             # IOError for Python2, OSError for Python3
             except (IOError, OSError):
@@ -1438,6 +1440,7 @@ class Actions(  # pylint: disable=too-many-instance-attributes,too-many-public-m
             contexts = 'browser', 'console', 'pager', 'taskview'
 
         # Disable lint because TemporaryFiles are removed on close
+        # pylint: disable=consider-using-with
         temporary_file = tempfile.NamedTemporaryFile()
 
         def write(string):  # pylint: disable=redefined-outer-name
@@ -1464,6 +1467,7 @@ class Actions(  # pylint: disable=too-many-instance-attributes,too-many-public-m
 
     def dump_commands(self):
         # Disable lint because TemporaryFiles are removed on close
+        # pylint: disable=consider-using-with
         temporary_file = tempfile.NamedTemporaryFile()
 
         def write(string):  # pylint: disable=redefined-outer-name
@@ -1490,6 +1494,7 @@ class Actions(  # pylint: disable=too-many-instance-attributes,too-many-public-m
 
     def dump_settings(self):
         # Disable lint because TemporaryFiles are removed on close
+        # pylint: disable=consider-using-with
         temporary_file = tempfile.NamedTemporaryFile()
 
         def write(string):  # pylint: disable=redefined-outer-name
diff --git a/ranger/core/loader.py b/ranger/core/loader.py
index 866902fb..19611c7b 100644
--- a/ranger/core/loader.py
+++ b/ranger/core/loader.py
@@ -175,6 +175,7 @@ class CommandLoader(  # pylint: disable=too-many-instance-attributes
         # pylint: disable=too-many-branches,too-many-statements
         # TODO: Check whether we can afford to wait for processes and use a
         #       with-statement for Popen.
+        # pylint: disable=consider-using-with
         popenargs = {} if self.popenArgs is None else self.popenArgs
         popenargs['stdout'] = popenargs['stderr'] = PIPE
         popenargs['stdin'] = (
@@ -191,7 +192,7 @@ class CommandLoader(  # pylint: disable=too-many-instance-attributes
             try:
                 stdin.write(self.input)
             except IOError as ex:
-                if ex.errno != errno.EPIPE and ex.errno != errno.EINVAL:
+                if ex.errno not in (errno.EPIPE, errno.EINVAL):
                     raise
             stdin.close()
         if self.silent and not self.read:  # pylint: disable=too-many-nested-blocks
diff --git a/ranger/core/runner.py b/ranger/core/runner.py
index acd85fb8..1d2b91f7 100644
--- a/ranger/core/runner.py
+++ b/ranger/core/runner.py
@@ -193,6 +193,7 @@ class Runner(object):  # pylint: disable=too-few-public-methods
             context.wait = False
         if 's' in context.flags:
             # Using a with-statement for these is inconvenient.
+            # pylint: disable=consider-using-with
             devnull_writable = open(os.devnull, 'w', encoding="utf-8")
             devnull_readable = open(os.devnull, 'r', encoding="utf-8")
             for key in ('stdout', 'stderr'):
@@ -243,6 +244,7 @@ class Runner(object):  # pylint: disable=too-few-public-methods
                 if 'f' in context.flags and 'r' not in context.flags:
                     # This can fail and return False if os.fork() is not
                     # supported, but we assume it is, since curses is used.
+                    # pylint: disable=consider-using-with
                     Popen_forked(**popen_kws)
                 else:
                     process = Popen(**popen_kws)
diff --git a/ranger/ext/img_display.py b/ranger/ext/img_display.py
index 97b8f43f..61e10f96 100644
--- a/ranger/ext/img_display.py
+++ b/ranger/ext/img_display.py
@@ -143,6 +143,7 @@ class W3MImageDisplayer(ImageDisplayer, FileManagerAware):
         self.binary_path = None
         self.binary_path = self._find_w3mimgdisplay_executable()  # may crash
         # We cannot close the process because that stops the preview.
+        # pylint: disable=consider-using-with
         self.process = Popen([self.binary_path] + W3MIMGDISPLAY_OPTIONS, cwd=self.working_dir,
                              stdin=PIPE, stdout=PIPE, universal_newlines=True)
         self.is_initialized = True
@@ -642,7 +643,7 @@ class KittyImageDisplayer(ImageDisplayer, FileManagerAware):
             image = image.resize((int(scale * image.width), int(scale * image.height)),
                                  self.backend.LANCZOS)
 
-        if image.mode != 'RGB' and image.mode != 'RGBA':
+        if image.mode not in ('RGB', 'RGBA'):
             image = image.convert('RGB')
         # start_x += ((box[0] - image.width) // 2) // self.pix_row
         # start_y += ((box[1] - image.height) // 2) // self.pix_col
@@ -744,6 +745,7 @@ class UeberzugImageDisplayer(ImageDisplayer):
             return
 
         # We cannot close the process because that stops the preview.
+        # pylint: disable=consider-using-with
         self.process = Popen(['ueberzug', 'layer', '--silent'], cwd=self.working_dir,
                              stdin=PIPE, universal_newlines=True)
         self.is_initialized = True
diff --git a/ranger/ext/macrodict.py b/ranger/ext/macrodict.py
index b4613fbc..924fe5a9 100644
--- a/ranger/ext/macrodict.py
+++ b/ranger/ext/macrodict.py
@@ -15,7 +15,7 @@ def macro_val(thunk, fallback=MACRO_FAIL):
 try:
     from collections.abc import MutableMapping  # pylint: disable=no-name-in-module
 except ImportError:
-    from collections import MutableMapping
+    from collections import MutableMapping  # pylint: disable=deprecated-class
 
 
 class MacroDict(MutableMapping):
diff --git a/ranger/ext/popen_forked.py b/ranger/ext/popen_forked.py
index 46054e9c..40a5b833 100644
--- a/ranger/ext/popen_forked.py
+++ b/ranger/ext/popen_forked.py
@@ -24,7 +24,7 @@ def Popen_forked(*args, **kwargs):  # pylint: disable=invalid-name
         ) as null_w:
             kwargs['stdin'] = null_r
             kwargs['stdout'] = kwargs['stderr'] = null_w
-            Popen(*args, **kwargs)
+            Popen(*args, **kwargs)  # pylint: disable=consider-using-with
         os._exit(0)  # pylint: disable=protected-access
     else:
         os.wait()
diff --git a/ranger/ext/rifle.py b/ranger/ext/rifle.py
index 53800ed6..fbfe7c5a 100755
--- a/ranger/ext/rifle.py
+++ b/ranger/ext/rifle.py
@@ -141,12 +141,13 @@ except ImportError:
             return False
         if pid == 0:
             os.setsid()
+            # pylint: disable=unspecified-encoding
             with open(os.devnull, "r") as null_r, open(
                 os.devnull, "w"
             ) as null_w:
                 kwargs["stdin"] = null_r
                 kwargs["stdout"] = kwargs["stderr"] = null_w
-                Popen(*args, **kwargs)
+                Popen(*args, **kwargs)  # pylint: disable=consider-using-with
             os._exit(0)  # pylint: disable=protected-access
         return True
 
@@ -223,6 +224,7 @@ class Rifle(object):  # pylint: disable=too-many-instance-attributes
         """Replace the current configuration with the one in config_file"""
         if config_file is None:
             config_file = self.config_file
+        # pylint: disable=unspecified-encoding
         with open(config_file, "r") as fobj:
             self.rules = []
             for line in fobj:
diff --git a/ranger/ext/vcs/vcs.py b/ranger/ext/vcs/vcs.py
index 455bd175..93fb1d0b 100644
--- a/ranger/ext/vcs/vcs.py
+++ b/ranger/ext/vcs/vcs.py
@@ -88,6 +88,7 @@ class Vcs(object):  # pylint: disable=too-many-instance-attributes
         if self.root:
             if self.is_root:
                 self.rootvcs = self
+                # pylint: disable=invalid-class-object
                 self.__class__ = globals()[self.REPOTYPES[self.repotype]['class'] + 'Root']
 
                 if not os.access(self.repodir, os.R_OK):
@@ -101,6 +102,7 @@ class Vcs(object):  # pylint: disable=too-many-instance-attributes
                 if self.rootvcs is None or self.rootvcs.root is None:
                     return
                 self.rootvcs.links |= self.links
+                # pylint: disable=invalid-class-object
                 self.__class__ = globals()[self.REPOTYPES[self.repotype]['class']]
                 self.track = self.rootvcs.track
 
@@ -461,10 +463,10 @@ class VcsThread(threading.Thread):  # pylint: disable=too-many-instance-attribut
             self.paused.set()
             self._advance.wait()
             self._awoken.wait()
-            if self.__stop.isSet():
+            if self.__stop.is_set():
                 self.stopped.set()
                 return
-            if not self._advance.isSet():
+            if not self._advance.is_set():
                 continue
             self._awoken.clear()
             self.paused.clear()
@@ -489,7 +491,7 @@ class VcsThread(threading.Thread):  # pylint: disable=too-many-instance-attribut
         self._advance.set()
         self._awoken.set()
         self.stopped.wait(1)
-        return self.stopped.isSet()
+        return self.stopped.is_set()
 
     def pause(self):
         """Pause thread"""
diff --git a/tests/manpage_completion_test.py b/tests/manpage_completion_test.py
index 3b067b46..1a01f944 100755
--- a/tests/manpage_completion_test.py
+++ b/tests/manpage_completion_test.py
@@ -26,6 +26,7 @@ def get_path_of_man_page():
 
 def read_manpage():
     path = get_path_of_man_page()
+    # pylint: disable=unspecified-encoding
     with open(path, 'r') as man_page:
         return man_page.read()
 
diff --git a/tests/pylint/py2_compat.py b/tests/pylint/py2_compat.py
index 7e136148..e0353260 100644
--- a/tests/pylint/py2_compat.py
+++ b/tests/pylint/py2_compat.py
@@ -51,6 +51,9 @@ class Py2CompatibilityChecker(BaseChecker):
                   "Python 2 subprocess.Popen objects were not contextmanagers,"
                   "popen23.Popen wraps them to enable use with"
                   "with-statements."),
+        "E4240": ("Use format method",
+                  "use-format-method",
+                  "Python 2 (and <3.6) does not support f-strings."),
     }
     # This class variable declares the options
     # that are configurable by the user.
@@ -121,6 +124,11 @@ class Py2CompatibilityChecker(BaseChecker):
                     self.add_message("implicit-format-spec", node=node,
                                      confidence=HIGH)
 
+    def visit_joinedstr(self, node):
+        """Make sure we don't use f-strings"""
+        if isinstance(node, astroid.nodes.JoinedStr):
+            self.add_message("use-format-method", node=node, confidence=HIGH)
+
     def visit_with(self, node):
         """Make sure subprocess.Popen objects aren't used in with-statements"""
         for (cm, _) in node.items:
diff --git a/tests/pylint/test_py2_compat.py b/tests/pylint/test_py2_compat.py
index 2eb51599..33fc5681 100644
--- a/tests/pylint/test_py2_compat.py
+++ b/tests/pylint/test_py2_compat.py
@@ -4,6 +4,7 @@ import py2_compat
 
 import astroid
 import pylint.testutils
+from pylint.interfaces import HIGH
 
 from sys import version_info
 PY2 = version_info[0] < 3
@@ -25,6 +26,7 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
             pylint.testutils.MessageTest(
                 msg_id='old-style-class',
                 node=oldstyle_class,
+                confidence=HIGH,
             ),
         ):
             self.checker.visit_classdef(oldstyle_class)
@@ -56,6 +58,7 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
             pylint.testutils.MessageTest(
                 msg_id='print-without-import',
                 node=print_function_call,
+                confidence=HIGH,
             ),
         ):
             self.checker.visit_call(print_function_call)
@@ -95,6 +98,7 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
             pylint.testutils.MessageTest(
                 msg_id='print-without-import',
                 node=early_print_function_call,
+                confidence=HIGH,
             ),
         ):
             self.checker.visit_call(early_print_function_call)
@@ -111,6 +115,7 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
             pylint.testutils.MessageTest(
                 msg_id='implicit-format-spec',
                 node=implicit_format_spec,
+                confidence=HIGH,
             ),
         ):
             self.checker.visit_call(implicit_format_spec)
@@ -134,6 +139,7 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
             pylint.testutils.MessageTest(
                 msg_id='with-popen23',
                 node=with_Popen,
+                confidence=HIGH,
             ),
         ):
             self.checker.visit_with(with_subprocess_Popen)
@@ -141,6 +147,25 @@ class TestPy2CompatibilityChecker(pylint.testutils.CheckerTestCase):
         with self.assertNoMessages():
             self.checker.visit_with(with_Popen23)
 
+    def test_use_format(self):
+        old_format, new_format, f_string = astroid.extract_node("""
+            "2 + 2 is %s" % (2+2) #@
+            "2 + 2 is {0}".format(2+2) #@
+            f"2 + 2 is {2+2}" #@
+        """)
+
+        with self.assertAddsMessages(
+            pylint.testutils.MessageTest(
+                msg_id='use-format-method',
+                node=f_string,
+                confidence=HIGH,
+            ),
+        ):
+            self.checker.visit_joinedstr(f_string)
+        with self.assertNoMessages():
+            self.checker.visit_joinedstr(old_format)
+            self.checker.visit_joinedstr(new_format)
+
     # # These checks still exist as old-division and no-absolute-import
     # def test_division_without_import(self):
     #     division = astroid.extract_node("""