fix ruff rules B017, B018, B028, B905 - or explictly disable per line (#20990)

* fix ruff rules B017, B018, B028, B905 - or explictly disable per line

* slightly rework get_disco_recommendations

pyproject.toml

@@ -22,11 +22,7 @@ exclude = [
     "*/migrations/*.py",
 ]
 ignore = [
-  "B017",  # `assertRaises(Exception)` should be considered evil
-  "B018",  # Found useless attribute access. Either assign it to a variable or remove it.
-  "B028",  # No explicit `stacklevel` keyword argument found
   "B904",  # Within an `except` clause, raise exceptions with `raise ... from err|None`
-  "B905"   # `zip()` without an explicit `strict=` parameter
 ]
 line-length = 88
 select = [

settings.py

@@ -140,7 +140,8 @@ except ImportError:
     import warnings
 
     warnings.warn(
-        'Could not import local_settings module. {}'.format(traceback.format_exc())
+        'Could not import local_settings module. {}'.format(traceback.format_exc()),
+        stacklevel=1,
     )
 
 SITEMAP_DEBUG_AVAILABLE = True

src/olympia/addons/serializers.py

@@ -717,7 +717,7 @@ class CurrentVersionSerializer(SimpleVersionSerializer):
             # but we want {'min': min, 'max': max}.
             value = AddonAppVersionQueryParam(request.GET).get_values()
             application = value[0]
-            appversions = dict(zip(('min', 'max'), value[1:]))
+            appversions = dict(zip(('min', 'max'), value[1:], strict=True))
         except ValueError as exc:
             raise exceptions.ParseError(str(exc))
 

src/olympia/addons/tests/test_serializers.py

@@ -1543,7 +1543,7 @@ class TestLanguageToolsSerializerOutput(TestCase):
         # Create a new current version, just to prove that
         # current_compatible_version does not use that.
         version_factory(addon=self.addon)
-        self.addon.reload
+        self.addon.reload()
         assert self.addon.compatible_versions[0] != self.addon.current_version
         self.request = APIRequestFactory().get('/?app=firefox&appversion=57.0')
         result = self.serialize()

src/olympia/addons/tests/test_tasks.py

@@ -448,7 +448,7 @@ class TestResizeIcon(TestCase):
             os.makedirs(uploadto)
         except OSError:
             pass
-        for rsize, expected_size in zip(resize_size, final_size):
+        for rsize, expected_size in zip(resize_size, final_size, strict=True):
             # resize_icon moves the original
             shutil.copyfile(img, src.name)
             src_image = Image.open(src.name)

src/olympia/api/models.py

@@ -43,7 +43,7 @@ class APIKey(ModelBase):
     # is_active=False when revoking keys).
     is_active = models.BooleanField(default=True, null=True)
     type = models.PositiveIntegerField(
-        choices=dict(zip(API_KEY_TYPES, API_KEY_TYPES)).items(), default=0
+        choices=dict(zip(API_KEY_TYPES, API_KEY_TYPES, strict=True)).items(), default=0
     )
     key = models.CharField(max_length=255, db_index=True, unique=True)
     # TODO: use RSA public keys instead? If we were to use JWT RSA keys

src/olympia/blocklist/mlbf.py

@@ -228,11 +228,11 @@ class DatabaseMLBF(MLBF):
     @cached_property
     def not_blocked_items(self):
         # see blocked_items - we need self._version_excludes populated
-        self.blocked_items
+        blocked_items = self.blocked_items
         # even though we exclude all the version ids in the query there's an
         # edge case where the version string occurs twice for an addon so we
         # ensure not_blocked_items doesn't contain any blocked_items.
         return list(
             self.hash_filter_inputs(fetch_all_versions_from_db(self._version_excludes))
-            - set(self.blocked_items)
+            - set(blocked_items)
         )

src/olympia/devhub/forms.py

@@ -211,7 +211,7 @@ class BaseCategoryFormSet(BaseFormSet):
             cats = self.addon.app_categories.get(app.short, [])
             self.initial.append({'categories': [c.id for c in cats]})
 
-        for app, form in zip(apps, self.forms):
+        for app, form in zip(apps, self.forms, strict=True):
             key = app.id if app else None
             form.request = self.request
             form.initial['application'] = key
@@ -901,7 +901,7 @@ class BaseCompatFormSet(BaseModelFormSet):
         # del self.forms
         if hasattr(self, 'forms'):
             del self.forms
-        self.forms
+        self.forms  # noqa: B018
 
     def add_fields(self, form, index):
         # By default django handles can_delete globally for the whole formset,

src/olympia/discovery/utils.py

@@ -81,9 +81,7 @@ def get_disco_recommendations(hashed_client_id, overrides):
             .filter(guid__in=guids)
         )
         for addon in qs:
-            try:
-                addon.discoveryitem
-            except DiscoveryItem.DoesNotExist:
+            if not hasattr(addon, 'discoveryitem'):
                 # This just means the add-on isn't "known" as a possible
                 # recommendation, but this is fine: create a dummy instance,
                 # and it will use the add-on name and description to build the

src/olympia/files/models.py

@@ -127,12 +127,7 @@ class File(OnChangeMixin, ModelBase):
 
     @property
     def has_been_validated(self):
-        try:
-            self.validation
-        except FileValidation.DoesNotExist:
-            return False
-        else:
-            return True
+        return hasattr(self, 'validation')
 
     def get_url_path(self, attachment=False):
         # We allow requests to not specify a filename, but it's mandatory that

src/olympia/git/tests/test_commands.py

@@ -276,7 +276,7 @@ class TestGitExtraction(TestCase):
         version = self.addon.current_version
         repo = AddonGitRepository(self.addon)
         # Force the creation of the git repository.
-        repo.git_repository
+        repo.git_repository  # noqa: B018
         assert repo.is_extracted
         # Set the "creation time" of the git repository to something older than
         # 1 hour.

src/olympia/git/tests/test_tasks.py

@@ -68,7 +68,7 @@ class TestOnExtractionError(TestCase):
         addon = addon_factory()
         addon_repo = AddonGitRepository(addon)
         # Create the git repo
-        addon_repo.git_repository
+        addon_repo.git_repository  # noqa: B018
         update_git_repo_creation_time(addon_repo, time=datetime.datetime(2019, 1, 1))
         assert addon_repo.is_extracted
         assert not addon_repo.is_recent
@@ -90,7 +90,7 @@ class TestOnExtractionError(TestCase):
         addon = addon_factory()
         addon_repo = AddonGitRepository(addon)
         # Create the git repo
-        addon_repo.git_repository
+        addon_repo.git_repository  # noqa: B018
         update_git_repo_creation_time(addon_repo, time=datetime.datetime(2019, 1, 1))
         assert addon_repo.is_extracted
         assert not addon_repo.is_recent
@@ -113,7 +113,7 @@ class TestOnExtractionError(TestCase):
         addon = addon_factory()
         addon_repo = AddonGitRepository(addon)
         # Create the git repo
-        addon_repo.git_repository
+        addon_repo.git_repository  # noqa: B018
         assert addon_repo.is_extracted
         # Simulate a git extraction in progress.
         GitExtractionEntry.objects.create(addon_id=addon.pk, in_progress=True)
@@ -128,7 +128,7 @@ class TestOnExtractionError(TestCase):
         addon = addon_factory()
         addon_repo = AddonGitRepository(addon)
         # Create the git repo
-        addon_repo.git_repository
+        addon_repo.git_repository  # noqa: B018
         # We do not update the creation time of the git repository here so that
         # it is less than 1 hour (because the git repository was created in
         # this test case).

src/olympia/git/tests/test_utils.py

@@ -151,7 +151,7 @@ def test_git_repo_init_with_missing_master_branch_raises_error(settings):
     pygit2.init_repository(path=repo.git_repository_path, bare=False)
 
     with pytest.raises(MissingMasterBranchError):
-        repo.git_repository
+        repo.git_repository  # noqa: B018
 
 
 def test_git_repo_init_opens_existing_repo(settings):
@@ -168,7 +168,7 @@ def test_git_repo_init_opens_existing_repo(settings):
     assert not os.path.exists(expected_path)
 
     # accessing repo.git_repository creates the directory
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     assert os.path.exists(expected_path)
 
     repo2 = AddonGitRepository(4815162342)
@@ -233,7 +233,7 @@ def test_find_or_create_branch_raises_broken_ref_error(settings):
     )
     branch = 'listed'
     # Create the git repo
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     assert repo.is_extracted
     # Create a broken ref, see:
     # https://github.com/mozilla/addons-server/issues/13590
