report semgrep findings and fix findings assertions (#739)

* fix findings assertions

* semgrep rule should report findings

* fix int tests

* each sarif result should impl from_sarif

integration_tests/sonar/test_sonar_django_receiver_on_top.py

@@ -26,6 +26,7 @@ class TestDjangoReceiverOnTop(SonarIntegrationTest):
     )
     # fmt: on
 
-    expected_line_change = "7"
+    expected_line_change = "6"
     change_description = DjangoReceiverOnTopTransformer.change_description
     num_changed_files = 1
+    num_changes = 2

integration_tests/test_django_receiver_on_top.py

@@ -38,6 +38,7 @@ def foo():
     )
     # fmt: on
 
-    expected_line_change = "7"
+    expected_line_change = "6"
     change_description = DjangoReceiverOnTopTransformer.change_description
     num_changed_files = 1
+    num_changes = 2

src/codemodder/codemods/test/utils.py

@@ -225,5 +225,5 @@ def run_and_assert(
 
     def assert_findings(self, changes: list[Change]):
         assert all(
-            x.findings is not None for x in changes
+            x.findings for x in changes
         ), f"Expected all changes to have findings: {changes}"

src/codemodder/codeql.py

@@ -3,6 +3,7 @@
 
 from typing_extensions import Self
 
+from codemodder.codetf import Finding, Rule
 from codemodder.result import LineInfo, ResultSet, SarifLocation, SarifResult
 from codemodder.sarifs import AbstractSarifToolDetector
 
@@ -38,6 +39,31 @@ def from_sarif(cls, sarif_location) -> Self:
 class CodeQLResult(SarifResult):
     location_type = CodeQLLocation
 
+    @classmethod
+    def from_sarif(
+        cls, sarif_result, sarif_run, truncate_rule_id: bool = False
+    ) -> Self:
+        return cls(
+            rule_id=(
+                rule_id := cls.extract_rule_id(
+                    sarif_result, sarif_run, truncate_rule_id
+                )
+            ),
+            locations=cls.extract_locations(sarif_result),
+            codeflows=cls.extract_code_flows(sarif_result),
+            related_locations=cls.extract_related_locations(sarif_result),
+            finding_id=rule_id,
+            finding=Finding(
+                id=rule_id,
+                rule=Rule(
+                    id=rule_id,
+                    name=rule_id,
+                    # TODO: map to URL
+                    # url=,
+                ),
+            ),
+        )
+
 
 class CodeQLResultSet(ResultSet):
     @classmethod

src/codemodder/result.py

@@ -68,19 +68,19 @@ def match_location(self, pos: CodeRange, node: cst.CSTNode) -> bool:
 
 
 @dataclass(kw_only=True)
-class SarifResult(Result, ABCDataclass):
+class SASTResult(Result):
+    finding_id: str
+
+
+@dataclass(kw_only=True)
+class SarifResult(SASTResult, ABCDataclass):
     location_type: ClassVar[Type[SarifLocation]]
 
     @classmethod
     def from_sarif(
         cls, sarif_result, sarif_run, truncate_rule_id: bool = False
     ) -> Self:
-        return cls(
-            rule_id=cls.extract_rule_id(sarif_result, sarif_run, truncate_rule_id),
-            locations=cls.extract_locations(sarif_result),
-            codeflows=cls.extract_code_flows(sarif_result),
-            related_locations=cls.extract_related_locations(sarif_result),
-        )
+        raise NotImplementedError
 
     @classmethod
     def extract_locations(cls, sarif_result) -> list[Location]:
@@ -126,11 +126,6 @@ def extract_rule_id(cls, result, sarif_run, truncate_rule_id: bool = False) -> s
         raise ValueError("Could not extract rule id from sarif result.")
 
 
-@dataclass(kw_only=True)
-class SASTResult(Result):
-    finding_id: str
-
-
 def same_line(pos: CodeRange, location: Location) -> bool:
     return pos.start.line == location.start.line and pos.end.line == location.end.line
 

src/codemodder/semgrep.py

@@ -7,6 +7,7 @@
 
 from typing_extensions import Self, override
 
+from codemodder.codetf import Finding, Rule
 from codemodder.context import CodemodExecutionContext
 from codemodder.logging import logger
 from codemodder.result import LineInfo, Result, ResultSet, SarifLocation, SarifResult
@@ -43,6 +44,33 @@ def from_sarif(cls, sarif_location) -> Self:
 class SemgrepResult(SarifResult):
     location_type = SemgrepLocation
 
+    @classmethod
+    def from_sarif(
+        cls, sarif_result, sarif_run, truncate_rule_id: bool = False
+    ) -> Self:
+        # avoid circular import
+        from core_codemods.semgrep.api import semgrep_url_from_id
+
+        return cls(
+            rule_id=(
+                rule_id := cls.extract_rule_id(
+                    sarif_result, sarif_run, truncate_rule_id
+                )
+            ),
+            locations=cls.extract_locations(sarif_result),
+            codeflows=cls.extract_code_flows(sarif_result),
+            related_locations=cls.extract_related_locations(sarif_result),
+            finding_id=rule_id,
+            finding=Finding(
+                id=rule_id,
+                rule=Rule(
+                    id=rule_id,
+                    name=rule_id,
+                    url=semgrep_url_from_id(rule_id),
+                ),
+            ),
+        )
+
 
 class SemgrepResultSet(ResultSet):
     @classmethod

src/core_codemods/django_receiver_on_top.py

@@ -33,7 +33,8 @@ def leave_FunctionDef(
                 new_decorators.extend(
                     d for d in original_node.decorators if d != receiver
                 )
-                self.report_change(original_node)
+                for decorator in new_decorators:
+                    self.report_change(decorator)
                 return updated_node.with_changes(decorators=new_decorators)
         return updated_node
 

src/core_codemods/fix_assert_tuple.py

@@ -51,6 +51,7 @@ def _report_new_lines(
                 Change(
                     lineNumber=(line_number := start_line + idx),
                     description=self.change_description,
+                    # For now we can only link the finding to the first line changed
                     findings=self.file_context.get_findings_for_location(line_number),
                 )
             )

src/core_codemods/remove_assertion_in_pytest_raises.py

@@ -137,6 +137,7 @@ def leave_With(
                         body=[cst.SimpleStatementLine(body=[cst.Pass()])]
                     )
                 )
+            # TODO: need to report change for each line changed
             self.report_change(original_node)
             return cst.FlattenSentinel([new_with, *assert_stmts])
 

tests/codemods/sonar/test_sonar_break_or_continue_out_of_loop.py

@@ -14,11 +14,11 @@ def test_name(self):
         assert self.codemod.name == "break-or-continue-out-of-loop"
 
     def test_simple(self, tmpdir):
-        input_code = """
+        input_code = """\
         def f():
             continue
         """
-        expected = """
+        expected = """\
         def f():
             pass
         """

tests/codemods/sonar/test_sonar_django_model_without_dunder_str.py

@@ -15,14 +15,14 @@ def test_name(self):
         assert self.codemod.id == "sonar:python/django-model-without-dunder-str"
 
     def test_simple(self, tmpdir):
-        input_code = """
+        input_code = """\
         from django.db import models
                 
         class User(models.Model):
             name = models.CharField(max_length=100)
             phone = models.IntegerField(blank=True)
         """
-        expected = """
+        expected = """\
         from django.db import models
                 
         class User(models.Model):

tests/codemods/sonar/test_sonar_django_receiver_on_top.py

@@ -11,6 +11,11 @@ class TestSonarDjangoReceiverOnTop(BaseSASTCodemodTest):
     def test_name(self):
         assert self.codemod.name == "django-receiver-on-top"
 
+    def assert_findings(self, changes):
+        # For now we can only link the finding to the line with the receiver decorator
+        assert changes[0].findings
+        assert not changes[1].findings
+
     def test_simple(self, tmpdir):
         input_code = """
         from django.dispatch import receiver
@@ -43,4 +48,6 @@ def foo():
                 }
             ]
         }
