Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork937
Description
RemoteProgress._parse_progress_line is conceptually a "protected" method: code outside GitPython may subclassRemoteProgress and override it with another implementation (which may also reasonably call the base class implementation).
The method isconceptually void as implemented in the base class, always implicitly returningNone, and it is annotated accordingly:
Line 582 inafa5754
| def_parse_progress_line(self,line:AnyStr)->None: |
However, while it would not ordinarily be useful to do so, it is--orhas 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 inafa5754
| defnew_message_handler(self)->Callable[[str],None]: | |
| """ | |
| :return: | |
| A progress handler suitable for handle_process_output(), passing lines on to | |
| this Progress handler in a suitable format | |
| """ | |
| defhandler(line:AnyStr)->None: | |
| returnself._parse_progress_line(line.rstrip()) | |
| # END handler | |
| returnhandler |
And in the subclass ofRemoteProgress introduced in the test suite, which overrides the method:
Lines 54 to 59 inafa5754
| def_parse_progress_line(self,line): | |
| # We may remove the line later if it is dropped. | |
| # Keep it for debugging. | |
| self._seen_lines.append(line) | |
| rval=super()._parse_progress_line(line) | |
| returnrval |
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 asmypy andpyright will report a non-None return type as a violation.
If possible, the return type inRemoteProgress 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.