@@ -256,7 +256,7 @@ def test_delete(settings):
     )
     repo = AddonGitRepository(addon)
     # Create the git repo
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     assert repo.is_extracted
     assert addon.current_version.git_hash
     assert addon2.current_version.git_hash
@@ -281,7 +281,7 @@ def test_delete_with_deleted_version(settings):
     version.delete()
     repo = AddonGitRepository(addon)
     # Create the git repo
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     assert repo.is_extracted
     assert version.git_hash
 
@@ -1474,7 +1474,7 @@ def test_is_recent(settings):
     addon_pk = 123
     repo = AddonGitRepository(addon_pk)
     # Force the creation of the git repository.
-    repo.git_repository
+    repo.git_repository  # noqa: B018
 
     assert repo.is_recent
 
@@ -1483,7 +1483,7 @@ def test_is_recent_with_no_description_file(settings):
     addon_pk = 123
     repo = AddonGitRepository(addon_pk)
     # Force the creation of the git repository.
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     # We should have a description file but we noticed errors about missing
     # files in production so let's protect against those errors.
     Path(os.path.join(repo.git_repository_path, repo.GIT_DESCRIPTION)).unlink()
@@ -1495,7 +1495,7 @@ def test_is_recent_with_relatively_recent_repo(settings):
     addon_pk = 123
     repo = AddonGitRepository(addon_pk)
     # Force the creation of the git repository.
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     fifteen_min_ago = datetime.datetime.now() - datetime.timedelta(minutes=15)
     update_git_repo_creation_time(repo, time=fifteen_min_ago)
 
