typing: fix various errors caught by pyrefly

These should be fairly straightforward and not require much thought.
I've punted the more complicated typing changes.

Change-Id: I79167f07e0ec259586520104f07e4cee096a382e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/656663
Tested-by: Mike Frysinger <vapier@google.com>
Commit-Queue: Mike Frysinger <vapier@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>

color.py

@@ -82,7 +82,7 @@ def _Color(fg=None, bg=None, attr=None):
     return code
 
 
-DEFAULT = None
+DEFAULT: Optional[str] = None
 
 
 class _CheckConsoleSentinel:

command.py

@@ -43,7 +43,7 @@ WORKER_BATCH_SIZE = 32
 
 # How many jobs to run in parallel by default?  This assumes the jobs are
 # largely I/O bound and do not hit the network.
-DEFAULT_LOCAL_JOBS = min(os.cpu_count(), 8)
+DEFAULT_LOCAL_JOBS = min(os.cpu_count() or 1, 8)
 
 
 class UsageError(error.RepoExitError):
@@ -105,7 +105,8 @@ class Command:
         # Cache for the OptionParser property.
         self._optparse = None
 
-    def WantPager(self, _opt):
+    def WantPager(self, opt) -> bool:
+        del opt  # Unused.
         return False
 
     @staticmethod
@@ -623,7 +624,8 @@ class InteractiveCommand(Command):
     within a pager, even if the user asks to.
     """
 
-    def WantPager(self, _opt):
+    def WantPager(self, opt) -> bool:
+        del opt  # Unused.
         return False
 
 
@@ -632,7 +634,8 @@ class PagedCommand(Command):
     larger than one screen full.
     """
 
-    def WantPager(self, _opt):
+    def WantPager(self, opt) -> bool:
+        del opt  # Unused.
         return True
 
 

error.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import List
+from typing import Optional, Sequence
 
 
 class BaseRepoError(Exception):
@@ -22,7 +22,7 @@ class BaseRepoError(Exception):
 class RepoError(BaseRepoError):
     """Exceptions thrown inside repo that can be handled."""
 
-    def __init__(self, *args, project: str = None) -> None:
+    def __init__(self, *args, project: Optional[str] = None) -> None:
         super().__init__(*args)
         self.project = project
 
@@ -36,7 +36,7 @@ class RepoExitError(BaseRepoError):
         self,
         *args,
         exit_code: int = 1,
-        aggregate_errors: List[Exception] = None,
+        aggregate_errors: Optional[Sequence[Exception]] = None,
         **kwargs,
     ) -> None:
         super().__init__(*args, **kwargs)

git_command.py

@@ -18,7 +18,7 @@ import os
 import re
 import subprocess
 import sys
-from typing import Any, Optional
+from typing import Any, Dict, Optional
 
 import error
 import git_trace2_event_log_base
@@ -220,7 +220,7 @@ def git_require(min_version, fail=False, msg=""):
     return False
 
 
-def _get_basic_env() -> None:
+def _get_basic_env() -> Dict[str, str]:
     """Return a basic env for running git under.
 
     This is guaranteed to be side-effect free.

git_superproject.py

@@ -29,7 +29,7 @@ import os
 import sys
 import tempfile
 import time
-from typing import NamedTuple
+from typing import NamedTuple, Optional
 import urllib.parse
 
 import git_command
@@ -54,7 +54,7 @@ class CommitIdsResult(NamedTuple):
     """Return the commit ids and whether caller should exit."""
 
     # A dictionary with the projects/commit ids on success, otherwise None.
-    commit_ids: dict
+    commit_ids: Optional[dict]
     # Whether the caller should exit.
     fatal: bool
 
@@ -63,7 +63,7 @@ class UpdateProjectsResult(NamedTuple):
     """Return the overriding manifest file and whether caller should exit."""
 
     # Path name of the overriding manifest file if successful, otherwise None.
-    manifest_path: str
+    manifest_path: Optional[str]
     # Whether the caller should exit.
     fatal: bool
 

hooks.py

@@ -323,6 +323,7 @@ class RepoHook:
         """
         # Exec, storing global context in the context dict.  We catch exceptions
         # and convert to a HookError w/ just the failing traceback.
+        assert self._script_fullpath is not None
         try:
             exec(compile(data, self._script_fullpath, "exec"), context)
         except Exception:
@@ -367,6 +368,7 @@ class RepoHook:
             # Always run hooks with CWD as topdir.
             os.chdir(self._repo_topdir)
 
