Improve typing of config_levels, add assert_never()

Yobmod · Yobmod · commit 16f0607ed29f · 2021-07-05T12:31:00.000+01:00
diff --git a/git/config.py b/git/config.py
@@ -31,9 +31,10 @@
 
 # typing-------------------------------------------------------
 
-from typing import Any, Callable, IO, List, Dict, Sequence, TYPE_CHECKING, Tuple, Union, cast, overload
+from typing import (Any, Callable, IO, List, Dict, Sequence,
+                    TYPE_CHECKING, Tuple, Union, cast, overload)
 
-from git.types import Literal, Lit_config_levels, PathLike, TBD
+from git.types import Lit_config_levels, ConfigLevels_Tup, ConfigLevels_NT, PathLike, TBD, assert_never
 
 if TYPE_CHECKING:
     from git.repo.base import Repo
@@ -48,8 +49,9 @@
 
 # invariants
 # represents the configuration level of a configuration file
-CONFIG_LEVELS = ("system", "user", "global", "repository"
-                 )  # type: Tuple[Literal['system'], Literal['user'], Literal['global'], Literal['repository']]
+
+
+CONFIG_LEVELS: ConfigLevels_Tup = ConfigLevels_NT("system", "user", "global", "repository")
 
 # Section pattern to detect conditional includes.
 # https://git-scm.com/docs/git-config#_conditional_includes
@@ -229,8 +231,9 @@ def get_config_path(config_level: Lit_config_levels) -> str:
         return osp.normpath(osp.expanduser("~/.gitconfig"))
     elif config_level == "repository":
         raise ValueError("No repo to get repository configuration from. Use Repo._get_config_path")
-
-    raise ValueError("Invalid configuration level: %r" % config_level)
+    else:
+        # Should not reach here. Will raise ValueError if does. Static typing will warn about extra and missing elifs
+        assert_never(config_level, ValueError("Invalid configuration level: %r" % config_level))
 
 
 class GitConfigParser(with_metaclass(MetaParserBuilder, cp.RawConfigParser, object)):  # type: ignore ## mypy does not understand dynamic class creation # noqa: E501