@@ -1506,7 +1506,7 @@ def test_is_recent_with_very_old_repo(settings):
     addon_pk = 123
     repo = AddonGitRepository(addon_pk)
     # Force the creation of the git repository.
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     update_git_repo_creation_time(repo, time=datetime.datetime(2020, 1, 1))
 
     assert not repo.is_recent
@@ -1516,7 +1516,7 @@ def test_is_recent_with_oldish_repo(settings):
     addon_pk = 123
     repo = AddonGitRepository(addon_pk)
     # Force the creation of the git repository.
-    repo.git_repository
+    repo.git_repository  # noqa: B018
     time = datetime.datetime.now() - datetime.timedelta(minutes=60)
     update_git_repo_creation_time(repo, time=time)
 
@@ -1569,7 +1569,7 @@ def test_extract_version_to_git_with_error(incr_mock, extract_and_commit_mock):
     addon = addon_factory(file_kw={'filename': 'webextension_no_id.xpi'})
     extract_and_commit_mock.side_effect = Exception()
 
-    with pytest.raises(Exception):
+    with pytest.raises(Exception):  # noqa: B017
         extract_version_to_git(addon.current_version.pk)
 
     incr_mock.assert_called_with('git.extraction.version.failure')

src/olympia/ratings/tests/test_tasks.py

@@ -63,9 +63,9 @@ class TestAddonRatingAggregates(TestCase):
         assert addon2.text_ratings_count == 0
 
         with self.assertRaises(RatingAggregate.DoesNotExist):
-            addon.ratingaggregate
+            addon.ratingaggregate  # noqa: B018
         with self.assertRaises(RatingAggregate.DoesNotExist):
-            addon2.ratingaggregate
+            addon2.ratingaggregate  # noqa: B018
 
         # Trigger the task and test results.
         addon_rating_aggregates([addon.pk, addon2.pk])

src/olympia/reviewers/tests/test_serializers.py

@@ -65,7 +65,7 @@ class TestFileInfoSerializer(TestCase):
         serializer = FileInfoSerializer(instance=file)
 
         with self.assertRaises(RuntimeError):
