dev: Add E, PLC, and PLE ruff rules (#505)

database/models/core.py

@@ -297,9 +297,9 @@ def report(self):
             db_session.query(CommitReport)
             .filter(
                 (CommitReport.commit_id == self.id_)
-                & (CommitReport.code == None)
+                & (CommitReport.code == None)  # noqa: E711
                 & (
-                    (CommitReport.report_type == None)
+                    (CommitReport.report_type == None)  # noqa: E711
                     | (CommitReport.report_type == ReportType.COVERAGE.value)
                 )
             )
@@ -316,7 +316,7 @@ def commit_report(self, report_type: ReportType):
                 db_session.query(CommitReport)
                 .filter(
                     (CommitReport.commit_id == self.id_)
-                    & (CommitReport.code == None)
+                    & (CommitReport.code == None)  # noqa: E711
                     & (CommitReport.report_type == report_type.value)
                 )
                 .first()
@@ -483,7 +483,7 @@ def is_first_coverage_pull(self):
             self.repository.pulls.with_entities(
                 Pull.id_, Pull.commentid, Pull.bundle_analysis_commentid
             )
-            .filter(Pull.commentid != None)
+            .filter(Pull.commentid != None)  # noqa: E711
             .order_by(Pull.id_)
             .first()
         )

database/tests/unit/test_model_utils.py

@@ -71,7 +71,7 @@ def test_archive_getter_archive_field_set(self, db, mocker):
         commit = CommitFactory()
         test_class = self.ClassWithArchiveField(commit, None, "gcs_path")
 
-        assert test_class._archive_field == None
+        assert test_class._archive_field is None
         assert test_class._archive_field_storage_path == "gcs_path"
         assert test_class.archive_field == some_json
         mock_read_file.assert_called_with("gcs_path")
@@ -93,9 +93,9 @@ def test_archive_getter_file_not_in_storage(self, db, mocker):
         commit = CommitFactory()
         test_class = self.ClassWithArchiveField(commit, None, "gcs_path")
 
-        assert test_class._archive_field == None
+        assert test_class._archive_field is None
         assert test_class._archive_field_storage_path == "gcs_path"
-        assert test_class.archive_field == None
+        assert test_class.archive_field is None
         mock_read_file.assert_called_with("gcs_path")
         mock_archive_service.assert_called_with(repository=commit.repository)
 
@@ -122,7 +122,7 @@ def test_archive_setter_archive_field(self, db, mocker):
         mock_archive_service.return_value.write_json_data_to_storage = mock_write_file
 
         assert test_class._archive_field == "db_value"
-        assert test_class._archive_field_storage_path == None
+        assert test_class._archive_field_storage_path is None
         assert test_class.archive_field == "db_value"
         assert mock_read_file.call_count == 0
 
@@ -132,7 +132,7 @@ def test_archive_setter_archive_field(self, db, mocker):
 
         # Now we write to the property
         test_class.archive_field = some_json
-        assert test_class._archive_field == None
+        assert test_class._archive_field is None
         assert test_class._archive_field_storage_path == "path/to/written/object"
         assert test_class.archive_field == some_json
         # Cache is updated on write

helpers/checkpoint_logger/__init__.py

@@ -245,7 +245,9 @@ def _subflows() -> TSubflows:
             # We get our subflows in the form: [(metric, begin, end)]
             # We want them in the form: {end: [(metric, begin)]}
             # The first step of munging is to group by end
-            key_on_end = lambda x: x[2]
+            def key_on_end(x):
+                return x[2]
+
             sorted_by_end = sorted(args, key=key_on_end)
             grouped_by_end = itertools.groupby(args, key=key_on_end)
 

helpers/tests/pathmap/test_tree.py

@@ -41,7 +41,7 @@ def test_drill_multiple_possible_paths(self):
 
         branch = self.tree.instance.get("list.rs")
         results = []
-        assert self.tree._drill(branch, results) == None
+        assert self.tree._drill(branch, results) is None
 
     def test_recursive_lookup(self):
         path = "one/two/three.py"

ruff.toml

@@ -36,9 +36,10 @@ indent-width = 4
 target-version = "py312"
 
 [lint]
