Enable ruff's flake8-bugbear ruleset

bugbear was one of the most popular flake8 plugins and now ruff supports
it. Running it found a number of missing "assert" statements, among
other issues I all found valuable to fix:

* Useless statements that were probably missing a leading "assert"
* Mutable default arguments that should be constructed at function
  runtime
* Unused loop variables that don't start with _
* Usage of str.strip() with a multi-character string

I suppressed two types of errors:
* Raising an exception inside an exception handler without using
  `from err`; would be nice to address later, but there were too many
  to assess.
* Loop variables being bound past their lifetime; the two uses were only
  in tests and safe so I applied individual suppressions as we'd want to
  flag problematic uses in real code.

client/securedrop_client/app.py

@@ -194,7 +194,7 @@ def prevent_second_instance(app: QApplication, unique_name: str) -> None:
 def threads(count: int) -> Any:
     """Ensures that the thread is properly closed before its reference is dropped."""
     threads = []
-    for i in range(count):
+    for _i in range(count):
         threads.append(QThread())
 
     yield threads

client/securedrop_client/export.py

@@ -346,7 +346,7 @@ def print(self, filepaths: list[str]) -> None:
                 self.print_failed.emit(ExportStatus.ERROR_PRINT)
 
     def _create_archive(
-        self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: list[str] = []
+        self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: list[str] | None = None
     ) -> str:
         """
         Create the archive to be sent to the Export VM.
@@ -361,6 +361,8 @@ def _create_archive(
             str: The path to newly-created archive file.
         """
         archive_path = os.path.join(archive_dir, archive_fn)
+        if filepaths is None:
+            filepaths = []
 
         with tarfile.open(archive_path, "w:gz") as archive:
             self._add_virtual_file_to_archive(archive, self._METADATA_FN, metadata)

client/securedrop_client/gui/base/misc.py

@@ -154,11 +154,13 @@ def __init__(
         self,
         text: str = "",
         parent: QWidget | None = None,
-        flags: Qt.WindowFlags | Qt.WindowType = Qt.WindowFlags(),
+        flags: Qt.WindowFlags | Qt.WindowType | None = None,
         wordwrap: bool = True,
         max_length: int = 0,
         with_tooltip: bool = False,
     ):
+        if flags is None:
+            flags = Qt.WindowFlags()
         super().__init__(parent, flags)
         self.wordwrap = wordwrap
         self.max_length = max_length

client/tests/api_jobs/test_base.py

@@ -182,7 +182,7 @@ def test_ApiJob_order_number_unset(mocker):
     api_job_2 = api_job_cls()
 
     with pytest.raises(ValueError):
-        api_job_1 < api_job_2
+        assert api_job_1 < api_job_2
 
 
 def test_SingleObjectApiJob_comparison_obj_without_uuid_attr(mocker):

client/tests/gui/conversation/delete/test_dialog.py

@@ -18,9 +18,9 @@ def test_displays_accurate_source_information_by_default(self):
         assert "0 replies" in self.dialog.text()
 
     def test_displays_updated_source_information_when_shown(self):
-        for i in range(2):
+        for _i in range(2):
             factory.Reply(source=self._source)
-        for i in range(3):
+        for _i in range(3):
             factory.Message(source=self._source)
 
         QTimer.singleShot(300, self.dialog.close)

client/tests/gui/test_main.py

@@ -141,7 +141,7 @@ def test_show_main_window_without_username(mocker, homedir, session_maker):
 
     w.show.assert_called_once_with()
     w.showMaximized.assert_not_called()
-    w.set_logged_in_as.called is False
+    assert w.set_logged_in_as.called is False
 
     controller.qubes = False
     w.setup(controller)
@@ -153,7 +153,7 @@ def test_show_main_window_without_username(mocker, homedir, session_maker):
 
     w.show.assert_not_called()
     w.showMaximized.assert_called_once_with()
-    w.set_logged_in_as.called is False
+    assert w.set_logged_in_as.called is False
 
 
 def test_show_main_window_without_username_when_already_showing(mocker, homedir, session_maker):
