Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fixed findings metadata to align with CodeTF spec #941

Merged
merged 3 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/codemodder/codemods/imported_call_modifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def leave_Call(self, original_node: cst.Call, updated_node: cst.Call):
Change(
lineNumber=line_number,
description=self.change_description,
findings=self.file_context.get_findings_for_location(
fixedFindings=self.file_context.get_findings_for_location(
line_number
),
)
Expand Down
2 changes: 1 addition & 1 deletion src/codemodder/codemods/libcst_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def report_change_for_line(
Change(
lineNumber=line_number,
description=description or self.change_description,
findings=findings
fixedFindings=findings
or self.file_context.get_findings_for_location(line_number),
)
)
Expand Down
4 changes: 2 additions & 2 deletions src/codemodder/codemods/regex_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _apply(self, original_lines, file_context, results):
Change(
lineNumber=lineno + 1,
description=self.change_description,
findings=file_context.get_findings_for_location(lineno),
fixedFindings=file_context.get_findings_for_location(lineno),
)
)
return changes, updated_lines
Expand Down Expand Up @@ -110,7 +110,7 @@ def _apply(self, original_lines, file_context, results):
Change(
lineNumber=lineno + 1,
description=self.change_description,
findings=file_context.get_findings_for_location(lineno),
fixedFindings=file_context.get_findings_for_location(lineno),
)
)

Expand Down
2 changes: 1 addition & 1 deletion src/codemodder/codemods/test/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,5 @@ def run_and_assert(

def assert_findings(self, changes: list[Change]):
assert all(
x.findings for x in changes
x.fixedFindings for x in changes
), f"Expected all changes to have findings: {changes}"
2 changes: 1 addition & 1 deletion src/codemodder/codemods/xml_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def add_change(self, line):
Change(
lineNumber=line,
description=self.change_description or None,
findings=self.file_context.get_findings_for_location(line),
fixedFindings=self.file_context.get_findings_for_location(line),
)
)

Expand Down
7 changes: 5 additions & 2 deletions src/codemodder/codetf.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class Change(BaseModel):
diffSide: DiffSide = DiffSide.RIGHT
properties: Optional[dict] = None
packageActions: Optional[list[PackageAction]] = None
findings: Optional[list[Finding]] = None
fixedFindings: Optional[list[Finding]] = None

@model_validator(mode="after")
def validate_lineNumber(self):
Expand All @@ -83,7 +83,7 @@ def with_findings(self, findings: list[Finding] | None) -> Change:
diffSide=self.diffSide,
properties=self.properties,
packageActions=self.packageActions,
findings=findings,
fixedFindings=findings,
)


Expand All @@ -108,6 +108,8 @@ class ChangeSet(BaseModel):
ai: Optional[AIMetadata] = None
strategy: Optional[Strategy] = None
provisional: Optional[bool] = False
# For fixed findings that are not associated with a specific change
fixedFindings: Optional[list[Finding]] = None

def with_changes(self, changes: list[Change]) -> ChangeSet:
return ChangeSet(
Expand All @@ -117,6 +119,7 @@ def with_changes(self, changes: list[Change]) -> ChangeSet:
ai=self.ai,
strategy=self.strategy,
provisional=self.provisional,
fixedFindings=self.fixedFindings,
)


Expand Down
22 changes: 12 additions & 10 deletions src/codemodder/file_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,22 @@ def add_unfixed_findings(
]
)

