Add basic typechecking

Signed-off-by: Thomas Scholtes <geigerzaehler@axiom.fm>

.github/workflows/main.yaml

@@ -26,6 +26,7 @@ jobs:
     - run: poetry run black --check .
     - run: poetry run isort --check .
     - run: poetry run flake8
+    - run: poetry run pyright --warnings
     - run: poetry run pytest
     - uses: coverallsapp/github-action@v2
       if: github.event_name == 'pull_request' && matrix.python-version == '3.8'

beetsplug/alternatives.py

@@ -18,6 +18,7 @@
 import traceback
 from concurrent import futures
 from enum import Enum
+from typing import Iterator, List, Optional, Tuple
 
 import beets
 import six
@@ -27,7 +28,7 @@
 from beets.ui import Subcommand, UserError, decargs, get_path_formats, input_yn, print_
 from beets.util import FilesystemError, bytestring_path, displayable_path, syspath
 
-from beetsplug import convert
+import beetsplug.convert as convert
 
 
 def _remove(path, soft=True):
@@ -62,7 +63,7 @@ def update(self, lib, options):
     def list_tracks(self, lib, options):
         if options.format is not None:
             (fmt,) = decargs([options.format])
-            beets.config[beets.library.Item._format_config_key].set(fmt)
+            beets.config[Item._format_config_key].set(fmt)
 
         alt = self.alternative(options.name, lib)
 
@@ -79,6 +80,7 @@ def alternative(self, name, lib):
 
         if conf["formats"].exists():
             fmt = conf["formats"].as_str()
+            assert isinstance(fmt, str)
             if fmt == "link":
                 return SymlinkView(self._log, name, lib, conf)
             else:
@@ -220,7 +222,7 @@ def matched_item_action(self, item):
         else:
             return (item, [Action.ADD])
 
-    def items_actions(self):
+    def items_actions(self) -> Iterator[Tuple[Item, List[Action]]]:
         matched_ids = set()
         for album in self.lib.albums():
             if self.query.match(album):
@@ -266,6 +268,7 @@ def update(self, create=None):
                     )
                     util.mkdirall(dest)
                     util.move(path, dest)
+                    assert path is not None
                     util.prune_dirs(os.path.dirname(path), root=self.directory)
                     self.set_path(item, dest)
                     item.store()
@@ -289,20 +292,22 @@ def update(self, create=None):
             item.store()
         converter.shutdown()
 
-    def destination(self, item):
-        return item.destination(basedir=self.directory, path_formats=self.path_formats)
+    def destination(self, item: Item) -> bytes:
+        path = item.destination(basedir=self.directory, path_formats=self.path_formats)
+        assert isinstance(path, bytes)
+        return path
 
     def set_path(self, item, path):
         item[self.path_key] = six.text_type(path, "utf8")
 
     @staticmethod
-    def _get_path(item, path_key):
+    def _get_path(item, path_key) -> Optional[bytes]:
         try:
             return item[path_key].encode("utf8")
         except KeyError:
             return None
 
-    def get_path(self, item):
+    def get_path(self, item) -> Optional[bytes]:
         return self._get_path(item, self.path_key)
 
     def remove_item(self, item):
@@ -364,7 +369,7 @@ def _convert(item):
 
         return Worker(_convert, self.max_workers)
 
-    def destination(self, item):
+    def destination(self, item: Item):
         dest = super(ExternalConvert, self).destination(item)
         if self.should_transcode(item):
             return os.path.splitext(dest)[0] + b"." + self.ext
@@ -450,7 +455,7 @@ def sync_art(self, item, path):
 
 
 class Worker(futures.ThreadPoolExecutor):
-    def __init__(self, fn, max_workers):
+    def __init__(self, fn, max_workers: Optional[int]):
         super(Worker, self).__init__(max_workers)
         self._tasks = set()
         self._fn = fn

pyproject.toml

@@ -34,6 +34,8 @@ isort = "^5.12.0"
 black = "^23.3.0"
 confuse = "^2.0.1"
 mediafile = "^0.12.0"
+typeguard = "^4.1.5"
+pyright = "^1.1.340"
 
 [tool.pytest.ini_options]
 addopts = "--cov --cov-report=term --cov-report=html"
@@ -43,9 +45,13 @@ filterwarnings = [
   "ignore:.*setlocale.*:DeprecationWarning:beets.util",
   "ignore:.*pkgutil.get_loader.*:DeprecationWarning:confuse.util",
 ]
+
 [tool.isort]
 profile = "black"
 