@@ -169,7 +169,7 @@ def test_show_main_window_without_username_when_already_showing(mocker, homedir,
 
     w.show.assert_not_called()
     w.showMaximized.assert_not_called()
-    w.set_logged_in_as.called is False
+    assert w.set_logged_in_as.called is False
 
     controller.qubes = False
     w.setup(controller)
@@ -181,7 +181,7 @@ def test_show_main_window_without_username_when_already_showing(mocker, homedir,
 
     w.show.assert_not_called()
     w.showMaximized.assert_not_called()
-    w.set_logged_in_as.called is False
+    assert w.set_logged_in_as.called is False
 
 
 def test_show_login(mocker):

client/tests/gui/test_widgets.py

@@ -451,7 +451,7 @@ def test_UserButton_setup(mocker):
 def test_UserButton_set_username():
     ub = UserButton()
     ub.set_username("test_username")
-    ub.text() == "test_username"
+    assert ub.text() == "test_username"
 
 
 def test_UserButton_set_long_username(mocker):
@@ -661,7 +661,7 @@ def test_MainView_on_source_changed_shows_correct_context(mocker, homedir, sessi
     """
     # Build sourcelist
     sources = []
-    for i in range(10):
+    for _i in range(10):
         s = factory.Source()
         sources.append(s)
         session.add(s)
@@ -2645,7 +2645,7 @@ def test_SpeechBubble_init(mocker):
 
     sb = SpeechBubble("mock id", "hello", mock_update_signal, mock_download_error_signal, 0, 123)
 
-    sb.message.text() == "hello"
+    assert sb.message.text() == "hello"
 
     assert mock_update_connect.called
     assert mock_download_error_connect.called
@@ -2673,7 +2673,7 @@ def test_SpeechBubble_init_with_error(mocker):
         failed_to_decrypt=True,
     )
 
-    sb.message.text() == "hello"
+    assert sb.message.text() == "hello"
     assert mock_update_connect.called
     assert mock_download_error_connect.called
 
@@ -3107,7 +3107,7 @@ def test_ReplyWidget__on_update_authenticated_user_updates_sender_when_changed(m
     user.username = "baz"
     reply_widget._on_update_authenticated_user(authenticated_user)
 
-    reply_widget.sender.username == "baz"
+    assert reply_widget.sender.username == "baz"
     assert reply_widget.sender_is_current_user
     assert reply_widget.sender_icon.is_current_user
     reply_widget._update_styles.assert_called_once_with()
@@ -3272,7 +3272,7 @@ def test_FileWidget_event_handler_left_click(mocker, session, source):
     fw._on_left_click = mocker.MagicMock()
 
     fw.eventFilter(fw, test_event)
-    fw._on_left_click.call_count == 1
+    assert fw._on_left_click.call_count == 1
 
 
 def test_FileWidget_event_handler_hover(mocker, session, source):
@@ -3362,8 +3362,8 @@ def test_FileWidget_on_left_click_downloading_in_progress(mocker, session, sourc
     fw.download_button = mocker.MagicMock()
 
     fw._on_left_click()
-    get_file.call_count == 0
-    controller.on_submission_download.call_count == 0
+    assert get_file.call_count == 0
+    assert controller.on_submission_download.call_count == 0
 
 
 def test_FileWidget_start_button_animation(mocker, session, source):

client/tests/integration/test_styles_sdclient.py

@@ -72,9 +72,9 @@ def test_class_name_matches_css_object_name(mocker, main_window):
         main_view.MULTI_SELECTED_INDEX,
     ]:
         view = main_view.view_layout.widget(index)
-        view.objectName() == "EmptyConversationView"
-        "EmptyConversationView" in view.objectName()
-        "EmptyConversationView" in view.objectName()
+        assert view.objectName() == "EmptyConversationView"
+        assert "EmptyConversationView" in view.objectName()
+        assert "EmptyConversationView" in view.objectName()
 
     source_list = main_view.source_list
 
@@ -272,20 +272,20 @@ def test_styles_for_main_view(mocker, main_window):
     assert no_source_selected_spacer1.maximumSize().height() == 35
     bullet1_bullet = no_source_selected.layout().itemAt(2).widget().layout().itemAt(0).widget()
     assert bullet1_bullet.getContentsMargins() == (0, 4, 0, 0)
-    bullet1_bullet.font().pixelSize() == 30
-    QFont.Bold == bullet1_bullet.font().weight()
+    assert bullet1_bullet.font().pixelSize() == 30
+    assert QFont.Bold == bullet1_bullet.font().weight()
     assert bullet1_bullet.font().family() == "Montserrat"
     assert bullet1_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9"
     bullet2_bullet = no_source_selected.layout().itemAt(3).widget().layout().itemAt(0).widget()
     assert bullet2_bullet.getContentsMargins() == (0, 4, 0, 0)
-    bullet2_bullet.font().pixelSize() == 30
-    QFont.Bold == bullet2_bullet.font().weight()
+    assert bullet2_bullet.font().pixelSize() == 30
+    assert QFont.Bold == bullet2_bullet.font().weight()
     assert bullet2_bullet.font().family() == "Montserrat"
     assert bullet2_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9"
     bullet3_bullet = no_source_selected.layout().itemAt(4).widget().layout().itemAt(0).widget()
     assert bullet3_bullet.getContentsMargins() == (0, 4, 0, 0)
-    bullet3_bullet.font().pixelSize() == 30
-    QFont.Bold == bullet3_bullet.font().weight()
+    assert bullet3_bullet.font().pixelSize() == 30
+    assert QFont.Bold == bullet3_bullet.font().weight()
     assert bullet3_bullet.font().family() == "Montserrat"
     assert bullet3_bullet.palette().color(QPalette.Foreground).name() == "#a5b3e9"
     no_source_selected_spacer2 = no_source_selected.layout().itemAt(5)
@@ -298,23 +298,23 @@ def test_styles_source_list(mocker, main_window):
     source_widget = source_list.itemWidget(source_list.item(0))
     preview = source_widget.preview
     assert preview.font().family() == "Source Sans Pro"
-    QFont.Normal == preview.font().weight()
-    preview.font().pixelSize() == 13
+    assert QFont.Normal == preview.font().weight()
+    assert preview.font().pixelSize() == 13
     assert preview.palette().color(QPalette.Foreground).name() == "#383838"
     deletion_indicator = source_widget.deletion_indicator
     assert deletion_indicator.font().family() == "Source Sans Pro"
-    QFont.Normal == deletion_indicator.font().weight()
-    deletion_indicator.font().pixelSize() == 13
+    assert QFont.Normal == deletion_indicator.font().weight()
+    assert deletion_indicator.font().pixelSize() == 13
     assert deletion_indicator.palette().color(QPalette.Foreground).name() == "#000000"
     name = source_widget.name
     assert name.font().family() == "Montserrat"
-    QFont.Normal == name.font().weight()
-    name.font().pixelSize() == 13
+    assert QFont.Normal == name.font().weight()
+    assert name.font().pixelSize() == 13
     assert name.palette().color(QPalette.Foreground).name() == "#2a319d"
     timestamp = source_widget.timestamp
     assert timestamp.font().family() == "Montserrat"
-    QFont.Normal == timestamp.font().weight()
-    timestamp.font().pixelSize() == 13
+    assert QFont.Normal == timestamp.font().weight()
+    assert timestamp.font().pixelSize() == 13
     assert timestamp.palette().color(QPalette.Foreground).name() == "#383838"
 
 

client/tests/test_app.py

@@ -205,7 +205,7 @@ def test_create_app_dir_permissions(tmpdir, mocker):
         mocker.patch("securedrop_client.app.make_session_maker", return_value=mock_session_maker)
 
         def func():
-            start_app(mock_args, mock_qt_args)
+            start_app(mock_args, mock_qt_args)  # noqa: B023
 
         func()
 

client/tests/test_logic.py

@@ -596,7 +596,7 @@ def test_Controller_on_sync_success_username_change(homedir, session, config, mo
 
     co.on_sync_success()
 
-    co.authenticated_user.username == "baz"
+    assert co.authenticated_user.username == "baz"
     assert len(update_authenticated_user_emissions) == 1
     assert update_authenticated_user_emissions[0] == [co.authenticated_user]
 
@@ -618,7 +618,7 @@ def test_Controller_on_sync_success_firstname_change(homedir, session, config, m
 
     co.on_sync_success()
 
-    co.authenticated_user.firstname == "baz"
+    assert co.authenticated_user.firstname == "baz"
     assert len(update_authenticated_user_emissions) == 1
     assert update_authenticated_user_emissions[0] == [co.authenticated_user]
 
@@ -640,7 +640,7 @@ def test_Controller_on_sync_success_lastname_change(homedir, session, config, mo
 
     co.on_sync_success()
 
-    co.authenticated_user.lastname == "baz"
+    assert co.authenticated_user.lastname == "baz"
     assert len(update_authenticated_user_emissions) == 1
     assert update_authenticated_user_emissions[0] == [co.authenticated_user]
 
@@ -1134,7 +1134,7 @@ def test_create_client_dir_permissions(tmpdir, mocker, session_maker):
             os.chmod(sdc_home, case["home_perms"])
 
         def func() -> None:
-            Controller("http://localhost", mock_gui, session_maker, sdc_home, None)
+            Controller("http://localhost", mock_gui, session_maker, sdc_home, None)  # noqa: B023
 
         if case["should_pass"]:
             func()
@@ -1391,9 +1391,10 @@ def test_Controller_on_file_downloaded_checksum_failure(homedir, config, mocker,
 
     # Job should get resubmitted and we should log this is happening
     assert co._submit_download_job.call_count == 1
-    warning_logger.call_args_list[0][0][
-        0
-    ] == f"Failure due to checksum mismatch, retrying {file_.uuid}"
+    assert (
+        warning_logger.call_args_list[0][0][0]
+        == f"Failure due to checksum mismatch, retrying {file_.uuid}"
+    )
 
     # No status will be set if it's a file corruption issue, the file just gets
     # re-downloaded.
@@ -1425,7 +1426,7 @@ def test_Controller_on_file_decryption_failure(homedir, config, mocker, session,
     mock_update_error_status.assert_called_once_with("The file download failed. Please try again.")
 
     assert co._submit_download_job.call_count == 0
-    error_logger.call_args_list[0][0][0] == f"Failed to decrypt {file_.uuid}"
+    assert error_logger.call_args_list[0][0][0] == f"Failed to decrypt {file_.uuid}"
 
     mock_set_status.assert_not_called()
 
@@ -1660,9 +1661,10 @@ def test_Controller_on_reply_downloaded_checksum_failure(mocker, homedir, sessio
 
     # Job should get resubmitted and we should log this is happening
     assert co._submit_download_job.call_count == 1
-    warning_logger.call_args_list[0][0][
-        0
-    ] == f"Failure due to checksum mismatch, retrying {reply.uuid}"
+    assert (
+        warning_logger.call_args_list[0][0][0]
+        == f"Failure due to checksum mismatch, retrying {reply.uuid}"
+    )
 
 
 def test_Controller_on_reply_downloaded_decryption_failure(mocker, homedir, session_maker):
@@ -1773,7 +1775,7 @@ def test_Controller_download_new_messages_skips_recent_failures(
     co.download_new_messages()
 
     assert len(add_job_emissions) == 0
-    info_logger.call_args_list[0][0][0] == (
+    assert info_logger.call_args_list[0][0][0] == (
         f"Download of message {message.uuid} failed since client start; not retrying."
     )
 
@@ -1807,7 +1809,7 @@ def test_Controller_download_new_replies_skips_recent_failures(
     co.download_new_replies()
 
     assert len(add_job_emissions) == 0
-    info_logger.call_args_list[0][0][0] == (
+    assert info_logger.call_args_list[0][0][0] == (
         f"Download of reply {reply.uuid} failed since client start; not retrying."
     )
 

client/tests/test_queue.py

@@ -203,8 +203,7 @@ def test_RunnableQueue_duplicate_jobs(mocker):
     queue.add_job(dl_job)
     assert len(queue.queue.queue) == 1
 
-    log_msg = f"Duplicate job {dl_job}, skipping"
-    debug_logger.call_args[1] == log_msg
+    assert debug_logger.call_args[1] == f"Duplicate job {dl_job}, skipping"
 
     # Now add a _different_ job with the same arguments (same uuid).
     queue.add_job(msg_dl_job)

client/tests/test_storage.py

@@ -312,8 +312,8 @@ def test_update_local_storage_sanitizes_remote_data(mocker, homedir):
     )
 
     sanitize_sources.assert_called_once_with([remote_source])
-    sanitize_submissions_or_replies.call_args_list[0][0][0] == [remote_submissions]
-    sanitize_submissions_or_replies.call_args_list[1][0][0] == [remote_reply]
+    assert sanitize_submissions_or_replies.call_args_list[0][0][0] == [remote_submissions]
+    assert sanitize_submissions_or_replies.call_args_list[1][0][0] == [remote_reply]
 
 
 def test_sync_delete_race(homedir, mocker, session_maker, session):
@@ -1677,7 +1677,7 @@ def test_delete_single_submission_or_reply_race_guard(homedir, mocker):
     mock_remove = mocker.patch("os.remove", side_effect=FileNotFoundError)
     delete_single_submission_or_reply_on_disk(test_obj, homedir)
 
-    mock_remove.call_count == 1
+    assert mock_remove.call_count == 1
 
 
 def test_delete_single_submission_or_reply_single_file(homedir, mocker):

export/securedrop_export/disk/service.py

@@ -16,8 +16,11 @@ class Service:
     This is the "API" portion of the export workflow.
     """
 
-    def __init__(self, submission: Archive, cli: CLI = CLI()):
-        self.cli = cli
+    def __init__(self, submission: Archive, cli: CLI | None = None):
+        if cli is not None:
+            self.cli = cli
+        else:
+            self.cli = CLI()
         self.submission = submission
 
     def scan_all_devices(self) -> Status:

export/securedrop_export/print/service.py

@@ -287,7 +287,9 @@ def _get_supported_mimetypes_libreoffice(self, desktop_dir: Path):
                     for line in f.readlines():
                         if line.startswith("MimeType="):
                             # Semicolon-separated list; don't leave empty element at the end
-                            supported_mimetypes.update(line.strip("MimeType=").split(";")[:-1])
+                            supported_mimetypes.update(
+                                line.removeprefix("MimeType=").split(";")[:-1]
+                            )
 
         return supported_mimetypes
 

log/log_server/log_saver.py

@@ -46,7 +46,7 @@ def main():
     except Exception as e:
         print(e, file=sys.stderr)
         # Clean up all open files
-        for k, v in openfiles:
+        for _k, v in openfiles:
             v.close()
         sys.exit(1)