[PATCH b4 07/12] Enable mypy unreachable warnings

Turn on warn_unreachable and remove dead branches that it exposes.

Some of those branches were stale null checks against non-optional APIs.
Others were test assertions that hit mypy's stale narrowing of mutable
attributes, so add targeted ignores with a reference to the upstream
issue.

Signed-off-by: Tamir Duberstein <[email protected]>
---
 pyproject.toml                   |  2 +-
 src/b4/__init__.py               |  6 ++----
 src/b4/mbox.py                   |  2 --
 src/b4/pr.py                     |  2 +-
 src/b4/review/tracking.py        |  3 ++-
 src/b4/review_tui/_common.py     |  4 ----
 src/b4/review_tui/_modals.py     |  2 --
 src/b4/review_tui/_review_app.py |  4 ++--
 src/b4/ty.py                     |  3 ---
 src/tests/test_tui_modals.py     | 12 +++++++++---
 src/tests/test_tui_tracking.py   | 12 +++++++++---
 11 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index c08f47e..982ad95 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -121,6 +121,6 @@ flake8-quotes.inline-quotes = "single"
 [tool.pyright]
 typeCheckingMode = "off"
 
-# Configure mypy in strict mode
 [tool.mypy]
 strict = true
+warn_unreachable = true
diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index a14d89c..20d1f75 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -1056,9 +1056,7 @@ class LoreSeries:
         return len(self.indexes), mismatches
 
     def find_base(self, gitdir: Optional[str], branches: Optional[List[str]] = None, maxdays: int = 30) -> Tuple[str, int, int]:
-        if self.indexes is None:
-            self.populate_indexes()
-        if self.indexes is None or not len(self.indexes):
+        if not self.indexes:
             raise IndexError('No indexes to check against')
 
         pdate = self.submission_date
@@ -1554,7 +1552,7 @@ class LoreMessage:
         # walk until we find the first text/plain part
         self.body, self.charset = LoreMessage.get_payload(self.msg)
 
-        if self.body is None:
+        if not self.body:
             # Woah, we didn't find any usable parts
             logger.debug('  No plain or patch parts found in message')
             logger.info('  Not plaintext: %s', self.full_subject)
diff --git a/src/b4/mbox.py b/src/b4/mbox.py
index 624a2f3..5e8093a 100644
--- a/src/b4/mbox.py
+++ b/src/b4/mbox.py
@@ -202,8 +202,6 @@ def make_am(msgs: List[EmailMessage], cmdargs: argparse.Namespace, msgid: str) -
         if cmdargs.cherrypick == '_':
             # We might want to pick a patch sent as a followup, so create a fake series
             # and add followups with diffs
-            if lser is None:
-                lser = b4.LoreSeries(revision=1, expected=1)
             for followup in lmbx.followups:
                 if followup.has_diff:
                     lser.add_patch(followup)
diff --git a/src/b4/pr.py b/src/b4/pr.py
index cb2ca76..939c1f5 100644
--- a/src/b4/pr.py
+++ b/src/b4/pr.py
@@ -84,7 +84,7 @@ def git_get_commit_id_from_repo_ref(repo: str, ref: str) -> Optional[str]:
 
 def parse_pr_data(msg: email.message.EmailMessage) -> Optional[b4.LoreMessage]:
     lmsg = b4.LoreMessage(msg)
-    if lmsg.body is None:
+    if not lmsg.body:
         logger.critical('Could not find a plain part in the message body')
         return None
 
diff --git a/src/b4/review/tracking.py b/src/b4/review/tracking.py
index e66cfd8..2b0d058 100644
--- a/src/b4/review/tracking.py
+++ b/src/b4/review/tracking.py
@@ -222,13 +222,14 @@ def record_take_branch(gitdir: str, branch: str) -> None:
     metadata_dir = os.path.join(gitdir, REVIEW_METADATA_DIR)
     pathlib.Path(metadata_dir).mkdir(parents=True, exist_ok=True)
     metadata_path = get_repo_metadata_path(gitdir)
-    data: Dict[str, Any] = {}
     if os.path.exists(metadata_path):
         try:
             with open(metadata_path, 'r') as f:
                 data = json.load(f)
         except (json.JSONDecodeError, OSError):
             pass
+    else:
+        data = {}
     if not isinstance(data, dict):
         data = {}
     branches = data.get('recent-take-branches', [])
diff --git a/src/b4/review_tui/_common.py b/src/b4/review_tui/_common.py
index f30405d..536af2d 100644
--- a/src/b4/review_tui/_common.py
+++ b/src/b4/review_tui/_common.py
@@ -720,10 +720,6 @@ def gather_attestation_info(lser: b4.LoreSeries) -> Dict[str, Any]:
     apply_mismatches = 0
 
     if topdir:
-        # Ensure indexes are populated for applicability check
-        if lser.indexes is None:
-            lser.populate_indexes()
-
         if base_commit:
             base_exists = b4.git_commit_exists(topdir, base_commit)
 
diff --git a/src/b4/review_tui/_modals.py b/src/b4/review_tui/_modals.py
index 10bffb5..0cd80e1 100644
--- a/src/b4/review_tui/_modals.py
+++ b/src/b4/review_tui/_modals.py
@@ -2156,8 +2156,6 @@ class TargetBranchScreen(ModalScreen[Optional[str]]):
             ifh = io.BytesIO()
             b4.save_git_am_mbox(am_msgs, ifh)
             ambytes = ifh.getvalue()
