Description
RemoteProgress._parse_progress_line
is conceptually a "protected" method: code outside GitPython may subclass RemoteProgress
and override it with another implementation (which may also reasonably call the base class implementation).
The method is conceptually void as implemented in the base class, always implicitly returning None
, and it is annotated accordingly:
Line 582 in afa5754
However, while it would not ordinarily be useful to do so, it is--or has been--also intended that it be overridable to return a non-None
value. This is mostly for compatibility with existing code that may have done so in the past, but it may be that reasonable cases for doing so in new code can still arise. This is reflected in code of GitPython:
Lines 685 to 697 in afa5754
And in the subclass of RemoteProgress
introduced in the test suite, which overrides the method:
Lines 54 to 59 in afa5754
But any override that returns a non-None
value is currently inconsistent with the return type annotation on the base class, which is written as -> None
. For Liskov substitutability, an overridden method's return type must be the same or a subclass of the base method's return type, and if a subclass has type annotations, and the return type is specified as a non-Any
type, type checkers such as mypy
and pyright
will report a non-None
return type as a violation.
If possible, the return type in RemoteProgress
should be broadened, but in a way that does not unduly encourage subclasses to return non-None
. See comments on #1788, starting at #1788 (comment), for a discussion and analysis of this. This issue, and the above recommendation, is based on the findings and discussion there.