+            assert self._script_fullpath is not None
             # Put the hook dir as the first item of sys.path so hooks can do
             # relative imports.  We want to replace the repo dir as [0] so
             # hooks can't import repo files.
@@ -407,6 +409,7 @@ class RepoHook:
 
     def _CheckHook(self):
         # Bail with a nice error if we can't find the hook.
+        assert self._script_fullpath is not None
         if not os.path.isfile(self._script_fullpath):
             raise error.HookError(
                 "Couldn't find repo hook: %s" % self._script_fullpath

platform_utils.py

@@ -17,6 +17,7 @@ import os
 import platform
 import shutil
 import stat
+from typing import Union
 
 
 def isWindows():
@@ -224,7 +225,7 @@ def rmdir(path):
     os.rmdir(_makelongpath(path))
 
 
-def removedirs(path: str) -> None:
+def removedirs(path: Union[str, os.PathLike]) -> None:
     """Remove a directory tree of symlinks and empty directories.
 
     Walks |path| bottom-up without following symlinks.  Removes symlinks

repo

@@ -26,7 +26,7 @@ import platform
 import shlex
 import subprocess
 import sys
-from typing import NamedTuple, Tuple
+from typing import List, NamedTuple, Tuple
 
 
 # These should never be newer than the main.py version since this needs to be a
@@ -119,12 +119,10 @@ if __name__ == "__main__":
 
 # repo default configuration
 #
-REPO_URL = os.environ.get("REPO_URL", None)
-if not REPO_URL:
-    REPO_URL = "https://gerrit.googlesource.com/git-repo"
-REPO_REV = os.environ.get("REPO_REV")
-if not REPO_REV:
-    REPO_REV = "stable"
+REPO_URL = (
+    os.environ.get("REPO_URL") or "https://gerrit.googlesource.com/git-repo"
+)
+REPO_REV = os.environ.get("REPO_REV") or "stable"
 # URL to file bug reports for repo tool issues.
 BUG_URL = "https://issues.gerritcodereview.com/issues/new?component=1370071"
 
@@ -902,7 +900,7 @@ def _GetRepoConfig(name):
 
 
 def _InitHttp():
-    handlers = []
+    handlers: List[urllib.request.BaseHandler] = []
 
     mgr = urllib.request.HTTPPasswordMgrWithDefaultRealm()
     try:

repo_trace.py

@@ -25,6 +25,7 @@ import os
 import sys
 import tempfile
 import time
+from typing import Optional
 
 import platform_utils
 
@@ -35,7 +36,7 @@ REPO_TRACE = "REPO_TRACE"
 # Temporarily set tracing to always on unless user expicitly sets to 0.
 _TRACE = os.environ.get(REPO_TRACE) != "0"
 _TRACE_TO_STDERR = False
-_TRACE_FILE = None
+_TRACE_FILE: Optional[str] = None
 _TRACE_FILE_NAME = "TRACE_FILE"
 _MAX_SIZE = 70  # in MiB
 _NEW_COMMAND_SEP = "+++++++++++++++NEW COMMAND+++++++++++++++++++"
@@ -104,6 +105,7 @@ class Trace(contextlib.ContextDecorator):
             f"PID: {os.getpid()} START: {self._time()} :{self._trace_msg}\n"
         )
 
+        assert _TRACE_FILE is not None
         with open(_TRACE_FILE, "a") as f:
             print(print_msg, file=f)
 
@@ -120,6 +122,7 @@ class Trace(contextlib.ContextDecorator):
             f"PID: {os.getpid()} END: {self._time()} :{self._trace_msg}\n"
         )
 
+        assert _TRACE_FILE is not None
         with open(_TRACE_FILE, "a") as f:
             print(print_msg, file=f)
 
@@ -141,6 +144,7 @@ def _GetTraceFile(quiet):
 
 def _ClearOldTraces():
     """Clear the oldest commands if trace file is too big."""
+    assert _TRACE_FILE is not None
     try:
         with open(_TRACE_FILE, errors="ignore") as f:
             if os.path.getsize(f.name) / (1024 * 1024) <= _MAX_SIZE:

subcmds/checkout.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 import functools
-from typing import NamedTuple
+from typing import NamedTuple, Optional
 
 import command
 import error
@@ -26,9 +26,9 @@ logger = repo_logging.RepoLogger(__file__)
 
 class CheckoutBranchResult(NamedTuple):
     # Whether the Project is on the branch (i.e. branch exists and no errors)
-    result: bool
+    result: Optional[bool]
     project_idx: int
-    error: Exception
+    error: Optional[Exception]
 
 
 class CheckoutCommandError(error.RepoExitError):

subcmds/forall.py

@@ -195,7 +195,7 @@ without iterating through the remaining projects.
             "--interactive", action="store_true", help="force interactive usage"
         )
 
-    def WantPager(self, opt):
+    def WantPager(self, opt) -> bool:
         return opt.project_header and opt.jobs == 1
 
     def ValidateOptions(self, opt, args):

subcmds/grep.py

@@ -102,8 +102,9 @@ contain a line that matches both expressions:
         if value is not None:
             pt.append(value)
 
-    def _CommonOptions(self, p):
+    def _CommonOptions(self, p, opt_v=True):
         """Override common options slightly."""
+        del opt_v  # Unused.
         super()._CommonOptions(p, opt_v=False)
 
     def _Options(self, p):

subcmds/info.py

@@ -130,7 +130,7 @@ class Info(command.PagedCommand):
             help=f"output format: {', '.join(formats)} (default: %default)",
         )
 
-    def WantPager(self, opt):
+    def WantPager(self, opt) -> bool:
         return OutputFormat[opt.format.upper()] == OutputFormat.TEXT
 
     def ValidateOptions(self, opt, args):

subcmds/init.py

@@ -89,7 +89,7 @@ manifest, a subsequent `repo sync` (or `repo sync -d`) is necessary
 to update the working directory files.
 """
 
-    def _CommonOptions(self, p):
+    def _CommonOptions(self, p, opt_v=True):
         """Disable due to re-use of Wrapper()."""
 
     def _Options(self, p):

subcmds/start.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 import functools
-from typing import NamedTuple
+from typing import NamedTuple, Optional
 
 import command
 import error
@@ -28,7 +28,7 @@ logger = repo_logging.RepoLogger(__file__)
 
 class ExecuteOneResult(NamedTuple):
     project_idx: int
-    error: Exception
+    error: Optional[Exception]
 
 
 class StartError(error.RepoExitError):

tests/e2e/test_harness.py

@@ -28,6 +28,7 @@ def _http_request(
 ) -> Tuple[int, bytes]:
     """Send a request to `server` & return the response status & body."""
     url = urllib.parse.urlsplit(server.url)
+    assert url.hostname is not None
     conn = http.client.HTTPConnection(url.hostname, url.port, timeout=30)
     try:
         conn.request(method, path)
@@ -113,6 +114,7 @@ def test_http_post_body(
     if expect_continue:
         headers["Expect"] = "100-continue"
     url = urllib.parse.urlsplit(git_server.url)
+    assert url.hostname is not None
     conn = http.client.HTTPConnection(url.hostname, url.port, timeout=30)
     try:
         # http.client skips any interim `100 Continue` response.

tests/test_editor.py

@@ -14,13 +14,15 @@
 
 """Unittests for the editor.py module."""
 
+from typing import Iterator
+
 import pytest
 
 import editor
 
 
 @pytest.fixture(autouse=True)
-def reset_editor() -> None:
+def reset_editor() -> Iterator[None]:
     """Take care of resetting Editor state across tests."""
     editor.Editor._editor = None
     yield

tests/test_git_command.py

@@ -444,13 +444,13 @@ class GitRequireTests(unittest.TestCase):
         """Test fatal require calls with old versions."""
         with self.assertRaises(git_command.GitRequireError) as e:
             git_command.git_require((2,), fail=True)
-            self.assertNotEqual(0, e.code)
+        self.assertNotEqual(0, e.exception.exit_code)
 
     def test_older_fatal_msg(self):
         """Test fatal require calls with old versions and message."""
         with self.assertRaises(git_command.GitRequireError) as e:
             git_command.git_require((2,), fail=True, msg="so sad")
-            self.assertNotEqual(0, e.code)
+        self.assertNotEqual(0, e.exception.exit_code)
 
 
 class GitCommandErrorTest(unittest.TestCase):

tests/test_git_config.py

@@ -214,6 +214,9 @@ def test_get_sync_analysis_state_data(rw_config_file: Path) -> None:
     class Options:
         """Container for testing."""
 
+        verbose: str
+        mp_update: str
+
     options = Options()
     options.verbose = "true"
     options.mp_update = "false"

tests/test_git_refs.py

@@ -141,6 +141,7 @@ def test_root_ref_snapshot_falls_back_for_unborn_head(
 
     def read_head(name: str) -> None:
         assert name == "HEAD"
+        assert refs._symref is not None
         refs._symref[name] = "refs/heads/main"
 
     monkeypatch.setattr(refs, "_ReadSymbolicRef", read_head)
@@ -171,6 +172,7 @@ def test_old_git_keeps_separate_head_fallback(
 
     def read_head(name: str) -> None:
         assert name == "HEAD"
+        assert refs._symref is not None
         refs._symref[name] = "refs/heads/main"
 
     monkeypatch.setattr(refs, "_ReadSymbolicRef", read_head)
@@ -204,6 +206,7 @@ def test_for_each_ref_failure_falls_back_to_symbolic_ref(
 
     def read_head(name: str) -> None:
         assert name == "HEAD"
+        assert refs._symref is not None
         refs._symref[name] = "refs/heads/main"
 
     monkeypatch.setattr(refs, "_ReadSymbolicRef", read_head)

tests/test_manifest_xml.py

@@ -180,7 +180,9 @@ class TestValue:
     """Check utility parsing code."""
 
     def _get_node(self, text: str) -> xml.dom.minidom.Element:
-        return xml.dom.minidom.parseString(text).firstChild
+        node = xml.dom.minidom.parseString(text).firstChild
+        assert isinstance(node, xml.dom.minidom.Element)
+        return node
 
     def test_bool_default(self) -> None:
         """Check XmlBool default handling."""

tests/test_repo_trace.py

@@ -27,6 +27,7 @@ def test_trace_max_size_enforced(monkeypatch: pytest.MonkeyPatch) -> None:
 
     # Enable trace for the test, in case users have it disabled.
     monkeypatch.setattr(repo_trace, "_TRACE", True)
+    assert repo_trace._TRACE_FILE is not None
 
     with repo_trace.Trace(content, first_trace=True):
         pass

tests/test_ssh.py

@@ -91,11 +91,15 @@ def test_ssh_sock(monkeypatch: pytest.MonkeyPatch) -> None:
         # Old ssh version uses port.
         with mock.patch.object(ssh, "version", return_value=(6, 6)):
             with proxy as ssh_proxy:
-                assert ssh_proxy.sock().endswith("%p")
+                sock = ssh_proxy.sock()
+                assert sock is not None
+                assert sock.endswith("%p")
 
         proxy._sock_path = None
         # New ssh version uses hash.
         with mock.patch.object(ssh, "version", return_value=(6, 7)):
             with proxy as ssh_proxy:
-                assert ssh_proxy.sock().endswith("%C")
+                sock = ssh_proxy.sock()
+                assert sock is not None
+                assert sock.endswith("%C")
         proxy._sock_path = None

tests/test_subcmds.py

@@ -112,6 +112,7 @@ def test_cli_option_dest() -> None:
             bad_opts.append((str(opt), opt.dest))
 
     # Hook the option check list.
+    assert optparse.Option.CHECK_METHODS is not None
     optparse.Option.CHECK_METHODS.insert(0, _check_dest)
 
     try:

tests/test_subcmds_upload.py

@@ -14,7 +14,7 @@
 
 """Unittests for the subcmds/upload.py module."""
 
-from typing import List, Optional
+from typing import Iterator, List, Optional
 from unittest import mock
 
 import pytest
@@ -34,7 +34,7 @@ _STUB_PEOPLE = ([], [])
 
 
 @pytest.fixture
-def cmd() -> upload.Upload:
+def cmd() -> Iterator[upload.Upload]:
     """Fixture to provide an Upload command instance with mocked methods."""
     cmd = upload.Upload()
     with mock.patch.object(

tests/utils_for_test.py

@@ -50,7 +50,7 @@ def init_git_tree(
         else:
             # Use template dir for init.
             templatedir = stack.enter_context(
-                tempfile.mkdtemp(prefix="git-template")
+                tempfile.TemporaryDirectory(prefix="git-template")
             )
             (Path(templatedir) / "HEAD").write_text("ref: refs/heads/main\n")
             cmd += ["--template", templatedir]

wrapper.py

@@ -31,6 +31,7 @@ def Wrapper():
     modname = "wrapper"
     loader = importlib.machinery.SourceFileLoader(modname, WrapperPath())
     spec = importlib.util.spec_from_loader(modname, loader)
+    assert spec is not None
     module = importlib.util.module_from_spec(spec)
     loader.exec_module(module)
     return module