refactor: use pylint and fix warnings (#578)

* refactor: use pylint and fix warnings

* fix: some cleanup on top

.github/workflows/ci.yml

@@ -26,6 +26,8 @@ jobs:
     - uses: actions/checkout@v2
     - uses: actions/setup-python@v2
     - uses: pre-commit/action@v2.0.3
+    - name: pylint
+      run: pipx run nox -s pylint
 
   tests:
     name: Tests on 🐍 ${{ matrix.python-version }} ${{ matrix.os }}

noxfile.py

@@ -16,6 +16,16 @@ def lint(session):
     session.run("pre-commit", "run", "--all-files", *session.posargs)
 
 
+@nox.session
+def pylint(session):
+    """
+    Run pylint.
+    """
+
+    session.install(".", "paramiko", "ipython", "pylint")
+    session.run("pylint", "plumbum", *session.posargs)
+
+
 @nox.session(python=ALL_PYTHONS, reuse_venv=True)
 def tests(session):
     """

plumbum/__init__.py

@@ -35,6 +35,10 @@
 See https://plumbum.readthedocs.io for full details
 """
 
+import sys
+from types import ModuleType
+from typing import List
+
 # Avoids a circular import error later
 import plumbum.path  # noqa: F401
 from plumbum.commands import (
@@ -83,10 +87,8 @@
 
 # ===================================================================================================
 # Module hack: ``from plumbum.cmd import ls``
+# Can be replaced by a real module with __getattr__ after Python 3.6 is dropped
 # ===================================================================================================
-import sys
-from types import ModuleType
-from typing import List
 
 
 class LocalModule(ModuleType):
@@ -99,7 +101,7 @@ def __getattr__(self, name):
         try:
             return local[name]
         except CommandNotFound:
-            raise AttributeError(name)
+            raise AttributeError(name) from None
 
     __path__ = []  # type: List[str]
     __file__ = __file__
@@ -108,10 +110,6 @@ def __getattr__(self, name):
 cmd = LocalModule(__name__ + ".cmd", LocalModule.__doc__)
 sys.modules[cmd.__name__] = cmd
 
-del sys
-del ModuleType
-del LocalModule
-
 
 def __dir__():
     "Support nice tab completion"

plumbum/cli/application.py

@@ -61,7 +61,7 @@ def get(self):
             try:
                 cls = getattr(mod, clsname)
             except AttributeError:
-                raise ImportError(f"cannot import name {clsname}")
+                raise ImportError(f"cannot import name {clsname}") from None
             self.subapplication = cls
         return self.subapplication
 
@@ -173,10 +173,10 @@ def __new__(cls, executable=None):
         instead of an expression with a dot in it."""
 
         if executable is None:
-            return cls.run()
             # This return value was not a class instance, so __init__ is never called
-        else:
-            return super().__new__(cls)
+            return cls.run()
+
+        return super().__new__(cls)
 
     def __init__(self, executable):
         # Filter colors
@@ -193,7 +193,7 @@ def __init__(self, executable):
         # Allow None for the colors
         self.COLOR_GROUPS = defaultdict(
             lambda: colors.do_nothing,
-            dict() if type(self).COLOR_GROUPS is None else type(self).COLOR_GROUPS,
+            {} if type(self).COLOR_GROUPS is None else type(self).COLOR_GROUPS,
         )
 
         self.COLOR_GROUP_TITLES = defaultdict(
@@ -286,9 +286,8 @@ class FooApp(cli.Application):
         """
 
         def wrapper(subapp):
-            attrname = "_subcommand_{}".format(
-                subapp if isinstance(subapp, str) else subapp.__name__
-            )
+            subname = subapp if isinstance(subapp, str) else subapp.__name__
+            attrname = f"_subcommand_{subname}"
             setattr(cls, attrname, Subcommand(name, subapp))
             return subapp
 
@@ -325,7 +324,7 @@ def _parse_args(self, argv):
                 )
                 break
 
-            elif a.startswith("--") and len(a) >= 3:
+            if a.startswith("--") and len(a) >= 3:
                 # [--name], [--name=XXX], [--name, XXX], [--name, ==, XXX],
                 # [--name=, XXX], [--name, =XXX]
                 eqsign = a.find("=")
@@ -400,12 +399,11 @@ def _parse_args(self, argv):
                 else:
                     if swfuncs[swinfo.func].swname == swname:
                         raise SwitchError(T_("Switch {0} already given").format(swname))
-                    else:
-                        raise SwitchError(
-                            T_("Switch {0} already given ({1} is equivalent)").format(
-                                swfuncs[swinfo.func].swname, swname
-                            )
+                    raise SwitchError(
+                        T_("Switch {0} already given ({1} is equivalent)").format(
+                            swfuncs[swinfo.func].swname, swname
                         )
+                    )
             else:
                 if swinfo.list:
                     swfuncs[swinfo.func] = SwitchParseInfo(swname, ([val],), index)
@@ -441,7 +439,6 @@ def _parse_args(self, argv):
     @classmethod
     def autocomplete(cls, argv):
         """This is supplied to make subclassing and testing argument completion methods easier"""
-        pass
 
     @staticmethod
     def _handle_argument(val, argtype, name):
@@ -454,7 +451,7 @@ def _handle_argument(val, argtype, name):
                     T_(
                         "Argument of {name} expected to be {argtype}, not {val!r}:\n    {ex!r}"
                     ).format(name=name, argtype=argtype, val=val, ex=ex)
-                )
+                ) from None
         else:
             return NotImplemented
 