-# Currently only enabled for most F (Pyflakes) and I (isort) rules: https://docs.astral.sh/ruff/rules/
-select = ["F", "I"]
-ignore = ["F841", "F405", "F403"]
+# Currently only enabled for most F (Pyflakes), Pycodestyle (Error), 
+# PyLint (Convention, Error), and I (isort) rules: https://docs.astral.sh/ruff/rules/
+select = ["F", "E", "I", "PLC", "PLE"]
+ignore = ["F841", "F405", "F403", "E501", "E712"]
 
 # Allow fix for all enabled rules (when `--fix`) is provided.
 # The preferred method (for now) w.r.t. fixable rules is to manually update the makefile

services/commit_status.py

@@ -9,7 +9,7 @@ def _ci_providers() -> List[str]:
     providers = get_config("services", "ci_providers")
     if not providers:
         return []
-    elif type(providers) is list:
+    elif isinstance(providers, list):
         return providers
     else:
         return map(lambda p: p.strip(), providers.split(","))

services/github.py

@@ -29,7 +29,10 @@ def get_github_integration_token(
         raise RepositoryWithoutValidBotError()
 
 
-COMMIT_GHAPP_KEY_NAME = lambda commit_id: f"app_to_use_for_commit_{commit_id}"
+def COMMIT_GHAPP_KEY_NAME(commit_id):
+    return f"app_to_use_for_commit_{commit_id}"
+
+
 GHAPP_KEY_EXPIRY_SECONDS = 60 * 60 * 2  # 2h
 
 

services/notification/notifiers/mixins/message/__init__.py

@@ -225,7 +225,9 @@ async def write_section_to_msg(
             write("")
 
     def get_middle_layout_section_names(self, settings):
-        sections = map(lambda l: l.strip(), (settings["layout"] or "").split(","))
+        sections = map(
+            lambda layout: layout.strip(), (settings["layout"] or "").split(",")
+        )
         return [
             section
             for section in sections
@@ -242,7 +244,9 @@ def get_middle_layout_section_names(self, settings):
         ]
 
     def get_upper_section_names(self, settings):
-        sections = list(map(lambda l: l.strip(), (settings["layout"] or "").split(",")))
+        sections = list(
+            map(lambda layout: layout.strip(), (settings["layout"] or "").split(","))
+        )
         headers = ["newheader", "header", "condensed_header"]
         if all(x not in sections for x in headers):
             sections.insert(0, "condensed_header")

services/notification/notifiers/mixins/message/helpers.py

@@ -64,9 +64,9 @@ def make_metrics(before, after, relative, show_complexity, yaml, pull_url=None):
         complexity = " |" if show_complexity else ""
 
     else:
-        if type(before) is list:
+        if isinstance(before, list):
             before = ReportTotals(*before)
-        if type(after) is list:
+        if isinstance(after, list):
             after = ReportTotals(*after)
 
         layout = " `{absolute} <{relative}> ({impact})` |"

services/notification/notifiers/tests/unit/test_checks.py

@@ -297,7 +297,7 @@ async def test_checks_403_failure(
         assert fallback_notifier.title == "title"
         assert fallback_notifier.is_enabled() == True
         assert fallback_notifier.notification_type.value == "checks_patch"
-        assert fallback_notifier.decoration_type == None
+        assert fallback_notifier.decoration_type is None
 
         res = await fallback_notifier.notify(sample_comparison)
         fallback_notifier.store_results(sample_comparison, res)
@@ -306,7 +306,7 @@ async def test_checks_403_failure(
         assert fallback_notifier.title == "title"
         assert fallback_notifier.is_enabled() == True
         assert fallback_notifier.notification_type.value == "checks_patch"
-        assert fallback_notifier.decoration_type == None
+        assert fallback_notifier.decoration_type is None
         assert res == "success"
 
     @pytest.mark.asyncio
@@ -341,7 +341,7 @@ async def test_checks_failure(self, sample_comparison, mocker, mock_repo_provide
         assert fallback_notifier.title == "title"
         assert fallback_notifier.is_enabled() == True
         assert fallback_notifier.notification_type.value == "checks_patch"
-        assert fallback_notifier.decoration_type == None
+        assert fallback_notifier.decoration_type is None
 
         res = await fallback_notifier.notify(sample_comparison)
         assert res.notification_successful == False

services/notification/notifiers/tests/unit/test_status.py

@@ -718,7 +718,7 @@ def test_determine_status_check_behavior_to_apply(self, sample_comparison):
             notifier.determine_status_check_behavior_to_apply(
                 comparison, "flag_coverage_not_uploaded_behavior"
             )
-            == None
+            is None
         )
 
     def test_flag_coverage_was_uploaded_when_none_uploaded(

services/report/__init__.py

@@ -237,7 +237,7 @@ async def initialize_and_save_report(
             db_session.query(CommitReport)
             .filter_by(commit_id=commit.id_, code=report_code)
             .filter(
-                (CommitReport.report_type == None)
+                (CommitReport.report_type == None)  # noqa: E711
                 | (CommitReport.report_type == ReportType.COVERAGE.value)
             )
             .first()

services/report/languages/cobertura.py

@@ -125,7 +125,7 @@ def from_xml(xml, report_builder_session: ReportBuilderSession) -> Report:
                         ("parsers", "cobertura", "handle_missing_conditions"),
                         False,
                     ):
-                        if type(coverage) is str:
+                        if isinstance(coverage, str):
                             covered_conditions, total_conditions = coverage.split("/")
                             if len(conditions) < int(total_conditions):
                                 # <line number="23" hits="0" branch="true" condition-coverage="0% (0/2)">
@@ -150,7 +150,7 @@ def from_xml(xml, report_builder_session: ReportBuilderSession) -> Report:
                                 )
                     else:  # previous behaviour
                         if (
-                            type(coverage) is str
+                            isinstance(coverage, str)
                             and coverage[0] == "0"
                             and len(conditions) < int(coverage.split("/")[1])
                         ):
@@ -168,7 +168,7 @@ def from_xml(xml, report_builder_session: ReportBuilderSession) -> Report:
                     if conditions:
                         missing_branches = conditions
                 if (
-                    type(coverage) is str
+                    isinstance(coverage, str)
                     and not coverage[0] == "0"
                     and read_yaml_field(
                         repo_yaml,

services/report/languages/node.py

@@ -72,7 +72,7 @@ def get_location(node):
 
 
 def must_be_dict(value):
-    if type(value) is not dict:
+    if not isinstance(value, dict):
         return {}
     else:
         return value

services/report/languages/pycoverage.py

@@ -23,7 +23,7 @@ def matches_content(self, content, first_line, name) -> bool:
         )
 
     def _normalize_label(self, testname) -> str:
-        if type(testname) == int or type(testname) == float:
+        if isinstance(testname, int) or isinstance(testname, float):
             # This is from a compressed report.
             # Pull label from the labels_table
             # But the labels_table keys are strings, because of JSON format

services/report/languages/tests/unit/test_go.py

@@ -624,7 +624,7 @@ def test_combine_partials(self):
             [2, 24, 1],
             [24, None, 0],
         ]
-        assert go.combine_partials([(2, 2, 1), (2, 2, 0)]) == None
+        assert go.combine_partials([(2, 2, 1), (2, 2, 0)]) is None
         assert go.combine_partials([(0, None, 28), (0, None, 0)]) == [[0, None, 28]]
         assert go.combine_partials([(2, 35, 1), (35, None, 1)]) == [[2, None, 1]]
         assert go.combine_partials([(2, 35, "1/2"), (35, None, "1/2")]) == [

services/report/languages/tests/unit/test_xcode.py

@@ -16,7 +16,7 @@
     |   1|line
   1k|   2|line
          warning: The file '/Users/Jack/Documents/Coupgon/sdk-ios/Source/CPGCoupgonsViewController.swift' isn't covered.
-  \033[0;36m/file:\033[0m
+  \033\x1b[0;36m/file:\033[0m
   1m|   3|line
    1|   4|   }
 

services/report/languages/tests/unit/test_xcode2.py

@@ -16,7 +16,7 @@
     1|   |line
     2|   1k|line
            warning: The file '/Users/Jack/Documents/Coupgon/sdk-ios/Source/CPGCoupgonsViewController.swift' isn't covered.
-    \033[0;36m/file:\033[0m
+    \033\x1b[0;36m/file:\033[0m
     3|   1m|line
     4|   1|   }
 

services/report/languages/v1.py

@@ -32,7 +32,7 @@ def _list_to_dict(lines):
     in:  [None, 1] || {"1": 1}
     out: {"1": 1}
     """
-    if type(lines) is list:
+    if isinstance(lines, list):
         if len(lines) > 1:
             return dict(
                 [
@@ -53,7 +53,7 @@ def _list_to_dict(lines):
 
 
 def from_json(json, report_builder_session: ReportBuilderSession) -> Report:
-    if type(json["coverage"]) is dict:
+    if isinstance(json["coverage"], dict):
         # messages = json.get('messages', {})
         for fn, lns in json["coverage"].items():
             fn = report_builder_session.path_fixer(fn)

services/report/languages/xcode.py

@@ -15,7 +15,7 @@
 
 START_PARTIAL = "\033[0;41m"
 END_PARTIAL = "\033[0m"
-NAME_COLOR = "\033[0;36m"
+NAME_COLOR = "\033\x1b[0;36m"
 
 
 class XCodeProcessor(BaseLanguageProcessor):

services/report/raw_upload_processor.py

@@ -26,10 +26,12 @@
     SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label
 )
 
+
 # This is a lambda function to return different objects
-DEFAULT_LABEL_INDEX = lambda: {
-    SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label
-}
+def DEFAULT_LABEL_INDEX():
+    return {
+        SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_index: SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label
+    }
 
 
 def invert_pattern(string: str) -> str:
@@ -240,7 +242,7 @@ def make_sure_orginal_report_is_using_label_ids(original_report: Report) -> bool
         ] = SpecialLabelsEnum.CODECOV_ALL_LABELS_PLACEHOLDER.corresponding_label
 
     def possibly_translate_label(label_or_id: typing.Union[str, int]) -> int:
-        if type(label_or_id) == int:
+        if isinstance(label_or_id, int):
             return label_or_id
         if label_or_id in reverse_index_cache:
             return reverse_index_cache[label_or_id]

services/tests/test_github.py

@@ -75,17 +75,17 @@ def test_get_app_for_commit(self, mock_redis, dbsession):
         mock_redis.get.side_effect = lambda key: redis_keys.get(key)
         assert get_github_app_for_commit(fake_commit_12) == "1200"
         assert get_github_app_for_commit(fake_commit_10) == "1000"
-        assert get_github_app_for_commit(fake_commit_50) == None
+        assert get_github_app_for_commit(fake_commit_50) is None
         # This feature is Github-exclusive, so we skip checking for commits that are in repos of other providers
-        assert get_github_app_for_commit(fake_commit_gitlab) == None
+        assert get_github_app_for_commit(fake_commit_gitlab) is None
 
     def test_get_app_for_commit_error(self, mock_redis):
         repo_github = RepositoryFactory(owner__service="github")
         mock_redis.get.side_effect = RedisError
         fake_commit_12 = MagicMock(
             name="fake_commit", **{"id": 12, "repository": repo_github}
         )
-        assert get_github_app_for_commit(fake_commit_12) == None
+        assert get_github_app_for_commit(fake_commit_12) is None
         mock_redis.get.assert_called_with("app_to_use_for_commit_12")
 
     @pytest.mark.integration

services/tests/test_timeseries.py

@@ -552,7 +552,7 @@ def test_commit_measurement_insert_components(
             )
             .one_or_none()
         )
-        assert path_not_found_measurements == None
+        assert path_not_found_measurements is None
 
         empty_path_measurements = (
             dbsession.query(Measurement)
@@ -564,7 +564,7 @@ def test_commit_measurement_insert_components(
             )
             .one_or_none()
         )
-        assert empty_path_measurements == None
+        assert empty_path_measurements is None
 
     def test_commit_measurement_update_component(
         self, dbsession, sample_report_for_components, repository, mocker

tasks/backfill_commit_data_to_storage.py

@@ -82,7 +82,7 @@ def handle_all_report_rows(
             db_session.query(CommitReport)
             .filter_by(commit_id=commit.id_)
             .filter(
-                (CommitReport.report_type == None)
+                (CommitReport.report_type == None)  # noqa: E711
                 | (CommitReport.report_type == ReportType.COVERAGE.value)
             )
             .all()

tasks/backfill_existing_gh_app_installations.py

@@ -116,7 +116,7 @@ def run_impl(
                 extra=dict(ownerid=ownerid, parent_id=self.request.parent_id),
             )
             return {"successful": True, "reason": "backfill task finished"}
-        except:
+        except Exception:
             log.info(
                 "Backfill unsuccessful for this owner",
                 extra=dict(ownerid=ownerid, parent_id=self.request.parent_id),