-        self.run_and_assert(tmpdir, input_code, expected, results=json.dumps(issues))
+        self.run_and_assert(
+            tmpdir, input_code, expected, results=json.dumps(issues), num_changes=2
+        )

tests/codemods/sonar/test_sonar_fix_assert_tuple.py

@@ -11,6 +11,12 @@ class TestSonarFixAssertTuple(BaseSASTCodemodTest):
     def test_name(self):
         assert self.codemod.name == "fix-assert-tuple"
 
+    def assert_findings(self, changes):
+        # For now we can only link the finding to the first line changed
+        assert changes[0].findings
+        assert not changes[1].findings
+        assert not changes[2].findings
+
     def test_simple(self, tmpdir):
         input_code = """
         assert (1,2,3)

tests/codemods/sonar/test_sonar_remove_assertion_in_pytest_raises.py

@@ -13,6 +13,9 @@ class TestRemoveAssertionInPytestRaises(BaseSASTCodemodTest):
     def test_name(self):
         assert self.codemod.name == "remove-assertion-in-pytest-raises"
 
+    def assert_findings(self, changes):
+        assert not all(x.findings for x in changes)
+
     def test_simple(self, tmpdir):
         input_code = """
         import pytest

tests/codemods/test_django_receiver_on_top.py

@@ -25,7 +25,7 @@ def foo():
         def foo():
             pass
         """
-        self.run_and_assert(tmpdir, input_code, expected)
+        self.run_and_assert(tmpdir, input_code, expected, num_changes=2)
 
     def test_simple_alias(self, tmpdir):
         input_code = """
@@ -44,7 +44,7 @@ def foo():
         def foo():
             pass
         """
-        self.run_and_assert(tmpdir, input_code, expected)
+        self.run_and_assert(tmpdir, input_code, expected, num_changes=2)
 
     def test_no_receiver(self, tmpdir):
         input_code = """