@@ -515,7 +512,7 @@ def _validate_args(self, swfuncs, tailargs):
                     min_args,
                 ).format(min_args, tailargs)
             )
-        elif len(tailargs) > max_args:
+        if len(tailargs) > max_args:
             raise PositionalArgumentsError(
                 ngettext(
                     "Expected at most {0} positional argument, got {1}",
@@ -579,7 +576,11 @@ def _positional_validate(self, args, validator_list, varargs, argnames, varargna
         return out_args
 
     @classmethod
-    def run(cls, argv=None, exit=True):  # @ReservedAssignment
+    def run(
+        cls,
+        argv=None,
+        exit=True,  # pylint: disable=redefined-builtin
+    ):
         """
         Runs the application, taking the arguments from ``sys.argv`` by default if
         nothing is passed. If ``exit`` is
@@ -673,9 +674,9 @@ def _parse_kwd_args(self, switches):
         """Parses keywords (positional arguments), used by invoke."""
         swfuncs = {}
         for index, (swname, val) in enumerate(switches.items(), 1):
-            switch = getattr(type(self), swname)
-            swinfo = self._switches_by_func[switch._switch_info.func]
-            if isinstance(switch, CountOf):
+            switch_local = getattr(type(self), swname)
+            swinfo = self._switches_by_func[switch_local._switch_info.func]
+            if isinstance(switch_local, CountOf):
                 p = (range(val),)
             elif swinfo.list and not hasattr(val, "__iter__"):
                 raise SwitchError(
@@ -704,9 +705,10 @@ def main(self, *args):
                 print(T_("------"))
                 self.help()
                 return 1
-        else:
-            print(T_("main() not implemented"))
-            return 1
+            return 0
+
+        print(T_("main() not implemented"))
+        return 1
 
     def cleanup(self, retcode):
         """Called after ``main()`` and all sub-applications have executed, to perform any necessary cleanup.
@@ -857,11 +859,7 @@ def wrapped_paragraphs(text, width):
             for i, d in enumerate(reversed(m.defaults)):
                 tailargs[-i - 1] = f"[{tailargs[-i - 1]}={d}]"
         if m.varargs:
-            tailargs.append(
-                "{}...".format(
-                    m.varargs,
-                )
-            )
+            tailargs.append(f"{m.varargs}...")
         tailargs = " ".join(tailargs)
 
         utc = self.COLOR_USAGE_TITLE if self.COLOR_USAGE_TITLE else self.COLOR_USAGE
@@ -922,28 +920,28 @@ def switchs(by_groups, show_groups):
         indentation = "\n" + " " * (cols - wrapper.width)
 
         for switch_info, prefix, color in switchs(by_groups, True):
-            help = switch_info.help  # @ReservedAssignment
+            help_txt = switch_info.help
             if switch_info.list:
-                help += T_("; may be given multiple times")
+                help_txt += T_("; may be given multiple times")
             if switch_info.mandatory:
-                help += T_("; required")
+                help_txt += T_("; required")
             if switch_info.requires:
-                help += T_("; requires {0}").format(
+                help_txt += T_("; requires {0}").format(
                     ", ".join(
                         (("-" if len(switch) == 1 else "--") + switch)
                         for switch in switch_info.requires
                     )
                 )
             if switch_info.excludes:
-                help += T_("; excludes {0}").format(
+                help_txt += T_("; excludes {0}").format(
                     ", ".join(
                         (("-" if len(switch) == 1 else "--") + switch)
                         for switch in switch_info.excludes
                     )
                 )
 
             msg = indentation.join(
-                wrapper.wrap(" ".join(ln.strip() for ln in help.splitlines()))
+                wrapper.wrap(" ".join(ln.strip() for ln in help_txt.splitlines()))
             )
 
             if len(prefix) + wrapper.width >= cols:
@@ -960,15 +958,17 @@ def switchs(by_groups, show_groups):
                     subapp = subcls.get()
                     doc = subapp.DESCRIPTION if subapp.DESCRIPTION else getdoc(subapp)
                     if self.SUBCOMMAND_HELPMSG:
-                        help = doc + "; " if doc else ""  # @ReservedAssignment
-                        help += self.SUBCOMMAND_HELPMSG.format(
+                        help_str = doc + "; " if doc else ""
+                        help_str += self.SUBCOMMAND_HELPMSG.format(
                             parent=self.PROGNAME, sub=name
                         )
                     else:
-                        help = doc if doc else ""  # @ReservedAssignment
+                        help_str = doc if doc else ""
 
                     msg = indentation.join(
-                        wrapper.wrap(" ".join(ln.strip() for ln in help.splitlines()))
+                        wrapper.wrap(
+                            " ".join(ln.strip() for ln in help_str.splitlines())
+                        )
                     )
 
                     if len(name) + wrapper.width >= cols:

plumbum/cli/config.py

@@ -39,7 +39,6 @@ def __exit__(self, exc_type, exc_val, exc_tb):
     @abstractmethod
     def read(self):
         """Read in the linked file"""
-        pass
 
     @abstractmethod
     def write(self):
@@ -49,12 +48,10 @@ def write(self):
     @abstractmethod
     def _get(self, option):
         """Internal get function for subclasses"""
-        pass
 
     @abstractmethod
     def _set(self, option, value):
         """Internal set function for subclasses. Must return the value that was set."""
-        pass
 
     def get(self, option, default=None):
         "Get an item from the store, returns default if fails"
@@ -89,7 +86,7 @@ def read(self):
         super().read()
 
     def write(self):
-        with open(self.filename, "w") as f:
+        with open(self.filename, "w", encoding="utf-8") as f:
             self.parser.write(f)
         super().write()
 
@@ -107,7 +104,7 @@ def _get(self, option):
         try:
             return self.parser.get(sec, option)
         except (NoSectionError, NoOptionError):
-            raise KeyError(f"{sec}:{option}")
+            raise KeyError(f"{sec}:{option}") from None
 
     def _set(self, option, value):
         sec, option = self._sec_opt(option)

plumbum/cli/i18n.py

@@ -5,16 +5,18 @@
 if loc is None or loc.startswith("en"):
 
     class NullTranslation:
-        def gettext(self, str):
-            return str
+        def gettext(self, str1):  # pylint: disable=no-self-use
+            return str1
 
-        def ngettext(self, str1, strN, n):
+        def ngettext(self, str1, strN, n):  # pylint: disable=no-self-use
             if n == 1:
                 return str1.replace("{0}", str(n))
-            else:
-                return strN.replace("{0}", str(n))
 
-    def get_translation_for(package_name: str) -> NullTranslation:
+            return strN.replace("{0}", str(n))
+
+    def get_translation_for(
+        package_name: str,  # pylint: disable=unused-argument
+    ) -> NullTranslation:
         return NullTranslation()
 
 else:

plumbum/cli/image.py

@@ -34,18 +34,19 @@ def show(self, filename, double=False):
 
         import PIL.Image
 
-        if double:
-            return self.show_pil_double(PIL.Image.open(filename))
-        else:
-            return self.show_pil(PIL.Image.open(filename))
+        return (
+            self.show_pil_double(PIL.Image.open(filename))
+            if double
+            else self.show_pil(PIL.Image.open(filename))
+        )
 
     def _init_size(self, im):
         """Return the expected image size"""
         if self.size is None:
             term_size = get_terminal_size()
             return self.best_aspect(im.size, term_size)
-        else:
-            return self.size
+
+        return self.size
 
     def show_pil(self, im):
         "Standard show routine"
@@ -87,7 +88,7 @@ class ShowImageApp(cli.Application):
     )
 
     @cli.switch(["-c", "--colors"], cli.Range(1, 4), help="Level of color, 1-4")
-    def colors_set(self, n):
+    def colors_set(self, n):  # pylint: disable=no-self-use
         colors.use_color = n
 
     size = cli.SwitchAttr(["-s", "--size"], help="Size, should be in the form 100x150")