From 33e55903878ec9f1e0bb4a0ce56bc07a373b4d03 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 24 Jul 2024 09:44:24 -0300 Subject: [PATCH 1/4] fix findings assertions --- src/codemodder/codemods/test/utils.py | 2 +- src/core_codemods/django_receiver_on_top.py | 3 ++- src/core_codemods/fix_assert_tuple.py | 1 + src/core_codemods/remove_assertion_in_pytest_raises.py | 1 + .../sonar/test_sonar_break_or_continue_out_of_loop.py | 4 ++-- .../sonar/test_sonar_django_model_without_dunder_str.py | 4 ++-- .../codemods/sonar/test_sonar_django_receiver_on_top.py | 9 ++++++++- tests/codemods/sonar/test_sonar_fix_assert_tuple.py | 6 ++++++ .../test_sonar_remove_assertion_in_pytest_raises.py | 3 +++ tests/codemods/test_django_receiver_on_top.py | 4 ++-- 10 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/codemodder/codemods/test/utils.py b/src/codemodder/codemods/test/utils.py index 44246c51..59242a5d 100644 --- a/src/codemodder/codemods/test/utils.py +++ b/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}" diff --git a/src/core_codemods/django_receiver_on_top.py b/src/core_codemods/django_receiver_on_top.py index 62c1699b..26aad7be 100644 --- a/src/core_codemods/django_receiver_on_top.py +++ b/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 diff --git a/src/core_codemods/fix_assert_tuple.py b/src/core_codemods/fix_assert_tuple.py index d0c4a791..2209e733 100644 --- a/src/core_codemods/fix_assert_tuple.py +++ b/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), ) ) diff --git a/src/core_codemods/remove_assertion_in_pytest_raises.py b/src/core_codemods/remove_assertion_in_pytest_raises.py index 9c7aca47..6067d10f 100644 --- a/src/core_codemods/remove_assertion_in_pytest_raises.py +++ b/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]) diff --git a/tests/codemods/sonar/test_sonar_break_or_continue_out_of_loop.py b/tests/codemods/sonar/test_sonar_break_or_continue_out_of_loop.py index 2dd9a760..4e07a4ae 100644 --- a/tests/codemods/sonar/test_sonar_break_or_continue_out_of_loop.py +++ b/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 """ diff --git a/tests/codemods/sonar/test_sonar_django_model_without_dunder_str.py b/tests/codemods/sonar/test_sonar_django_model_without_dunder_str.py index 7095509c..d4074ae2 100644 --- a/tests/codemods/sonar/test_sonar_django_model_without_dunder_str.py +++ b/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): diff --git a/tests/codemods/sonar/test_sonar_django_receiver_on_top.py b/tests/codemods/sonar/test_sonar_django_receiver_on_top.py index 9174d451..513906ce 100644 --- a/tests/codemods/sonar/test_sonar_django_receiver_on_top.py +++ b/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 + ) diff --git a/tests/codemods/sonar/test_sonar_fix_assert_tuple.py b/tests/codemods/sonar/test_sonar_fix_assert_tuple.py index a766c9f2..3d371d13 100644 --- a/tests/codemods/sonar/test_sonar_fix_assert_tuple.py +++ b/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) diff --git a/tests/codemods/sonar/test_sonar_remove_assertion_in_pytest_raises.py b/tests/codemods/sonar/test_sonar_remove_assertion_in_pytest_raises.py index e81d31b8..ca9eae0a 100644 --- a/tests/codemods/sonar/test_sonar_remove_assertion_in_pytest_raises.py +++ b/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 diff --git a/tests/codemods/test_django_receiver_on_top.py b/tests/codemods/test_django_receiver_on_top.py index 69fb7b92..3fccfc10 100644 --- a/tests/codemods/test_django_receiver_on_top.py +++ b/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 = """ From b653fd8dbd44c8283fd4b119ecab40b1492f08c7 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 24 Jul 2024 11:15:23 -0300 Subject: [PATCH 2/4] semgrep rule should report findings --- src/codemodder/result.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/codemodder/result.py b/src/codemodder/result.py index d6bbdea3..e0c1f109 100644 --- a/src/codemodder/result.py +++ b/src/codemodder/result.py @@ -9,7 +9,7 @@ from libcst._position import CodeRange from typing_extensions import Self -from codemodder.codetf import Finding +from codemodder.codetf import Finding, Rule from .utils.abc_dataclass import ABCDataclass @@ -68,18 +68,39 @@ 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: + # avoid circular import + from core_codemods.semgrep.api import semgrep_url_from_id + return cls( - rule_id=cls.extract_rule_id(sarif_result, sarif_run, truncate_rule_id), + 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), + ), + ), ) @classmethod @@ -126,11 +147,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 From 0c297f1f7a6a71cccd0e8be9afe187cbc3786088 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Wed, 24 Jul 2024 11:22:53 -0300 Subject: [PATCH 3/4] fix int tests --- integration_tests/sonar/test_sonar_django_receiver_on_top.py | 3 ++- integration_tests/test_django_receiver_on_top.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/integration_tests/sonar/test_sonar_django_receiver_on_top.py b/integration_tests/sonar/test_sonar_django_receiver_on_top.py index 3ed29942..dff0de29 100644 --- a/integration_tests/sonar/test_sonar_django_receiver_on_top.py +++ b/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 diff --git a/integration_tests/test_django_receiver_on_top.py b/integration_tests/test_django_receiver_on_top.py index ea36f67b..01f322da 100644 --- a/integration_tests/test_django_receiver_on_top.py +++ b/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 From b5951f2368792f7d9cc0933ca9d7d1a34c2f6e88 Mon Sep 17 00:00:00 2001 From: clavedeluna Date: Thu, 25 Jul 2024 10:41:05 -0300 Subject: [PATCH 4/4] each sarif result should impl from_sarif --- src/codemodder/codeql.py | 26 ++++++++++++++++++++++++++ src/codemodder/result.py | 25 ++----------------------- src/codemodder/semgrep.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/codemodder/codeql.py b/src/codemodder/codeql.py index cbfe8cfb..041ed3e3 100644 --- a/src/codemodder/codeql.py +++ b/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 diff --git a/src/codemodder/result.py b/src/codemodder/result.py index e0c1f109..8dd66c55 100644 --- a/src/codemodder/result.py +++ b/src/codemodder/result.py @@ -9,7 +9,7 @@ from libcst._position import CodeRange from typing_extensions import Self -from codemodder.codetf import Finding, Rule +from codemodder.codetf import Finding from .utils.abc_dataclass import ABCDataclass @@ -80,28 +80,7 @@ class SarifResult(SASTResult, ABCDataclass): 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), - ), - ), - ) + raise NotImplementedError @classmethod def extract_locations(cls, sarif_result) -> list[Location]: diff --git a/src/codemodder/semgrep.py b/src/codemodder/semgrep.py index d92b79ca..84a3293b 100644 --- a/src/codemodder/semgrep.py +++ b/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