@@ -300,12 +303,12 @@ def __init__(self, file_or_files: Union[None, PathLike, IO, Sequence[Union[PathL
             self._proxies = self._dict()
 
         if file_or_files is not None:
-            self._file_or_files = file_or_files  # type: Union[PathLike, IO, Sequence[Union[PathLike, IO]]]
+            self._file_or_files: Union[PathLike, IO, Sequence[Union[PathLike, IO]]] = file_or_files
         else:
             if config_level is None:
                 if read_only:
-                    self._file_or_files = [get_config_path(f)  # type: ignore
-                                           for f in CONFIG_LEVELS    # Can type f properly when 3.5 dropped
+                    self._file_or_files = [get_config_path(f)
+                                           for f in CONFIG_LEVELS
                                            if f != 'repository']
                 else:
                     raise ValueError("No configuration level or configuration files specified")
@@ -323,15 +326,13 @@ def __init__(self, file_or_files: Union[None, PathLike, IO, Sequence[Union[PathL
     def _acquire_lock(self) -> None:
         if not self._read_only:
             if not self._lock:
-                if isinstance(self._file_or_files, (tuple, list)):
-                    raise ValueError(
-                        "Write-ConfigParsers can operate on a single file only, multiple files have been passed")
-                # END single file check
-
                 if isinstance(self._file_or_files, (str, os.PathLike)):
                     file_or_files = self._file_or_files
+                elif isinstance(self._file_or_files, (tuple, list, Sequence)):
+                    raise ValueError(
+                        "Write-ConfigParsers can operate on a single file only, multiple files have been passed")
                 else:
-                    file_or_files = cast(IO, self._file_or_files).name
+                    file_or_files = self._file_or_files.name
 
                 # END get filename from handle/stream
                 # initialize lock base - we want to write
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -7,6 +7,8 @@
 from unittest import SkipTest
 import uuid
 
+from git import IndexFile
+
 import git
 from git.cmd import Git
 from git.compat import (
@@ -49,7 +51,7 @@
 
 
 # typing ----------------------------------------------------------------------
-from typing import Dict, TYPE_CHECKING
+from typing import Callable, Dict, TYPE_CHECKING
 from typing import Any, Iterator, Union
 
 from git.types import Commit_ish, PathLike
@@ -131,14 +133,14 @@ def __init__(self, repo: 'Repo', binsha: bytes,
         if url is not None:
             self._url = url
         if branch_path is not None:
-            assert isinstance(branch_path, str)
+            # assert isinstance(branch_path, str)
             self._branch_path = branch_path
         if name is not None:
             self._name = name
 
     def _set_cache_(self, attr: str) -> None:
         if attr in ('path', '_url', '_branch_path'):
-            reader = self.config_reader()
+            reader: SectionConstraint = self.config_reader()
             # default submodule values
             try:
                 self.path = reader.get('path')
@@ -807,7 +809,8 @@ def move(self, module_path, configuration=True, module=True):
         return self
 
     @unbare_repo
-    def remove(self, module=True, force=False, configuration=True, dry_run=False):
+    def remove(self, module: bool = True, force: bool = False,
+               configuration: bool = True, dry_run: bool = False) -> 'Submodule':
         """Remove this submodule from the repository. This will remove our entry
         from the .gitmodules file and the entry in the .git / config file.
 
@@ -861,7 +864,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
                 # TODO: If we run into permission problems, we have a highly inconsistent
                 # state. Delete the .git folders last, start with the submodules first
                 mp = self.abspath
-                method = None
+                method: Union[None, Callable[[PathLike], None]] = None
                 if osp.islink(mp):
                     method = os.remove
                 elif osp.isdir(mp):
@@ -928,7 +931,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
                     rmtree(git_dir)
                 except Exception as ex:
                     if HIDE_WINDOWS_KNOWN_ERRORS:
-                        raise SkipTest("FIXME: fails with: PermissionError\n  %s", ex) from ex
+                        raise SkipTest(f"FIXME: fails with: PermissionError\n  {ex}") from ex
                     else:
                         raise
             # end handle separate bare repository
@@ -961,7 +964,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
 
         return self
 
-    def set_parent_commit(self, commit: Union[Commit_ish, None], check=True):
+    def set_parent_commit(self, commit: Union[Commit_ish, None], check: bool = True) -> 'Submodule':
         """Set this instance to use the given commit whose tree is supposed to
         contain the .gitmodules blob.
 
@@ -1009,7 +1012,7 @@ def set_parent_commit(self, commit: Union[Commit_ish, None], check=True):
         return self
 
     @unbare_repo
-    def config_writer(self, index=None, write=True):
+    def config_writer(self, index: Union[IndexFile, None] = None, write: bool = True) -> SectionConstraint:
         """:return: a config writer instance allowing you to read and write the data
             belonging to this submodule into the .gitmodules file.
 
@@ -1030,7 +1033,7 @@ def config_writer(self, index=None, write=True):
         return writer
 
     @unbare_repo
-    def rename(self, new_name):
+    def rename(self, new_name: str) -> 'Submodule':
         """Rename this submodule
         :note: This method takes care of renaming the submodule in various places, such as
 
@@ -1081,7 +1084,7 @@ def rename(self, new_name):
     #{ Query Interface
 
     @unbare_repo
-    def module(self):
+    def module(self) -> 'Repo':
         """:return: Repo instance initialized from the repository at our submodule path
         :raise InvalidGitRepositoryError: if a repository was not available. This could
             also mean that it was not yet initialized"""
@@ -1098,7 +1101,7 @@ def module(self):
             raise InvalidGitRepositoryError("Repository at %r was not yet checked out" % module_checkout_abspath)
         # END handle exceptions
 
-    def module_exists(self):
+    def module_exists(self) -> bool:
         """:return: True if our module exists and is a valid git repository. See module() method"""
         try:
             self.module()
@@ -1107,7 +1110,7 @@ def module_exists(self):
             return False
         # END handle exception
 
-    def exists(self):
+    def exists(self) -> bool:
         """
         :return: True if the submodule exists, False otherwise. Please note that
             a submodule may exist ( in the .gitmodules file) even though its module
diff --git a/git/objects/submodule/util.py b/git/objects/submodule/util.py
@@ -5,11 +5,18 @@
 import weakref
 
 
+# typing -----------------------------------------------------------------------
+
 from typing import Any, TYPE_CHECKING, Union
 
+from git.types import PathLike
+
 if TYPE_CHECKING:
     from .base import Submodule
     from weakref import ReferenceType
+    from git.repo import Repo
+    from git.refs import Head
+
 
 __all__ = ('sm_section', 'sm_name', 'mkhead', 'find_first_remote_branch',
            'SubmoduleConfigParser')
@@ -28,7 +35,7 @@ def sm_name(section):
     return section[11:-1]
 
 
-def mkhead(repo, path):
+def mkhead(repo: 'Repo', path: PathLike) -> 'Head':
     """:return: New branch/head instance"""
     return git.Head(repo, git.Head.to_full_path(path))
 
diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py
@@ -1,3 +1,4 @@
+from git.types import PathLike
 import os
 
 from git.compat import defenc
@@ -408,7 +409,7 @@ def log_entry(self, index):
         return RefLog.entry_at(RefLog.path(self), index)
 
     @classmethod
-    def to_full_path(cls, path):
+    def to_full_path(cls, path) -> PathLike:
         """
         :return: string with a full repository-relative path which can be used to initialize
             a Reference instance, for instance by using ``Reference.from_path``"""
diff --git a/git/repo/base.py b/git/repo/base.py
@@ -36,13 +36,15 @@
 
 # typing ------------------------------------------------------
 
-from git.types import TBD, PathLike, Lit_config_levels, Commit_ish, Tree_ish
+from git.types import ConfigLevels_NT, TBD, PathLike, Lit_config_levels, Commit_ish, Tree_ish
 from typing import (Any, BinaryIO, Callable, Dict,
                     Iterator, List, Mapping, Optional, Sequence,
                     TextIO, Tuple, Type, Union,
                     NamedTuple, cast, TYPE_CHECKING)
 
-if TYPE_CHECKING:  # only needed for types
+from git.types import ConfigLevels_Tup
+
+if TYPE_CHECKING:
     from git.util import IterableList
     from git.refs.symbolic import SymbolicReference
     from git.objects import Tree
@@ -55,12 +57,11 @@
 __all__ = ('Repo',)
 
 
-BlameEntry = NamedTuple('BlameEntry', [
-    ('commit', Dict[str, TBD]),
-    ('linenos', range),
-    ('orig_path', Optional[str]),
-    ('orig_linenos', range)]
-)
+class BlameEntry(NamedTuple):
+    commit: Dict[str, TBD]
+    linenos: range
+    orig_path: Optional[str]
+    orig_linenos: range
 
 
 class Repo(object):
@@ -95,7 +96,7 @@ class Repo(object):
 
     # invariants
     # represents the configuration level of a configuration file
-    config_level = ("system", "user", "global", "repository")  # type: Tuple[Lit_config_levels, ...]
+    config_level: ConfigLevels_Tup = ConfigLevels_NT("system", "user", "global", "repository")
 
     # Subclass configuration
     # Subclasses may easily bring in their own custom types by placing a constructor or type here
diff --git a/git/types.py b/git/types.py
@@ -4,7 +4,8 @@
 
 import os
 import sys
-from typing import Dict, Union, Any, TYPE_CHECKING
+from typing import (Callable, Dict, NoReturn, Tuple, Union, Any, Iterator,       # noqa: F401
+                    NamedTuple, TYPE_CHECKING, get_args, TypeVar)       # noqa: F401
 
 
 if sys.version_info[:2] >= (3, 8):
@@ -35,6 +36,32 @@
 
 Lit_config_levels = Literal['system', 'global', 'user', 'repository']
 
+T = TypeVar('T', bound=Literal['system', 'global', 'user', 'repository'], covariant=True)
+
+
+class ConfigLevels_NT(NamedTuple):
+    """NamedTuple of allowed CONFIG_LEVELS"""
+    # works for pylance, but not mypy
+    system: Literal['system']
+    user: Literal['user']
+    global_: Literal['global']
+    repository: Literal['repository']
+
+
+ConfigLevels_Tup = Tuple[Lit_config_levels, Lit_config_levels, Lit_config_levels, Lit_config_levels]
+# Typing this as specific literals breaks for mypy
+
+
+def is_config_level(inp: str) -> TypeGuard[Lit_config_levels]:
+    return inp in get_args(Lit_config_levels)
+
+
+def assert_never(inp: NoReturn, exc: Union[Exception, None] = None) -> NoReturn:
+    if exc is None:
+        assert False, f"An unhandled Literal ({inp}) in an if else chain was found"
+    else:
+        raise exc
+
 
 class Files_TD(TypedDict):
     insertions: int
diff --git a/output.txt b/output.txt
