Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mypy warning "Missing return statement" #1789

Merged
merged 1 commit into from
Dec 26, 2023

Commits on Dec 25, 2023

  1. 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.)
    EliahKagan committed Dec 25, 2023
    Configuration menu
    Copy the full SHA
    e194d46 View commit details
    Browse the repository at this point in the history