Add types to submodule.update()

4 files changed, 52 insertions, 32 deletions

diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
index 7cd4356e..c975c0f5 100644
--- a/git/objects/submodule/base.py
+++ b/git/objects/submodule/base.py
@@ -49,10 +49,10 @@ from .util import (
 
 
 # typing ----------------------------------------------------------------------
-from typing import Callable, Dict, TYPE_CHECKING
+from typing import Callable, Dict, Mapping, Sequence, TYPE_CHECKING
 from typing import Any, Iterator, Union
 
-from git.types import Commit_ish, PathLike
+from git.types import Commit_ish, PathLike, TBD
 
 if TYPE_CHECKING:
     from git.repo import Repo
@@ -227,7 +227,7 @@ class Submodule(IndexObject, TraversableIterableObj):
 
         return SubmoduleConfigParser(fp_module, read_only=read_only)
 
-    def _clear_cache(self):
+    def _clear_cache(self) -> None:
         # clear the possibly changed values
         for name in self._cache_attrs:
             try:
@@ -247,7 +247,7 @@ class Submodule(IndexObject, TraversableIterableObj):
     def _config_parser_constrained(self, read_only: bool) -> SectionConstraint:
         """:return: Config Parser constrained to our submodule in read or write mode"""
         try:
-            pc = self.parent_commit
+            pc: Union['Commit_ish', None] = self.parent_commit
         except ValueError:
             pc = None
         # end handle empty parent repository
@@ -256,10 +256,12 @@ class Submodule(IndexObject, TraversableIterableObj):
         return SectionConstraint(parser, sm_section(self.name))
 
     @classmethod
-    def _module_abspath(cls, parent_repo, path, name):
+    def _module_abspath(cls, parent_repo: 'Repo', path: PathLike, name: str) -> PathLike:
         if cls._need_gitfile_submodules(parent_repo.git):
             return osp.join(parent_repo.git_dir, 'modules', name)
-        return osp.join(parent_repo.working_tree_dir, path)
+        if parent_repo.working_tree_dir:
+            return osp.join(parent_repo.working_tree_dir, path)
+        raise NotADirectoryError()
         # end
 
     @classmethod
@@ -287,7 +289,7 @@ class Submodule(IndexObject, TraversableIterableObj):
         return clone
 
     @classmethod
-    def _to_relative_path(cls, parent_repo, path):
+    def _to_relative_path(cls, parent_repo: 'Repo', path: PathLike) -> PathLike:
         """:return: a path guaranteed  to be relative to the given parent - repository
         :raise ValueError: if path is not contained in the parent repository's working tree"""
         path = to_native_path_linux(path)
@@ -295,7 +297,7 @@ class Submodule(IndexObject, TraversableIterableObj):
             path = path[:-1]
         # END handle trailing slash
 
-        if osp.isabs(path):
+        if osp.isabs(path) and parent_repo.working_tree_dir:
             working_tree_linux = to_native_path_linux(parent_repo.working_tree_dir)
             if not path.startswith(working_tree_linux):
                 raise ValueError("Submodule checkout path '%s' needs to be within the parents repository at '%s'"
@@ -309,7 +311,7 @@ class Submodule(IndexObject, TraversableIterableObj):
         return path
 
     @classmethod
-    def _write_git_file_and_module_config(cls, working_tree_dir, module_abspath):
+    def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_abspath: PathLike) -> None:
         """Writes a .git file containing a(preferably) relative path to the actual git module repository.
         It is an error if the module_abspath cannot be made into a relative path, relative to the working_tree_dir
         :note: will overwrite existing files !
@@ -336,7 +338,8 @@ class Submodule(IndexObject, TraversableIterableObj):
 
     @classmethod
     def add(cls, repo: 'Repo', name: str, path: PathLike, url: Union[str, None] = None,
-            branch=None, no_checkout: bool = False, depth=None, env=None, clone_multi_options=None
+            branch: Union[str, None] = None, no_checkout: bool = False, depth: Union[int, None] = None,
+            env: Mapping[str, str] = None, clone_multi_options: Union[Sequence[TBD], None] = None
             ) -> 'Submodule':
         """Add a new submodule to the given repository. This will alter the index
         as well as the .gitmodules file, but will not create a new commit.
@@ -415,7 +418,8 @@ class Submodule(IndexObject, TraversableIterableObj):
             # END check url
         # END verify urls match
 
-        mrepo = None
+        # mrepo: Union[Repo, None] = None
+
         if url is None:
             if not has_module:
                 raise ValueError("A URL was not given and a repository did not exist at %s" % path)
@@ -428,7 +432,7 @@ class Submodule(IndexObject, TraversableIterableObj):
             url = urls[0]
         else:
             # clone new repo
-            kwargs: Dict[str, Union[bool, int]] = {'n': no_checkout}
+            kwargs: Dict[str, Union[bool, int, Sequence[TBD]]] = {'n': no_checkout}
             if not branch_is_default:
                 kwargs['b'] = br.name
             # END setup checkout-branch
@@ -452,6 +456,8 @@ class Submodule(IndexObject, TraversableIterableObj):
         # otherwise there is a '-' character in front of the submodule listing
         #  a38efa84daef914e4de58d1905a500d8d14aaf45 mymodule (v0.9.0-1-ga38efa8)
         # -a38efa84daef914e4de58d1905a500d8d14aaf45 submodules/intermediate/one
+        writer: Union[GitConfigParser, SectionConstraint]
+
         with sm.repo.config_writer() as writer:
             writer.set_value(sm_section(name), 'url', url)
 
@@ -473,8 +479,10 @@ class Submodule(IndexObject, TraversableIterableObj):
 
         return sm
 
-    def update(self, recursive=False, init=True, to_latest_revision=False, progress=None, dry_run=False,
-               force=False, keep_going=False, env=None, clone_multi_options=None):
+    def update(self, recursive: bool = False, init: bool = True, to_latest_revision: bool = False,
+               progress: Union['UpdateProgress', None] = None, dry_run: bool = False,
+               force: bool = False, keep_going: bool = False, env: Mapping[str, str] = None,
+               clone_multi_options: Union[Sequence[TBD], None] = None):
         """Update the repository of this submodule to point to the checkout
         we point at with the binsha of this instance.
 
@@ -581,6 +589,7 @@ class Submodule(IndexObject, TraversableIterableObj):
                 if not dry_run:
                     # see whether we have a valid branch to checkout
                     try:
+                        assert isinstance(mrepo, Repo)
                         # find  a remote which has our branch - we try to be flexible
                         remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name)
                         local_branch = mkhead(mrepo, self.branch_path)
@@ -641,7 +650,7 @@ class Submodule(IndexObject, TraversableIterableObj):
                 may_reset = True
                 if mrepo.head.commit.binsha != self.NULL_BIN_SHA:
                     base_commit = mrepo.merge_base(mrepo.head.commit, hexsha)
-                    if len(base_commit) == 0 or base_commit[0].hexsha == hexsha:
+                    if len(base_commit) == 0 or (base_commit[0] is not None and base_commit[0].hexsha == hexsha):
                         if force:
                             msg = "Will force checkout or reset on local branch that is possibly in the future of"
                             msg += "the commit it will be checked out to, effectively 'forgetting' new commits"
@@ -916,7 +925,7 @@ class Submodule(IndexObject, TraversableIterableObj):
                     import gc
                     gc.collect()
                     try:
-                        rmtree(wtd)
+                        rmtree(str(wtd))
                     except Exception as ex:
                         if HIDE_WINDOWS_KNOWN_ERRORS:
                             raise SkipTest("FIXME: fails with: PermissionError\n  {}".format(ex)) from ex
@@ -954,6 +963,8 @@ class Submodule(IndexObject, TraversableIterableObj):
 
             # now git config - need the config intact, otherwise we can't query
             # information anymore
+            writer: Union[GitConfigParser, SectionConstraint]
+
             with self.repo.config_writer() as writer:
                 writer.remove_section(sm_section(self.name))
 
@@ -1067,13 +1078,14 @@ class Submodule(IndexObject, TraversableIterableObj):
             destination_module_abspath = self._module_abspath(self.repo, self.path, new_name)
             source_dir = mod.git_dir
             # Let's be sure the submodule name is not so obviously tied to a directory
-            if destination_module_abspath.startswith(mod.git_dir):
+            if str(destination_module_abspath).startswith(str(mod.git_dir)):
                 tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4()))
                 os.renames(source_dir, tmp_dir)
                 source_dir = tmp_dir
             # end handle self-containment
             os.renames(source_dir, destination_module_abspath)
-            self._write_git_file_and_module_config(mod.working_tree_dir, destination_module_abspath)
+            if mod.working_tree_dir:
+                self._write_git_file_and_module_config(mod.working_tree_dir, destination_module_abspath)
         # end move separate git repository
 
         return self
@@ -1150,26 +1162,26 @@ class Submodule(IndexObject, TraversableIterableObj):
         return mkhead(self.module(), self._branch_path)
 
     @property
-    def branch_path(self):
+    def branch_path(self) -> PathLike:
         """
         :return: full(relative) path as string to the branch we would checkout
             from the remote and track"""
         return self._branch_path
 
     @property
-    def branch_name(self):
+    def branch_name(self) -> str:
         """:return: the name of the branch, which is the shortest possible branch name"""
         # use an instance method, for this we create a temporary Head instance
         # which uses a repository that is available at least ( it makes no difference )
         return git.Head(self.repo, self._branch_path).name
 
     @property
-    def url(self):
+    def url(self) -> str:
         """:return: The url to the repository which our module - repository refers to"""
         return self._url
 
     @property
-    def parent_commit(self):
+    def parent_commit(self) -> 'Commit_ish':
         """:return: Commit instance with the tree containing the .gitmodules file
         :note: will always point to the current head's commit if it was not set explicitly"""
         if self._parent_commit is None:
@@ -1177,7 +1189,7 @@ class Submodule(IndexObject, TraversableIterableObj):
         return self._parent_commit
 
     @property
-    def name(self):
+    def name(self) -> str:
         """:return: The name of this submodule. It is used to identify it within the
             .gitmodules file.
         :note: by default, the name is the path at which to find the submodule, but
diff --git a/git/remote.py b/git/remote.py
index 0ef54ea7..739424ee 100644
--- a/git/remote.py
+++ b/git/remote.py
@@ -42,6 +42,7 @@ from git.types import PathLike, Literal, TBD, TypeGuard, Commit_ish
 
 if TYPE_CHECKING:
     from git.repo.base import Repo
+    from git.objects.submodule.base import UpdateProgress
     # from git.objects.commit import Commit
     # from git.objects import Blob, Tree, TagObject
 
@@ -64,7 +65,9 @@ __all__ = ('RemoteProgress', 'PushInfo', 'FetchInfo', 'Remote')
 #{ Utilities
 
 
-def add_progress(kwargs: Any, git: Git, progress: Union[Callable[..., Any], None]) -> Any:
+def add_progress(kwargs: Any, git: Git,
+                 progress: Union[RemoteProgress, 'UpdateProgress', Callable[..., RemoteProgress], None]
+                 ) -> Any:
     """Add the --progress flag to the given kwargs dict if supported by the
     git command. If the actual progress in the given progress instance is not
     given, we do not request any progress
@@ -794,7 +797,7 @@ class Remote(LazyMixin, IterableObj):
             config.release()
 
     def fetch(self, refspec: Union[str, List[str], None] = None,
-              progress: Union[Callable[..., Any], None] = None,
+              progress: Union[RemoteProgress, None, 'UpdateProgress'] = None,
               verbose: bool = True, **kwargs: Any) -> IterableList[FetchInfo]:
         """Fetch the latest changes for this remote
 
@@ -841,7 +844,7 @@ class Remote(LazyMixin, IterableObj):
         return res
 
     def pull(self, refspec: Union[str, List[str], None] = None,
-             progress: Union[Callable[..., Any], None] = None,
+             progress: Union[RemoteProgress, 'UpdateProgress', None] = None,
              **kwargs: Any) -> IterableList[FetchInfo]:
         """Pull changes from the given branch, being the same as a fetch followed
         by a merge of branch with your local branch.
@@ -862,7 +865,7 @@ class Remote(LazyMixin, IterableObj):
         return res
 
     def push(self, refspec: Union[str, List[str], None] = None,
-             progress: Union[Callable[..., Any], None] = None,
+             progress: Union[RemoteProgress, 'UpdateProgress', Callable[..., RemoteProgress], None] = None,
              **kwargs: Any) -> IterableList[PushInfo]:
         """Push changes from source branch in refspec to target branch in refspec.
 
diff --git a/git/repo/base.py b/git/repo/base.py
index e1b1fc76..ea86139b 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -48,6 +48,8 @@ if TYPE_CHECKING:
     from git.util import IterableList
     from git.refs.symbolic import SymbolicReference
     from git.objects import Tree
+    from git.objects.submodule.base import UpdateProgress
+    from git.remote import RemoteProgress
 
 
 # -----------------------------------------------------------
@@ -575,7 +577,7 @@ class Repo(object):
         return Commit.iter_items(self, rev, paths, **kwargs)
 
     def merge_base(self, *rev: TBD, **kwargs: Any
-                   ) -> List[Union['SymbolicReference', Commit_ish, None]]:
+                   ) -> List[Union[Commit_ish, None]]:
         """Find the closest common ancestor for the given revision (e.g. Commits, Tags, References, etc)
 
         :param rev: At least two revs to find the common ancestor for.
@@ -588,7 +590,7 @@ class Repo(object):
             raise ValueError("Please specify at least two revs, got only %i" % len(rev))
         # end handle input
 
-        res = []  # type: List[Union['SymbolicReference', Commit_ish, None]]
+        res = []  # type: List[Union[Commit_ish, None]]
         try:
             lines = self.git.merge_base(*rev, **kwargs).splitlines()  # List[str]
         except GitCommandError as err:
@@ -1014,7 +1016,8 @@ class Repo(object):
 
     @classmethod
     def _clone(cls, git: 'Git', url: PathLike, path: PathLike, odb_default_type: Type[GitCmdObjectDB],
-               progress: Optional[Callable], multi_options: Optional[List[str]] = None, **kwargs: Any
+               progress: Union['RemoteProgress', 'UpdateProgress', Callable[..., 'RemoteProgress'], None],
+               multi_options: Optional[List[str]] = None, **kwargs: Any
                ) -> 'Repo':
         odbt = kwargs.pop('odbt', odb_default_type)
 
diff --git a/git/util.py b/git/util.py
index abc82bd3..b13af358 100644
--- a/git/util.py
+++ b/git/util.py
@@ -82,13 +82,15 @@ HIDE_WINDOWS_FREEZE_ERRORS = is_win and os.environ.get('HIDE_WINDOWS_FREEZE_ERRO
 
 #{ Utility Methods
 
+T = TypeVar('T')
 
-def unbare_repo(func: Callable) -> Callable:
+
+def unbare_repo(func: Callable[..., T]) -> Callable[..., T]:
     """Methods with this decorator raise InvalidGitRepositoryError if they
     encounter a bare repository"""
 
     @wraps(func)
-    def wrapper(self: 'Remote', *args: Any, **kwargs: Any) -> Callable:
+    def wrapper(self: 'Remote', *args: Any, **kwargs: Any) -> T:
         if self.repo.bare:
             raise InvalidGitRepositoryError("Method '%s' cannot operate on bare repositories" % func.__name__)
         # END bare method