feat(runfiles): Added type hints to `@rules_python//python/runfiles` (#1654)

This change adds mypy types directly to the
`rules_python.python.runfiles` library as well as regression testing to
CI.

---------

Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>

6 files changed, 210 insertions, 173 deletions

diff --git a/.github/workflows/mypy.yaml b/.github/workflows/mypy.yaml
new file mode 100644
index 0000000..b0d0cdf
--- /dev/null
+++ b/.github/workflows/mypy.yaml
@@ -0,0 +1,32 @@
+name: mypy
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    types:
+      - opened
+      - synchronize
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  ci:
+    runs-on: ubuntu-20.04
+    steps:
+      # Checkout the code
+      - uses: actions/checkout@v2
+      - uses: jpetrucciani/mypy-check@master
+        with:
+          requirements: 1.6.0
+          python_version: 3.8
+          path: 'python/runfiles'
+      - uses: jpetrucciani/mypy-check@master
+        with:
+          requirements: 1.6.0
+          python_version: 3.8
+          path: 'tests/runfiles'
+      
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c9baef7..e9af917 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,9 @@ A brief description of the categories of changes:
 
 ### Changed
 
+* (runfiles) `rules_python.python.runfiles` now directly implements type hints
+  and drops support for python2 as a result.
+
 * (toolchains) `py_runtime`, `py_runtime_pair`, and `PyRuntimeInfo` now use the
   rules_python Starlark implementation, not the one built into Bazel. NOTE: This
   only applies to Bazel 6+; Bazel 5 still uses the builtin implementation.
diff --git a/python/runfiles/BUILD.bazel b/python/runfiles/BUILD.bazel
index c6cfc2f..55c25c8 100644
--- a/python/runfiles/BUILD.bazel
+++ b/python/runfiles/BUILD.bazel
@@ -27,6 +27,7 @@ py_library(
         "__init__.py",
         "runfiles.py",
     ],
+    data = ["py.typed"],
     imports = [
         # Add the repo root so `import python.runfiles.runfiles` works. This makes it agnostic
         # to the --experimental_python_import_all_repositories setting.
@@ -49,6 +50,7 @@ py_wheel(
     dist_folder = "dist",
     distribution = "bazel_runfiles",
     homepage = "https://github.com/bazelbuild/rules_python",
+    python_requires = ">=3.7",
     strip_path_prefixes = ["python"],
     twine = "@publish_deps_twine//:pkg",
     # this can be replaced by building with --stamp --embed_label=1.2.3
diff --git a/python/runfiles/py.typed b/python/runfiles/py.typed
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/python/runfiles/py.typed
diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 9bdb61b..2240979 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -14,82 +14,116 @@
 
 """Runfiles lookup library for Bazel-built Python binaries and tests.
 
-See README.md for usage instructions.
+See @rules_python//python/runfiles/README.rst for usage instructions.
 """
 import inspect
 import os
 import posixpath
 import sys
+from typing import Dict, Optional, Tuple, Union
 
-if False:
-    # Mypy needs these symbols imported, but since they only exist in python 3.5+,
-    # this import may fail at runtime. Luckily mypy can follow this conditional import.
-    from typing import Callable, Dict, Optional, Tuple, Union
-
-
-def CreateManifestBased(manifest_path):
-    # type: (str) -> _Runfiles
-    return _Runfiles(_ManifestBased(manifest_path))
 
+class _ManifestBased:
+    """`Runfiles` strategy that parses a runfiles-manifest to look up runfiles."""
 
-def CreateDirectoryBased(runfiles_dir_path):
-    # type: (str) -> _Runfiles
-    return _Runfiles(_DirectoryBased(runfiles_dir_path))
+    def __init__(self, path: str) -> None:
+        if not path:
+            raise ValueError()
+        if not isinstance(path, str):
+            raise TypeError()
+        self._path = path
+        self._runfiles = _ManifestBased._LoadRunfiles(path)
 
+    def RlocationChecked(self, path: str) -> Optional[str]:
+        """Returns the runtime path of a runfile."""
+        exact_match = self._runfiles.get(path)
+        if exact_match:
+            return exact_match
+        # If path references a runfile that lies under a directory that
+        # itself is a runfile, then only the directory is listed in the
+        # manifest. Look up all prefixes of path in the manifest and append
+        # the relative path from the prefix to the looked up path.
+        prefix_end = len(path)
+        while True:
+            prefix_end = path.rfind("/", 0, prefix_end - 1)
+            if prefix_end == -1:
+                return None
+            prefix_match = self._runfiles.get(path[0:prefix_end])
+            if prefix_match:
+                return prefix_match + "/" + path[prefix_end + 1 :]
 
-def Create(env=None):
-    # type: (Optional[Dict[str, str]]) -> Optional[_Runfiles]
-    """Returns a new `Runfiles` instance.
+    @staticmethod
+    def _LoadRunfiles(path: str) -> Dict[str, str]:
+        """Loads the runfiles manifest."""
+        result = {}
+        with open(path, "r") as f:
+            for line in f:
+                line = line.strip()
+                if line:
+                    tokens = line.split(" ", 1)
+                    if len(tokens) == 1:
+                        result[line] = line
+                    else:
+                        result[tokens[0]] = tokens[1]
+        return result
 
-    The returned object is either:
-    - manifest-based, meaning it looks up runfile paths from a manifest file, or
-    - directory-based, meaning it looks up runfile paths under a given directory
-      path
+    def _GetRunfilesDir(self) -> str:
+        if self._path.endswith("/MANIFEST") or self._path.endswith("\\MANIFEST"):
+            return self._path[: -len("/MANIFEST")]
+        if self._path.endswith(".runfiles_manifest"):
+            return self._path[: -len("_manifest")]
+        return ""
 
-    If `env` contains "RUNFILES_MANIFEST_FILE" with non-empty value, this method
-    returns a manifest-based implementation. The object eagerly reads and caches
-    the whole manifest file upon instantiation; this may be relevant for
-    performance consideration.
+    def EnvVars(self) -> Dict[str, str]:
+        directory = self._GetRunfilesDir()
+        return {
+            "RUNFILES_MANIFEST_FILE": self._path,
+            "RUNFILES_DIR": directory,
+            # TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can
+            # pick up RUNFILES_DIR.
+            "JAVA_RUNFILES": directory,
+        }
 
-    Otherwise, if `env` contains "RUNFILES_DIR" with non-empty value (checked in
-    this priority order), this method returns a directory-based implementation.
 
-    If neither cases apply, this method returns null.
+class _DirectoryBased:
+    """`Runfiles` strategy that appends runfiles paths to the runfiles root."""
 
-    Args:
-      env: {string: string}; optional; the map of environment variables. If None,
-          this function uses the environment variable map of this process.
-    Raises:
-      IOError: if some IO error occurs.
-    """
-    env_map = os.environ if env is None else env
-    manifest = env_map.get("RUNFILES_MANIFEST_FILE")
-    if manifest:
-        return CreateManifestBased(manifest)
+    def __init__(self, path: str) -> None:
+        if not path:
+            raise ValueError()
+        if not isinstance(path, str):
+            raise TypeError()
+        self._runfiles_root = path
 
-    directory = env_map.get("RUNFILES_DIR")
-    if directory:
-        return CreateDirectoryBased(directory)
+    def RlocationChecked(self, path: str) -> str:
+        # Use posixpath instead of os.path, because Bazel only creates a runfiles
+        # tree on Unix platforms, so `Create()` will only create a directory-based
+        # runfiles strategy on those platforms.
+        return posixpath.join(self._runfiles_root, path)
 
-    return None
+    def EnvVars(self) -> Dict[str, str]:
+        return {
+            "RUNFILES_DIR": self._runfiles_root,
+            # TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can
+            # pick up RUNFILES_DIR.
+            "JAVA_RUNFILES": self._runfiles_root,
+        }
 
 
-class _Runfiles(object):
+class Runfiles:
     """Returns the runtime location of runfiles.
 
     Runfiles are data-dependencies of Bazel-built binaries and tests.
     """
 
-    def __init__(self, strategy):
-        # type: (Union[_ManifestBased, _DirectoryBased]) -> None
+    def __init__(self, strategy: Union[_ManifestBased, _DirectoryBased]) -> None:
         self._strategy = strategy
         self._python_runfiles_root = _FindPythonRunfilesRoot()
         self._repo_mapping = _ParseRepoMapping(
             strategy.RlocationChecked("_repo_mapping")
         )
 
-    def Rlocation(self, path, source_repo=None):
-        # type: (str, Optional[str]) -> Optional[str]
+    def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[str]:
         """Returns the runtime path of a runfile.
 
         Runfiles are data-dependencies of Bazel-built binaries and tests.
@@ -153,14 +187,17 @@ class _Runfiles(object):
             #   which also should not be mapped.
             return self._strategy.RlocationChecked(path)
 
+        assert (
+            source_repo is not None
+        ), "BUG: if the `source_repo` is None, we should never go past the `if` statement above"
+
         # target_repo is an apparent repository name. Look up the corresponding
         # canonical repository name with respect to the current repository,
         # identified by its canonical name.
         target_canonical = self._repo_mapping[(source_repo, target_repo)]
         return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
 
-    def EnvVars(self):
-        # type: () -> Dict[str, str]
+    def EnvVars(self) -> Dict[str, str]:
         """Returns environment variables for subprocesses.
 
         The caller should set the returned key-value pairs in the environment of
@@ -173,8 +210,7 @@ class _Runfiles(object):
         """
         return self._strategy.EnvVars()
 
-    def CurrentRepository(self, frame=1):
-        # type: (int) -> str
+    def CurrentRepository(self, frame: int = 1) -> str:
         """Returns the canonical name of the caller's Bazel repository.
 
         For example, this function returns '' (the empty string) when called
@@ -204,12 +240,11 @@ class _Runfiles(object):
             ValueError: if the caller cannot be determined or the caller's file
             path is not contained in the Python runfiles tree
         """
-        # pylint:disable=protected-access  # for sys._getframe
-        # pylint:disable=raise-missing-from  # we're still supporting Python 2
         try:
+            # pylint: disable-next=protected-access
             caller_path = inspect.getfile(sys._getframe(frame))
-        except (TypeError, ValueError):
-            raise ValueError("failed to determine caller's file path")
+        except (TypeError, ValueError) as exc:
+            raise ValueError("failed to determine caller's file path") from exc
         caller_runfiles_path = os.path.relpath(caller_path, self._python_runfiles_root)
         if caller_runfiles_path.startswith(".." + os.path.sep):
             raise ValueError(
@@ -233,8 +268,11 @@ class _Runfiles(object):
         return caller_runfiles_directory
 
 
-def _FindPythonRunfilesRoot():
-    # type: () -> str
+# Support legacy imports by defining a private symbol.
+_Runfiles = Runfiles
+
+
+def _FindPythonRunfilesRoot() -> str:
     """Finds the root of the Python runfiles tree."""
     root = __file__
     # Walk up our own runfiles path to the root of the runfiles tree from which
@@ -246,8 +284,7 @@ def _FindPythonRunfilesRoot():
     return root
 
 
-def _ParseRepoMapping(repo_mapping_path):
-    # type: (Optional[str]) -> Dict[Tuple[str, str], str]
+def _ParseRepoMapping(repo_mapping_path: Optional[str]) -> Dict[Tuple[str, str], str]:
     """Parses the repository mapping manifest."""
     # If the repository mapping file can't be found, that is not an error: We
     # might be running without Bzlmod enabled or there may not be any runfiles.
@@ -271,98 +308,45 @@ def _ParseRepoMapping(repo_mapping_path):
     return repo_mapping
 
 
-class _ManifestBased(object):
-    """`Runfiles` strategy that parses a runfiles-manifest to look up runfiles."""
+def CreateManifestBased(manifest_path: str) -> Runfiles:
+    return Runfiles(_ManifestBased(manifest_path))
 
-    def __init__(self, path):
-        # type: (str) -> None
-        if not path:
-            raise ValueError()
-        if not isinstance(path, str):
-            raise TypeError()
-        self._path = path
-        self._runfiles = _ManifestBased._LoadRunfiles(path)
 
-    def RlocationChecked(self, path):
-        # type: (str) -> Optional[str]
-        """Returns the runtime path of a runfile."""
-        exact_match = self._runfiles.get(path)
-        if exact_match:
-            return exact_match
-        # If path references a runfile that lies under a directory that
-        # itself is a runfile, then only the directory is listed in the
-        # manifest. Look up all prefixes of path in the manifest and append
-        # the relative path from the prefix to the looked up path.
-        prefix_end = len(path)
-        while True:
-            prefix_end = path.rfind("/", 0, prefix_end - 1)
-            if prefix_end == -1:
-                return None
-            prefix_match = self._runfiles.get(path[0:prefix_end])
-            if prefix_match:
-                return prefix_match + "/" + path[prefix_end + 1 :]
+def CreateDirectoryBased(runfiles_dir_path: str) -> Runfiles:
+    return Runfiles(_DirectoryBased(runfiles_dir_path))
 
-    @staticmethod
-    def _LoadRunfiles(path):
-        # type: (str) -> Dict[str, str]
-        """Loads the runfiles manifest."""
-        result = {}
-        with open(path, "r") as f:
-            for line in f:
-                line = line.strip()
-                if line:
-                    tokens = line.split(" ", 1)
-                    if len(tokens) == 1:
-                        result[line] = line
-                    else:
-                        result[tokens[0]] = tokens[1]
-        return result
 
-    def _GetRunfilesDir(self):
-        # type: () -> str
-        if self._path.endswith("/MANIFEST") or self._path.endswith("\\MANIFEST"):
-            return self._path[: -len("/MANIFEST")]
-        elif self._path.endswith(".runfiles_manifest"):
-            return self._path[: -len("_manifest")]
-        else:
-            return ""
+def Create(env: Optional[Dict[str, str]] = None) -> Optional[Runfiles]:
+    """Returns a new `Runfiles` instance.
 
-    def EnvVars(self):
-        # type: () -> Dict[str, str]
-        directory = self._GetRunfilesDir()
-        return {
-            "RUNFILES_MANIFEST_FILE": self._path,
-            "RUNFILES_DIR": directory,
-            # TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can
-            # pick up RUNFILES_DIR.
-            "JAVA_RUNFILES": directory,
-        }
+    The returned object is either:
+    - manifest-based, meaning it looks up runfile paths from a manifest file, or
+    - directory-based, meaning it looks up runfile paths under a given directory
+      path
 
+    If `env` contains "RUNFILES_MANIFEST_FILE" with non-empty value, this method
+    returns a manifest-based implementation. The object eagerly reads and caches
+    the whole manifest file upon instantiation; this may be relevant for
+    performance consideration.
 
-class _DirectoryBased(object):
-    """`Runfiles` strategy that appends runfiles paths to the runfiles root."""
+    Otherwise, if `env` contains "RUNFILES_DIR" with non-empty value (checked in
+    this priority order), this method returns a directory-based implementation.
 
-    def __init__(self, path):
-        # type: (str) -> None
-        if not path:
-            raise ValueError()
-        if not isinstance(path, str):
-            raise TypeError()
-        self._runfiles_root = path
+    If neither cases apply, this method returns null.
 
-    def RlocationChecked(self, path):
-        # type: (str) -> str
+    Args:
+      env: {string: string}; optional; the map of environment variables. If None,
+          this function uses the environment variable map of this process.
+    Raises:
+      IOError: if some IO error occurs.
+    """
+    env_map = os.environ if env is None else env
+    manifest = env_map.get("RUNFILES_MANIFEST_FILE")
+    if manifest:
+        return CreateManifestBased(manifest)
 
-        # Use posixpath instead of os.path, because Bazel only creates a runfiles
-        # tree on Unix platforms, so `Create()` will only create a directory-based
-        # runfiles strategy on those platforms.
-        return posixpath.join(self._runfiles_root, path)
+    directory = env_map.get("RUNFILES_DIR")
+    if directory:
+        return CreateDirectoryBased(directory)
 
-    def EnvVars(self):
-        # type: () -> Dict[str, str]
-        return {
-            "RUNFILES_DIR": self._runfiles_root,
-            # TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can
-            # pick up RUNFILES_DIR.
-            "JAVA_RUNFILES": self._runfiles_root,
-        }
+    return None
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 5cc9568..03350f3 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -1,4 +1,3 @@
-# pylint: disable=g-bad-file-header
 # Copyright 2018 The Bazel Authors. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -16,18 +15,20 @@
 import os
 import tempfile
 import unittest
+from typing import Any, List, Optional
 
 from python.runfiles import runfiles
 
 
 class RunfilesTest(unittest.TestCase):
-    # """Unit tests for `runfiles.Runfiles`."""
+    """Unit tests for `rules_python.python.runfiles.Runfiles`."""
 
-    def testRlocationArgumentValidation(self):
+    def testRlocationArgumentValidation(self) -> None:
         r = runfiles.Create({"RUNFILES_DIR": "whatever"})
-        self.assertRaises(ValueError, lambda: r.Rlocation(None))
+        assert r is not None  # mypy doesn't understand the unittest api.
+        self.assertRaises(ValueError, lambda: r.Rlocation(None))  # type: ignore
         self.assertRaises(ValueError, lambda: r.Rlocation(""))
-        self.assertRaises(TypeError, lambda: r.Rlocation(1))
+        self.assertRaises(TypeError, lambda: r.Rlocation(1))  # type: ignore
         self.assertRaisesRegex(
             ValueError, "is not normalized", lambda: r.Rlocation("../foo")
         )
@@ -61,7 +62,7 @@ class RunfilesTest(unittest.TestCase):
             lambda: r.Rlocation("\\foo"),
         )
 
-    def testCreatesManifestBasedRunfiles(self):
+    def testCreatesManifestBasedRunfiles(self) -> None:
         with _MockFile(contents=["a/b c/d"]) as mf:
             r = runfiles.Create(
                 {
@@ -70,10 +71,11 @@ class RunfilesTest(unittest.TestCase):
                     "TEST_SRCDIR": "always ignored",
                 }
             )
+            assert r is not None  # mypy doesn't understand the unittest api.
             self.assertEqual(r.Rlocation("a/b"), "c/d")
             self.assertIsNone(r.Rlocation("foo"))
 
-    def testManifestBasedRunfilesEnvVars(self):
+    def testManifestBasedRunfilesEnvVars(self) -> None:
         with _MockFile(name="MANIFEST") as mf:
             r = runfiles.Create(
                 {
@@ -81,6 +83,7 @@ class RunfilesTest(unittest.TestCase):
                     "TEST_SRCDIR": "always ignored",
                 }
             )
+            assert r is not None  # mypy doesn't understand the unittest api.
             self.assertDictEqual(
                 r.EnvVars(),
                 {
@@ -97,6 +100,7 @@ class RunfilesTest(unittest.TestCase):
                     "TEST_SRCDIR": "always ignored",
                 }
             )
+            assert r is not None  # mypy doesn't understand the unittest api.
             self.assertDictEqual(
                 r.EnvVars(),
                 {
@@ -117,6 +121,7 @@ class RunfilesTest(unittest.TestCase):
                     "TEST_SRCDIR": "always ignored",
                 }
             )
+            assert r is not None  # mypy doesn't understand the unittest api.
             self.assertDictEqual(
                 r.EnvVars(),
                 {
@@ -126,23 +131,25 @@ class RunfilesTest(unittest.TestCase):
                 },
             )
 
-    def testCreatesDirectoryBasedRunfiles(self):
+    def testCreatesDirectoryBasedRunfiles(self) -> None:
         r = runfiles.Create(
             {
                 "RUNFILES_DIR": "runfiles/dir",
                 "TEST_SRCDIR": "always ignored",
             }
         )
+        assert r is not None  # mypy doesn't understand the unittest api.
         self.assertEqual(r.Rlocation("a/b"), "runfiles/dir/a/b")
         self.assertEqual(r.Rlocation("foo"), "runfiles/dir/foo")
 
-    def testDirectoryBasedRunfilesEnvVars(self):
+    def testDirectoryBasedRunfilesEnvVars(self) -> None:
         r = runfiles.Create(
             {
                 "RUNFILES_DIR": "runfiles/dir",
                 "TEST_SRCDIR": "always ignored",
             }
         )
+        assert r is not None  # mypy doesn't understand the unittest api.
         self.assertDictEqual(
             r.EnvVars(),
             {
@@ -151,13 +158,13 @@ class RunfilesTest(unittest.TestCase):
             },
         )
 
-    def testFailsToCreateManifestBasedBecauseManifestDoesNotExist(self):
+    def testFailsToCreateManifestBasedBecauseManifestDoesNotExist(self) -> None:
         def _Run():
             runfiles.Create({"RUNFILES_MANIFEST_FILE": "non-existing path"})
 
         self.assertRaisesRegex(IOError, "non-existing path", _Run)
 
-    def testFailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined(self):
+    def testFailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined(self) -> None:
         with _MockFile(contents=["a b"]) as mf:
             runfiles.Create(
                 {
@@ -175,7 +182,7 @@ class RunfilesTest(unittest.TestCase):
         self.assertIsNone(runfiles.Create({"TEST_SRCDIR": "always ignored"}))
         self.assertIsNone(runfiles.Create({"FOO": "bar"}))
 
-    def testManifestBasedRlocation(self):
+    def testManifestBasedRlocation(self) -> None:
         with _MockFile(
             contents=[
                 "Foo/runfile1",
@@ -205,7 +212,7 @@ class RunfilesTest(unittest.TestCase):
             else:
                 self.assertEqual(r.Rlocation("/foo"), "/foo")
 
-    def testManifestBasedRlocationWithRepoMappingFromMain(self):
+    def testManifestBasedRlocationWithRepoMappingFromMain(self) -> None:
         with _MockFile(
             contents=[
                 ",config.json,config.json~1.2.3",
@@ -280,7 +287,7 @@ class RunfilesTest(unittest.TestCase):
             self.assertIsNone(r.Rlocation("my_module", ""))
             self.assertIsNone(r.Rlocation("protobuf", ""))
 
-    def testManifestBasedRlocationWithRepoMappingFromOtherRepo(self):
+    def testManifestBasedRlocationWithRepoMappingFromOtherRepo(self) -> None:
         with _MockFile(
             contents=[
                 ",config.json,config.json~1.2.3",
@@ -362,7 +369,7 @@ class RunfilesTest(unittest.TestCase):
             self.assertIsNone(r.Rlocation("my_module", "protobuf~3.19.2"))
             self.assertIsNone(r.Rlocation("protobuf", "protobuf~3.19.2"))
 
-    def testDirectoryBasedRlocation(self):
+    def testDirectoryBasedRlocation(self) -> None:
         # The _DirectoryBased strategy simply joins the runfiles directory and the
         # runfile's path on a "/". This strategy does not perform any normalization,
         # nor does it check that the path exists.
@@ -374,7 +381,7 @@ class RunfilesTest(unittest.TestCase):
         else:
             self.assertEqual(r.Rlocation("/foo"), "/foo")
 
-    def testDirectoryBasedRlocationWithRepoMappingFromMain(self):
+    def testDirectoryBasedRlocationWithRepoMappingFromMain(self) -> None:
         with _MockFile(
             name="_repo_mapping",
             contents=[
@@ -441,7 +448,7 @@ class RunfilesTest(unittest.TestCase):
 
             self.assertEqual(r.Rlocation("config.json", ""), dir + "/config.json")
 
-    def testDirectoryBasedRlocationWithRepoMappingFromOtherRepo(self):
+    def testDirectoryBasedRlocationWithRepoMappingFromOtherRepo(self) -> None:
         with _MockFile(
             name="_repo_mapping",
             contents=[
@@ -513,40 +520,49 @@ class RunfilesTest(unittest.TestCase):
                 r.Rlocation("config.json", "protobuf~3.19.2"), dir + "/config.json"
             )
 
-    def testCurrentRepository(self):
+    def testCurrentRepository(self) -> None:
         # Under bzlmod, the current repository name is the empty string instead
         # of the name in the workspace file.
         if bool(int(os.environ["BZLMOD_ENABLED"])):
             expected = ""
         else:
             expected = "rules_python"
-        self.assertEqual(
-            runfiles.Create({"RUNFILES_DIR": "whatever"}).CurrentRepository(), expected
-        )
+        r = runfiles.Create({"RUNFILES_DIR": "whatever"})
+        assert r is not None  # mypy doesn't understand the unittest api.
+        self.assertEqual(r.CurrentRepository(), expected)
 
     @staticmethod
-    def IsWindows():
+    def IsWindows() -> bool:
         return os.name == "nt"
 
 
-class _MockFile(object):
-    def __init__(self, name=None, contents=None):
+class _MockFile:
+    def __init__(
+        self, name: Optional[str] = None, contents: Optional[List[Any]] = None
+    ) -> None:
         self._contents = contents or []
         self._name = name or "x"
-        self._path = None
+        self._path: Optional[str] = None
 
-    def __enter__(self):
+    def __enter__(self) -> Any:
         tmpdir = os.environ.get("TEST_TMPDIR")
         self._path = os.path.join(tempfile.mkdtemp(dir=tmpdir), self._name)
         with open(self._path, "wt") as f:
             f.writelines(l + "\n" for l in self._contents)
         return self
 
-    def __exit__(self, exc_type, exc_value, traceback):
-        os.remove(self._path)
-        os.rmdir(os.path.dirname(self._path))
-
-    def Path(self):
+    def __exit__(
+        self,
+        exc_type: Any,  # pylint: disable=unused-argument
+        exc_value: Any,  # pylint: disable=unused-argument
+        traceback: Any,  # pylint: disable=unused-argument
+    ) -> None:
+        if self._path:
+            os.remove(self._path)
+            os.rmdir(os.path.dirname(self._path))
+
+    def Path(self) -> str:
+        assert self._path is not None
         return self._path