-            serializer.data
+            serializer.data  # noqa: B018
 
     def test_can_access_version_from_parent(self):
         serializer = AddonBrowseVersionSerializer(
@@ -188,7 +188,7 @@ class TestFileInfoDiffSerializer(TestCase):
         serializer = FileInfoDiffSerializer(instance=file)
 
         with self.assertRaises(RuntimeError):
-            serializer.data
+            serializer.data  # noqa: B018
 
     def test_can_access_version_from_parent(self):
         parent_version = self.addon.current_version
@@ -545,7 +545,7 @@ class TestAddonBrowseVersionSerializer(TestCase):
 
     def test_sha256_only_calculated_or_fetched_for_selected_file(self):
         serializer = self.get_serializer(file='icons/LICENSE')
-        serializer.data
+        serializer.data  # noqa: B018
 
         assert serializer._entries['manifest.json']['sha256'] is None
         assert serializer._entries['icons/LICENSE']['sha256'] == (
@@ -553,7 +553,7 @@ class TestAddonBrowseVersionSerializer(TestCase):
         )
 
         serializer = self.get_serializer(file='manifest.json')
-        serializer.data
+        serializer.data  # noqa: B018
         assert serializer._entries['manifest.json']['sha256'] == (
             '71d4122c0f2f78e089136602f88dbf590f2fa04bb5bc417454bf21446d6cb4f0'
         )

src/olympia/reviewers/tests/test_utils.py

@@ -34,6 +34,7 @@ from olympia.constants.promoted import (
     STRATEGIC,
 )
 from olympia.files.models import File
+from olympia.lib.crypto.signing import SigningError
 from olympia.lib.crypto.tests.test_signing import (
     _get_recommendation_data,
     _get_signature_details,
@@ -187,7 +188,7 @@ class TestReviewHelper(TestReviewHelperBase):
 
     def test_process_action_none(self):
         self.helper.set_data({'action': 'foo'})
-        with self.assertRaises(Exception):
+        with self.assertRaises(KeyError):
             self.helper.process()
 
     def test_process_action_good(self):
@@ -1885,11 +1886,11 @@ class TestReviewHelper(TestReviewHelperBase):
         assert self.check_log_count(amo.LOG.APPROVE_VERSION.id) == 1
 
     def test_nomination_to_public_failed_signing(self):
-        self.sign_file_mock.side_effect = Exception
+        self.sign_file_mock.side_effect = SigningError
         self.sign_file_mock.reset()
         self.setup_data(amo.STATUS_NOMINATED)
 
-        with self.assertRaises(Exception):
+        with self.assertRaises(SigningError):
             self.helper.handler.approve_latest_version()
 
         # AddonApprovalsCounter was not touched since we failed signing.

src/olympia/scanners/tests/test_actions.py

@@ -647,7 +647,7 @@ class TestActions(TestCase):
     def test_flag_for_human_review_by_scanner(self):
         version = version_factory(addon=addon_factory())
         with self.assertRaises(VersionReviewerFlags.DoesNotExist):
-            version.reviewerflags
+            version.reviewerflags  # noqa: B018
 
         _flag_for_human_review_by_scanner(version=version, rule=None, scanner=MAD)
 
@@ -844,4 +844,4 @@ class TestRunAction(TestCase):
         ScannerResult.run_action(version)
 
         with self.assertRaises(VersionReviewerFlags.DoesNotExist):
-            version.reviewerflags
+            version.reviewerflags  # noqa: B018

src/olympia/scanners/tests/test_tasks.py

@@ -534,11 +534,11 @@ class TestRunYara(UploadMixin, TestCase):
     @mock.patch('yara.compile')
     @mock.patch('olympia.scanners.tasks.statsd.incr')
     def test_throws_errors(self, incr_mock, yara_compile_mock):
-        yara_compile_mock.side_effect = Exception()
+        yara_compile_mock.side_effect = RuntimeError()
 
         # We use `_run_yara()` because `run_yara()` is decorated with
         # `@validation_task`, which gracefully handles exceptions.
-        with self.assertRaises(Exception):
+        with self.assertRaises(RuntimeError):
             _run_yara(self.results, self.upload.pk)
 
         assert incr_mock.called
@@ -647,7 +647,7 @@ class TestRunYaraQueryRule(TestCase):
         # Listed Webextension version belonging to mozilla disabled add-on.
         addon_factory(
             status=amo.STATUS_DISABLED, file_kw={'filename': 'webextension.xpi'}
-        ).current_version
+        )
         # Unlisted extension without a File instance
         Version.objects.create(
             addon=other_addon, channel=amo.CHANNEL_UNLISTED, version='42.42.42.42'

src/olympia/translations/tests/test_models.py

@@ -591,7 +591,7 @@ class TranslationMultiDbTests(TransactionTestCase):
         # that early as well.
         for con in django.db.connections:
             connections[con].cursor()
-            connections[con].mysql_version
+            connections[con].mysql_version  # noqa: B018
         reset_queries()
 
     @property