-            if lser.indexes is None:
-                lser.populate_indexes()
             return lser, ambytes
 
     def _check_applicability(self, branch: str) -> None:
diff --git a/src/b4/review_tui/_review_app.py b/src/b4/review_tui/_review_app.py
index c6c926d..c14005b 100644
--- a/src/b4/review_tui/_review_app.py
+++ b/src/b4/review_tui/_review_app.py
@@ -1150,7 +1150,7 @@ class ReviewApp(CheckRunnerMixin, App[None]):
             result = b4.edit_in_editor(
                 editor_text.encode(), filehint='reply.b4-review.eml')
 
-        if result is None:
+        if not result:
             self.notify('Editor returned no content')
             return
         reply_text = result.decode(errors='replace')
@@ -1271,7 +1271,7 @@ class ReviewApp(CheckRunnerMixin, App[None]):
         with self.suspend():
             result = b4.edit_in_editor(editor_text.encode(), filehint='note.txt')
 
-        if result is None:
+        if not result:
             self.notify('Editor returned no content')
             return
         raw_text = result.decode(errors='replace')
diff --git a/src/b4/ty.py b/src/b4/ty.py
index 5786222..74c094a 100644
--- a/src/b4/ty.py
+++ b/src/b4/ty.py
@@ -460,9 +460,6 @@ def send_messages(listing: List[JsonDictT], branch: str, cmdargs: argparse.Names
             # This is a patch series
             msg = generate_am_thanks(gitdir, jsondata, branch, cmdargs)
 
-        if msg is None:
-            continue
-
         assert isinstance(jsondata['msgid'], str), 'msgid must be a string'
         msgids.append(jsondata['msgid'])
         assert isinstance(jsondata['patches'], list), 'patches must be a list'
diff --git a/src/tests/test_tui_modals.py b/src/tests/test_tui_modals.py
index e0e6f3f..6a608a9 100644
--- a/src/tests/test_tui_modals.py
+++ b/src/tests/test_tui_modals.py
@@ -84,7 +84,9 @@ class TestHelpScreen:
             await pilot.pause()
             # Should be back on the host screen
             assert not isinstance(app.screen, HelpScreen)
-            assert dismissed == [None]
+            # https://github.com/python/mypy/issues/9457:
+            # app.screen is stale-narrowed across await.
+            assert dismissed == [None]  # type: ignore[unreachable]
 
     @pytest.mark.asyncio
     async def test_question_mark_dismisses(self) -> None:
@@ -168,7 +170,9 @@ class TestConfirmScreen:
             await pilot.press('y')
             await pilot.pause()
             assert not isinstance(app.screen, ConfirmScreen)
-            assert results == [True]
+            # https://github.com/python/mypy/issues/9457:
+            # app.screen is stale-narrowed across await.
+            assert results == [True]  # type: ignore[unreachable]
 
     @pytest.mark.asyncio
     async def test_escape_cancels(self) -> None:
@@ -459,7 +463,9 @@ class TestPriorReviewScreen:
             await pilot.press('escape')
             await pilot.pause()
             assert not isinstance(app.screen, PriorReviewScreen)
-            assert results == [None]
+            # https://github.com/python/mypy/issues/9457:
+            # app.screen is stale-narrowed across await.
+            assert results == [None]  # type: ignore[unreachable]
 
     @pytest.mark.asyncio
     async def test_content_rendered(self) -> None:
diff --git a/src/tests/test_tui_tracking.py b/src/tests/test_tui_tracking.py
index 80004e8..103a7c3 100644
--- a/src/tests/test_tui_tracking.py
+++ b/src/tests/test_tui_tracking.py
@@ -934,7 +934,9 @@ class TestTrackingSnooze:
             assert not isinstance(app.screen, SnoozeScreen)
 
             # Verify DB was updated
-            conn = tracking.get_db(identifier)
+            # https://github.com/python/mypy/issues/9457:
+            # app.screen is stale-narrowed across await.
+            conn = tracking.get_db(identifier)  # type: ignore[unreachable]
             cursor = conn.execute(
                 'SELECT status, snoozed_until FROM series WHERE change_id = ?',
                 ('snooze-test-1',))
@@ -1930,7 +1932,9 @@ class TestTargetBranch:
             assert not isinstance(app.screen, TargetBranchScreen)
 
             # Verify DB cleared
-            conn = tracking.get_db(identifier)
+            # https://github.com/python/mypy/issues/9457:
+            # app.screen is stale-narrowed across await.
+            conn = tracking.get_db(identifier)  # type: ignore[unreachable]
             target = tracking.get_target_branch(conn, change_id)
             conn.close()
             assert target is None
@@ -2358,7 +2362,9 @@ class TestLoadSeriesCaching:
             assert app._cached_branch_tips is not None
             app._invalidate_caches()
             assert app._cached_branch_tips is None
-            assert app._cached_newest_revisions is None
+            # https://github.com/python/mypy/issues/9457:
+            # app._cached_branch_tips is stale-narrowed across a method call.
+            assert app._cached_newest_revisions is None  # type: ignore[unreachable]
             assert app._cached_revision_counts is None
             assert app._cached_art_counts is None
 

-- 
2.53.0