+[tool.pyright]
+typeCheckingMode = "basic"
+
 [build-system]
 requires = ["poetry-core"]
 build-backend = "poetry.core.masonry.api"

test/cli_test.py

@@ -7,6 +7,7 @@
 from confuse import ConfigValueError
 from helper import TestHelper, control_stdin
 from mediafile import MediaFile
+from typeguard import check_type
 
 
 class DocTest(TestHelper):
@@ -315,7 +316,7 @@ def test_move_and_write_after_tags_changed(self):
 
     def test_prune_after_move(self):
         item = self.add_external_track("myexternal")
-        artist_dir = os.path.dirname(self.get_path(item))
+        artist_dir = os.path.dirname(check_type(self.get_path(item), bytes))
         self.assertTrue(os.path.isdir(artist_dir))
 
         item["artist"] = "a new artist"
@@ -391,7 +392,7 @@ def touch_art(item, image_path):
         # affect the repository.
         image_dir = bytestring_path(self.mkdtemp())
         image_path = os.path.join(image_dir, b"image")
-        shutil.copy(self.IMAGE_FIXTURE1, syspath(image_path))
+        shutil.copy(self.IMAGE_FIXTURE1, check_type(syspath(image_path), bytes))
         touch_art(item, image_path)
 
         # Add a cover image, assert that it is being embedded.

test/conftest.py

@@ -0,0 +1,4 @@
+import typeguard
+
+typeguard.install_import_hook("beetsplug.alternatives")
+print("install import hooks")

test/helper.py

@@ -4,10 +4,12 @@
 import tempfile
 from concurrent import futures
 from contextlib import contextmanager
+from typing import Optional
 from unittest import TestCase
 from zlib import crc32
 
 import beets
+import beets.library
 import six
 from beets import logging, plugins, ui, util
 from beets.library import Item
@@ -16,7 +18,8 @@
 from mock import patch
 from six import StringIO
 
-from beetsplug import alternatives, convert
+import beetsplug.alternatives as alternatives
+import beetsplug.convert as convert
 
 logging.getLogger("beets").propagate = True
 
@@ -95,7 +98,7 @@ def _convert_args(args):
     return args
 
 
-class Assertions(object):
+class Assertions(TestCase):
     def assertFileTag(self, path, tag):
         self.assertIsFile(path)
         with open(syspath(path), "rb") as f:
@@ -162,13 +165,15 @@ def assertSymlink(self, link, target, absolute=True):
             )
 
 
-class MediaFileAssertions(object):
+class MediaFileAssertions(TestCase):
     def assertHasEmbeddedArtwork(self, path, compare_file=None):
         mediafile = MediaFile(syspath(path))
         self.assertIsNotNone(mediafile.art, msg="MediaFile has no embedded artwork")
         if compare_file:
             with open(syspath(compare_file), "rb") as compare_fh:
-                crc_is = crc32(mediafile.art)
+                crc_is = crc32(
+                    mediafile.art  # pyright: ignore[reportGeneralTypeIssues]
+                )
                 crc_expected = crc32(compare_fh.read())
                 self.assertEqual(
                     crc_is,
@@ -193,7 +198,7 @@ def assertMediaFileFields(self, path, **kwargs):
             )
 
 
-class TestHelper(TestCase, Assertions, MediaFileAssertions):
+class TestHelper(Assertions, MediaFileAssertions):
     def setUp(self, mock_worker=True):
         """Setup required for running test. Must be called before
         running any tests.
@@ -243,7 +248,9 @@ def setup_beets(self):
         self.config["directory"] = libdir
         self.libdir = bytestring_path(libdir)
 
-        self.lib = beets.library.Library(":memory:", self.libdir)
+        self.lib = beets.library.Library(
+            ":memory:", self.libdir  # pyright: ignore[reportGeneralTypeIssues]
+        )
         self.fixture_dir = os.path.join(
             bytestring_path(os.path.dirname(__file__)), b"fixtures"
         )
@@ -333,7 +340,7 @@ def add_external_album(self, ext_name, **kwargs):
         album.load()
         return album
 
-    def get_path(self, item, path_key="alt.myexternal"):
+    def get_path(self, item, path_key="alt.myexternal") -> Optional[bytes]:
         return alternatives.External._get_path(item, path_key)
 
 
@@ -355,5 +362,5 @@ def submit(self, *args, **kwargs):
         self._tasks.add(fut)
         return fut
 
-    def shutdown(wait=True):
+    def shutdown(self, wait=True):
         pass