review

Author: Swatinem

shared/reports/carryforward.py

@@ -37,34 +37,11 @@ def generate_carryforward_report(
     session_extras: Mapping[str, str] | None = None,
 ) -> Report:
     """
-        Generates a carriedforward report starting from report `report`, flags `flags`
-            and paths `paths`
+    Generates a carriedforward report by filtering the given `report` in-place,
+    to only those files and sessions matching the given `flags` and `paths`.
 
-    What this function does it basically take a report `report` and creates a new report
-        from it (so no changes are done in-place). On this new report, it adds all the information
-        from `report` that relates to sessions that have any of the flags `f`
-
-    This way, for example, if none of the sessions in `report` have a flag in `flags`,
-        it will just produce an empty report
-
-    If there are sessions with any of the flags in `flags`, let's call them `relevant_sessions`,
-        this function will go through all files in `report` that match any of the paths `paths
-        and build a new 'carriedforward' ReportFile from it, with only the ReportLines
-        that had at least one LineSession among the `relevant_sessions` (and proper filter out
-        all the the other sessions from that line). Then all the new EditableReportFile will
-        be added to the report.
-
-    Also, the old sessions are copied over to the new report, with their numbering changed to match
-        the new session order they are in now (they could be the fifth session before,
-        and the first session now)
-
-    Args:
-        report (Report): Description
-        flags (Sequence[str]): Description
-
-    Returns:
-        EditableReport: A new report with only the info related to `flags` on it, as described above
-    """
+    The sessions that are matching the `flags` are being flagged as `carriedforward`,
+    and other sessions are removed from the report."""
     if paths:
         matcher = Matcher(paths)
         files_to_delete = {

shared/reports/filtered.py

@@ -165,9 +165,9 @@ def network(self):
             if file:
                 yield fname, make_network_file(file.totals)
 
-    def get(self, filename, _else=None):
+    def get(self, filename):
         if not self.should_include(filename):
-            return _else
+            return None
         if not self.flags:
             return self.report.get(filename)
         r = self.report.get(filename)

shared/reports/readonly.py

@@ -115,10 +115,6 @@ def get_flag_names(self) -> list[str]:
     def sessions(self):
         return self.inner_report.sessions
 
-    @property
-    def size(self):
-        return self.inner_report.size
-
     def apply_diff(self, *args, **kwargs):
         return self.inner_report.apply_diff(*args, **kwargs)
 

shared/reports/resources.py

@@ -173,15 +173,6 @@ def lookup_label_by_id(self, label_id: int) -> str:
     def from_chunks(cls, *args, **kwargs):
         return cls(*args, **kwargs)
 
-    @property
-    def size(self):
-        # TODO: this `size` is only used for internal metrics and an ATS announcement,
-        # so we can work towards just removing this altogether.
-        size = 0
-        for file in self:
-            size += len(file._lines)
-        return size
-
     def has_precalculated_totals(self):
         return self._totals is not None
 
@@ -261,7 +252,7 @@ def append(self, _file, joined=True):
         self._invalidate_caches()
         return True
 
-    def get(self, filename, **kwargs):
+    def get(self, filename):
         return self._files.get(filename)
 
     def resolve_paths(self, paths: list[tuple[str, str | None]]):
@@ -516,7 +507,7 @@ def does_diff_adjust_tracked_lines(self, diff, future_report, future_diff):
                     in_future = future_state != "deleted" and path in future_report
                     if in_past and in_future:
                         # get the future version
-                        future_file = future_report.get(path, bind=False)
+                        future_file = future_report.get(path)
                         # if modified
                         if future_state == "modified":
                             # shift the lines to "guess" what C was

tests/benchmarks/test_report.py

@@ -40,13 +40,9 @@ def do_parse(raw_report_json, raw_chunks):
 def do_full_parse(raw_report_json, raw_chunks):
     report = do_parse(raw_report_json, raw_chunks)
     # full parsing in this case means iterating over everything in the report
-    # we iterate over `files` and use `get(bind=True)`, as that is caching the
-    # parsed `ReportFile` instance.
-    # the `__iter__` method does not do that.
-    for filename in report.files:
-        file = report.get(filename, bind=True)
-        # … however the same is not true for the `ReportLine`s, which are being
-        # re-parsed *every damn time* :-(
+    for file in report:
+        # Which in particular means iterating over every `ReportLine`s,
+        # which are unfortunately being re-parsed *every damn time* :-(
         for _line in file:
             pass
 

tests/unit/reports/test_readonly.py

@@ -427,9 +427,6 @@ def test_get(self, sample_rust_report):
             diff=0,
         )
 
-    def test_size(self, sample_rust_report):
-        assert sample_rust_report.size == 34
-
     def test_differing_totals_calculation(self, mocker, sample_report):
         rust_analyzer = mocker.MagicMock(
             get_totals=mocker.MagicMock(return_value=ReportTotals(1, 999))