Skip to content

Commit

Permalink
Don't return with operand when conceptually void
Browse files Browse the repository at this point in the history
This changes `return None` to `return` where it appears in
functions that are "conceptually void," i.e. cases where a function
always returns None, its purpose is never to provide a return value
to the caller, and it would be a bug (or at least confusing style)
to use the return value or have the call as a subexpression.

This is already the prevailing style, so this is a consistency
improvement, as well as improving clarity by avoiding giving the
impression that "None" is significant in those cases.

Of course, functions that sometimes return values other than None
do not have "return None" replaced with a bare "return" statement.
Those are left alone (when they return None, it means something).

For the most part this is straightforward, but:

- The handle_stderr local function in IndexFile.checkout is also
  slightly further refactored to better express what the early
  return is doing.

- The "None" operand is removed as usual in GitConfigParser.read,
  even though a superclass has a same-named method with a return
  type that is not None. Usually when this happens, the superclass
  method returns something like Optional[Something], and it is a
  case of return type covariance, in which case the None operands
  should still be written. Here, however, the overriding method
  does not intend to be an override: it has an incompatible
  signature and cannot be called as if it were the superclass
  method, nor is its return type compatible.

- The "None" operand is *retained* in git.cmd.handle_process_output
  even though the return type is annotated as None, because the
  function also returns the expression `finalizer(process)`. The
  argument `finalizer` is itself annotated to return None, but it
  looks like the intent may be to play along with the strange case
  that `finalizer` returns a non-None value, and forward it.
  • Loading branch information
EliahKagan committed Dec 3, 2023
1 parent b8ee9be commit 39e2dff
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 15 deletions.
4 changes: 2 additions & 2 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,13 @@ def _terminate(self) -> None:
try:
if proc.poll() is not None:
self.status = self._status_code_if_terminate or proc.poll()
return None
return
except OSError as ex:
log.info("Ignored error after process had died: %r", ex)

# It can be that nothing really exists anymore...
if os is None or getattr(os, "kill", None) is None:
return None
return

# Try to kill it.
try:
Expand Down
9 changes: 5 additions & 4 deletions git/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ def __setitem__(self, key: str, value: _T) -> None:
def add(self, key: str, value: Any) -> None:
if key not in self:
super().__setitem__(key, [value])
return None
return

super().__getitem__(key).append(value)

def setall(self, key: str, values: List[_T]) -> None:
Expand Down Expand Up @@ -579,7 +580,7 @@ def read(self) -> None: # type: ignore[override]
:raise IOError: If a file cannot be handled
"""
if self._is_initialized:
return None
return
self._is_initialized = True

files_to_read: List[Union[PathLike, IO]] = [""]
Expand Down Expand Up @@ -697,7 +698,7 @@ def write(self) -> None:
a file lock"""
self._assure_writable("write")
if not self._dirty:
return None
return

if isinstance(self._file_or_files, (list, tuple)):
raise AssertionError(
Expand All @@ -711,7 +712,7 @@ def write(self) -> None:
"Skipping write-back of configuration file as include files were merged in."
+ "Set merge_includes=False to prevent this."
)
return None
return
# END stop if we have include files

fp = self._file_or_files
Expand Down
8 changes: 4 additions & 4 deletions git/index/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def _set_cache_(self, attr: str) -> None:
except OSError:
# In new repositories, there may be no index, which means we are empty.
self.entries: Dict[Tuple[PathLike, StageType], IndexEntry] = {}
return None
return
# END exception handling

try:
Expand Down Expand Up @@ -1210,9 +1210,9 @@ def checkout(
def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLike]) -> None:
stderr_IO = proc.stderr
if not stderr_IO:
return None # Return early if stderr empty.
else:
stderr_bytes = stderr_IO.read()
return # Return early if stderr empty.

stderr_bytes = stderr_IO.read()
# line contents:
stderr = stderr_bytes.decode(defenc)
# git-checkout-index: this already exists
Expand Down
2 changes: 1 addition & 1 deletion git/index/fun.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
"""
hp = hook_path(name, index.repo.git_dir)
if not os.access(hp, os.X_OK):
return None
return

env = os.environ.copy()
env["GIT_INDEX_FILE"] = safe_decode(str(index.path))
Expand Down
2 changes: 1 addition & 1 deletion git/objects/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def git_cmp(t1: TreeCacheTup, t2: TreeCacheTup) -> int:

def merge_sort(a: List[TreeCacheTup], cmp: Callable[[TreeCacheTup, TreeCacheTup], int]) -> None:
if len(a) < 2:
return None
return

mid = len(a) // 2
lefthalf = a[:mid]
Expand Down
4 changes: 2 additions & 2 deletions git/objects/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,8 @@ def addToStack(
depth: int,
) -> None:
lst = self._get_intermediate_items(item)
if not lst: # empty list
return None
if not lst: # Empty list
return
if branch_first:
stack.extendleft(TraverseNT(depth, i, src_item) for i in lst)
else:
Expand Down
2 changes: 1 addition & 1 deletion git/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ def _parse_progress_line(self, line: AnyStr) -> None:
self.line_dropped(line_str)
# Note: Don't add this line to the other lines, as we have to silently
# drop it.
return None
return
# END handle op code

# Figure out stage.
Expand Down

0 comments on commit 39e2dff

Please sign in to comment.