Repository: Bachmann1234/diff_cover

Issue:
Add support for report generator plugins that process modified files as a batch, rather than individually
SQLFluff implements the `diff-quality` plugin protocol. SQLFluff users have requested that the `diff-quality` integration should take advantage of SQLFluff's multiprocessing capability (processing files in parallel across multiple processes).

In order to do this, `BaseReportGenerator._diff_violations()` would need to be modified to call a different function if provided by the plugin, e.g. `violations_batch()` rather than `violations()`. If `violations_batch()` is not implemented, fall back to using `violations()`.

I'm happy to provide a PR to implement this, but wanted to ask if this is a reasonable feature and if there are any concerns with the proposed implementation. 🙏🏽

Discussion:
Bachmann1234: I'll look at this closely this week. Sorry I did not get to it yet
barrywhart: No problem! It's a roadmap item for us, not urgent. Hope you had a great holiday (if you're in the U.S.).
barrywhart: Ready for another review, I think.
Bachmann1234: Ok. Think this can work. I think if you just update from main (and just remove the change to `pyproject.toml` ill do that when I cut the release) ill go ahead and merge this
Bachmann1234: wait, im an admin, what am I doing. Illl fix the conflict
Bachmann1234: @barrywhart I realized I should probably get a final "this is ready" from you before I merge and release. I tend to be a bit trigger happy with the release button. So just leave me a note and ill get this out asap (likely the evening I see it)
barrywhart: I removed a few lines of commented-out code. This is ready to merge once the build passes.

Thanks for the review! SQLFluff users will appreciate the speedier performance once we update it to take advantage of this. 🙏

Patch:
diff --git a/README.rst b/README.rst
index f184bc5..6c8180b 100644
--- a/README.rst
+++ b/README.rst
@@ -64,7 +64,8 @@ To install the development version:
 
     git clone https://github.com/Bachmann1234/diff-cover.git
     cd diff-cover
-    python setup.py install
+    poetry install
+    poetry shell
 
 
 Getting Started
diff --git a/diff_cover/report_generator.py b/diff_cover/report_generator.py
index b77a182..35fa06f 100644
--- a/diff_cover/report_generator.py
+++ b/diff_cover/report_generator.py
@@ -172,15 +172,29 @@ class BaseReportGenerator(ABC):
 
         To make this efficient, we cache and reuse the result.
         """
+        src_paths_changed = self._diff.src_paths_changed()
         if not self._diff_violations_dict:
-            self._diff_violations_dict = {
-                src_path: DiffViolations(
-                    self._violations.violations(src_path),
-                    self._violations.measured_lines(src_path),
-                    self._diff.lines_changed(src_path),
+            try:
+                violations = self._violations.violations_batch(
+                    src_paths_changed
                 )
-                for src_path in self._diff.src_paths_changed()
-            }
+                self._diff_violations_dict = {
+                    src_path: DiffViolations(
+                        violations.get(src_path, []),
+                        self._violations.measured_lines(src_path),
+                        self._diff.lines_changed(src_path),
+                    )
+                for src_path in src_paths_changed
+                }
+            except NotImplementedError:
+                self._diff_violations_dict = {
+                    src_path: DiffViolations(
+                        self._violations.violations(src_path),
+                        self._violations.measured_lines(src_path),
+                        self._diff.lines_changed(src_path),
+                    )
+                for src_path in src_paths_changed
+                }
         return self._diff_violations_dict
 
     def report_dict(self):
diff --git a/diff_cover/violationsreporters/base.py b/diff_cover/violationsreporters/base.py
index f0d7000..3f1a6ca 100644
--- a/diff_cover/violationsreporters/base.py
+++ b/diff_cover/violationsreporters/base.py
@@ -34,6 +34,19 @@ class BaseViolationReporter(ABC):
         Return a list of Violations recorded in `src_path`.
         """
 
+    def violations_batch(self, src_paths):
+        """
+        Return a dict of Violations recorded in `src_paths`.
+
+        src_paths: Sequence[str] - sequence of paths to source files
+
+        Returns a Dict[str, List[Violation]]. Keys are paths to source files.
+
+        If a subclass does not implement this function, violations() will be
+        called instead, once for each src_path in src_paths.
+        """
+        raise NotImplementedError
+
     def measured_lines(self, src_path):
         """
         Return a list of the lines in src_path that were measured


Test patch:
diff --git a/tests/test_report_generator.py b/tests/test_report_generator.py
index 49b92ed..97d81b9 100644
--- a/tests/test_report_generator.py
+++ b/tests/test_report_generator.py
@@ -1,5 +1,6 @@
 # pylint: disable=attribute-defined-outside-init,not-callable
 
+import copy
 import json
 from io import BytesIO
 from textwrap import dedent
@@ -53,7 +54,9 @@ class BaseReportGeneratorTest:
     @pytest.fixture(autouse=True)
     def base_setup(self, mocker):
         # Create mocks of the dependencies
-        self.coverage = mocker.MagicMock(BaseViolationReporter)
+        self.coverage = mocker.MagicMock(
+            BaseViolationReporter,
+        )
         self.diff = mocker.MagicMock(BaseDiffReporter)
 
         # Patch snippet loading to always return the same string
@@ -81,6 +84,8 @@ class BaseReportGeneratorTest:
         self._violations_dict = dict()
         self.coverage.violations.side_effect = self._violations_dict.get
 
+        self.coverage.violations_batch.side_effect = NotImplementedError
+
         self._measured_dict = dict()
         self.coverage.measured_lines.side_effect = self._measured_dict.get
 
@@ -539,3 +544,26 @@ class TestMarkdownReportGenerator(BaseReportGeneratorTest):
         # Verify that we got the expected string
         expected = load_fixture("markdown_report_two_snippets.md").strip()
         self.assert_report(expected)
+
+
+class TestSimpleReportGeneratorWithBatchViolationReporter(BaseReportGeneratorTest):
+    REPORT_GENERATOR_CLASS = SimpleReportGenerator
+
+    @pytest.fixture(autouse=True)
+    def setup(self):
+        self.use_default_values()
+        # Have violations_batch() return the violations.
+        self.coverage.violations_batch.side_effect = None
+        self.coverage.violations_batch.return_value = copy.deepcopy(
+            self._violations_dict
+        )
+        # Have violations() return an empty list to ensure violations_batch()
+        # is used.
+        for src in self.SRC_PATHS:
+            self.set_violations(src, [])
+
+    def test_violation_lines(self):
+        # By construction, each file has the same coverage information
+        expected = [10, 11]
+        for src_path in self.SRC_PATHS:
+            assert self.report.violation_lines(src_path) == expected