def get_findings_for_location(self, line_number: int):
def get_findings_for_location(self, line_number: int) -> list[Finding]:
return [
result.finding
for result in (self.results or [])
if any(
location.start.line <= line_number <= location.end.line
for location in result.locations
)
or any(
location.start.line <= line_number <= location.end.line
for codeflow in result.codeflows
for location in codeflow
if result.finding is not None
and (
any(
location.start.line <= line_number <= location.end.line
for location in result.locations
)
or any(
location.start.line <= line_number <= location.end.line
for codeflow in result.codeflows
for location in codeflow
)
)
and result.finding is not None
]

def match_findings(self, line_numbers):
Expand Down
2 changes: 1 addition & 1 deletion src/codemodder/utils/update_finding_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def update_finding_metadata(
if finding.rule.id in tool_rule_map
else finding
)
for finding in change.findings or []
for finding in change.fixedFindings or []
]
or None
)
Expand Down
2 changes: 1 addition & 1 deletion src/core_codemods/file_resource_leak.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def _handle_block(
Change(
lineNumber=line_number,
description=FileResourceLeakTransformer.change_description,
findings=self.file_context.get_findings_for_location(
fixedFindings=self.file_context.get_findings_for_location(
line_number
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ def test_yaml_load(self, tmpdir):
)

assert changes is not None
assert changes[0].changes[0].findings is not None
assert changes[0].changes[0].findings[0].id == "1"
assert changes[0].changes[0].findings[0].rule.id == RULE_ID
assert changes[0].changes[0].fixedFindings is not None
assert changes[0].changes[0].fixedFindings[0].id == "1"
assert changes[0].changes[0].fixedFindings[0].rule.id == RULE_ID

@mock.patch("codemodder.codemods.api.FileContext.add_dependency")
def test_pickle_load(self, adds_dependency, tmpdir):
Expand Down Expand Up @@ -80,9 +80,9 @@ def test_pickle_load(self, adds_dependency, tmpdir):
adds_dependency.assert_called_once_with(Fickling)

assert changes is not None
assert changes[0].changes[0].findings is not None
assert changes[0].changes[0].findings[0].id == "2"
assert changes[0].changes[0].findings[0].rule.id == RULE_ID
assert changes[0].changes[0].fixedFindings is not None
assert changes[0].changes[0].fixedFindings[0].id == "2"
assert changes[0].changes[0].fixedFindings[0].rule.id == RULE_ID

@mock.patch("codemodder.codemods.api.FileContext.add_dependency")
def test_pickle_and_yaml(self, adds_dependency, tmpdir):
Expand Down Expand Up @@ -128,12 +128,12 @@ def test_pickle_and_yaml(self, adds_dependency, tmpdir):
adds_dependency.assert_called_once_with(Fickling)

assert changes is not None
assert changes[0].changes[0].findings is not None
assert changes[0].changes[0].findings[0].id == "4"
assert changes[0].changes[0].findings[0].rule.id == RULE_ID
assert changes[0].changes[1].findings is not None
assert changes[0].changes[1].findings[0].id == "3"
assert changes[0].changes[1].findings[0].rule.id == RULE_ID
assert changes[0].changes[0].fixedFindings is not None
assert changes[0].changes[0].fixedFindings[0].id == "4"
assert changes[0].changes[0].fixedFindings[0].rule.id == RULE_ID
assert changes[0].changes[1].fixedFindings is not None
assert changes[0].changes[1].fixedFindings[0].id == "3"
assert changes[0].changes[1].fixedFindings[0].rule.id == RULE_ID

@mock.patch("codemodder.codemods.api.FileContext.add_dependency")
def test_pickle_loads(self, adds_dependency, tmpdir):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ def test_simple(self, tmpdir):
)

assert changes is not None
assert changes[0].changes[0].findings is not None
assert changes[0].changes[0].findings[0].id == "1"
assert changes[0].changes[0].fixedFindings is not None
assert changes[0].changes[0].fixedFindings[0].id == "1"
assert (
changes[0].changes[0].findings[0].rule.id
changes[0].changes[0].fixedFindings[0].rule.id
== "python.django.security.audit.secure-cookies.django-secure-set-cookie"
)
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,6 @@ def f():
expexted_output,
results=json.dumps(results),
)
assert len(changes[0].changes[0].findings) == len(results["runs"][0]["results"])
assert len(changes[0].changes[0].fixedFindings) == len(
results["runs"][0]["results"]
)
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def foo(request):
tmpdir, input_code, expected, results=json.dumps(issues)
)
assert changes is not None
assert changes[0].changes[0].findings is not None
assert changes[0].changes[0].findings[0].id == rule_id
assert changes[0].changes[0].findings[0].rule.id == rule_id
assert changes[0].changes[0].findings[0].rule.name == rule_id
assert changes[0].changes[0].fixedFindings is not None
assert changes[0].changes[0].fixedFindings[0].id == rule_id
assert changes[0].changes[0].fixedFindings[0].rule.id == rule_id
assert changes[0].changes[0].fixedFindings[0].rule.name == rule_id
4 changes: 2 additions & 2 deletions tests/codemods/sonar/test_sonar_django_receiver_on_top.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def test_name(self):

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
assert changes[0].fixedFindings
assert not changes[1].fixedFindings

def test_simple(self, tmpdir):
input_code = """
Expand Down
6 changes: 3 additions & 3 deletions tests/codemods/sonar/test_sonar_fix_assert_tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ def test_name(self):

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
assert changes[0].fixedFindings
assert not changes[1].fixedFindings
assert not changes[2].fixedFindings

def test_simple(self, tmpdir):
input_code = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ 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)
assert not all(x.fixedFindings for x in changes)

def test_simple(self, tmpdir):
input_code = """
Expand Down
16 changes: 8 additions & 8 deletions tests/test_update_finding_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ def test_update_finding_metdata():
Change(
lineNumber=1,
description="foo",
findings=[
fixedFindings=[
Finding(id="rule_id", rule=Rule(id="rule_id", name="other_name"))
],
),
Change(
lineNumber=2,
description="bar",
findings=[
fixedFindings=[
Finding(id="other_id", rule=Rule(id="other_id", name="other_name"))
],
),
Expand All @@ -34,10 +34,10 @@ def test_update_finding_metdata():
tool_rules=[tool_rule], changesets=[changeset]
)

assert new_changesets[0].changes[0].findings
assert new_changesets[0].changes[0].findings[0].rule.name == "rule_name"
assert new_changesets[0].changes[0].findings[0].rule.url == "rule_url"
assert new_changesets[0].changes[1].findings
assert new_changesets[0].changes[1].findings[0].rule.name == "other_name"
assert new_changesets[0].changes[1].findings[0].rule.url is None
assert new_changesets[0].changes[0].fixedFindings
assert new_changesets[0].changes[0].fixedFindings[0].rule.name == "rule_name"
assert new_changesets[0].changes[0].fixedFindings[0].rule.url == "rule_url"
assert new_changesets[0].changes[1].fixedFindings
assert new_changesets[0].changes[1].fixedFindings[0].rule.name == "other_name"
assert new_changesets[0].changes[1].fixedFindings[0].rule.url is None
assert new_changesets[0].changes[2] == changeset.changes[2]
Loading