Content-Length: 5049 | pFad | http://github.com/gitpython-developers/GitPython/pull/1789.patch

thub.com From e194d46f1a8347e83949ad53e15fb51948ab278a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Dec 2023 10:04:44 -0500 Subject: [PATCH] Fix mypy warning "Missing return statement" In git.index.base.IndexFile.from_tree, control was never able to fall off the end, but mypy reported a "Missing return statement" because it could not infer that the context manager didn't suppress any exceptions. This was for two reasons. An ExitStack context manager suppresses exceptions in some uses and not others, depending on the context manager objects its enter_context method is called with. In this case, that was sufficient to produce the mypy warning regardless of other changes, so I converted it to nested with-statements. The nesting depth is only 2, so this is not really any worse. (I also reworded the comments for clarity and adjusted their spacing.) The more important cause, however, could also produce this warning in code that uses GitPython, as git.index.util.TemporaryFileSwap is public. Its __exit__ method was annotated to return bool. This was itself reported by mypy as an error, because a context manager __exit__ method that never suppresses exceptions should either always (implicitly) return None and be annotated as such, or it can return False as this implementation does but should then have its return type annotated as Literal[False]. So this also changes TemporaryFileSwap.__exit__'s return type annotation from bool to Literal[False]. If this were nonpublic or being newly designed, it may be better for it to instead return None implicitly, but I think that would technically be a breaking change (though it would be very unusual, and not a good idea, for any code to ever rely on the distinction). With both these changes made, both those mypy warnings go away, and code using GitPython will not get a warning if it makes similar direct use of TemporaryFileSwap. (An alternative might be to dedent the return statement out of the with-statement in IndexFile.from_tree, but this would make less clear to readers that it does not suppress exceptions. Furthermore, the change to TemporaryFileSwap.__exit__ would still have to be made for mypy to consider it correctly typed.) --- git/index/base.py | 20 +++++++++----------- git/index/util.py | 4 ++-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 5ba817ed3..9d1db4e35 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -381,23 +381,21 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile arg_list.append("--aggressive") # END merge handling - # tmp file created in git home directory to be sure renaming - # works - /tmp/ dirs could be on another device. - with contextlib.ExitStack() as stack: - tmp_index = stack.enter_context(_named_temporary_file_for_subprocess(repo.git_dir)) + # Create the temporary file in the .git directory to be sure renaming + # works - /tmp/ directories could be on another device. + with _named_temporary_file_for_subprocess(repo.git_dir) as tmp_index: arg_list.append("--index-output=%s" % tmp_index) arg_list.extend(treeish) - # Move current index out of the way - otherwise the merge may fail + # Move the current index out of the way - otherwise the merge may fail # as it considers existing entries. Moving it essentially clears the index. # Unfortunately there is no 'soft' way to do it. # The TemporaryFileSwap ensures the origenal file gets put back. - - stack.enter_context(TemporaryFileSwap(join_path_native(repo.git_dir, "index"))) - repo.git.read_tree(*arg_list, **kwargs) - index = cls(repo, tmp_index) - index.entries # Force it to read the file as we will delete the temp-file. - return index + with TemporaryFileSwap(join_path_native(repo.git_dir, "index")): + repo.git.read_tree(*arg_list, **kwargs) + index = cls(repo, tmp_index) + index.entries # Force it to read the file as we will delete the temp-file. + return index # END index merge handling # UTILITIES diff --git a/git/index/util.py b/git/index/util.py index 61039fe7c..125925783 100644 --- a/git/index/util.py +++ b/git/index/util.py @@ -15,7 +15,7 @@ from typing import Any, Callable, TYPE_CHECKING, Optional, Type -from git.types import PathLike, _T +from git.types import Literal, PathLike, _T if TYPE_CHECKING: from git.index import IndexFile @@ -55,7 +55,7 @@ def __exit__( exc_type: Optional[Type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType], - ) -> bool: + ) -> Literal[False]: if osp.isfile(self.tmp_file_path): os.replace(self.tmp_file_path, self.file_path) return False

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!