Skip to content

Commit 5066ddf

Browse files
Seppli11thomas-serre-sonarsource
authored andcommitted
SONARPY-3844 Create ruling-diff-comment action (#895)
Co-authored-by: Thomas Serre <118730793+thomas-serre-sonarsource@users.noreply.github.com> GitOrigin-RevId: 45dc70b3e7bfc393134a26919308e5f888ef1569
1 parent 326240c commit 5066ddf

File tree

12 files changed

+1514
-0
lines changed

12 files changed

+1514
-0
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
name: 'Ruling Diff Comment'
2+
description: 'Posts a human-readable summary of ruling file changes on PRs'
3+
4+
inputs:
5+
pr-number:
6+
description: 'Pull request number'
7+
required: true
8+
repository:
9+
description: 'owner/repo'
10+
required: true
11+
base-sha:
12+
description: 'Base commit SHA for diff'
13+
required: true
14+
head-sha:
15+
description: 'Head commit SHA for diff'
16+
required: true
17+
18+
runs:
19+
using: 'composite'
20+
steps:
21+
- name: Run unit tests
22+
shell: bash
23+
run: uv run --project "${{ github.action_path }}" python -m unittest discover -v -s "${{ github.action_path }}" -p "test_ruling_diff.py"
24+
25+
- name: Generate and post ruling diff comment
26+
shell: bash
27+
env:
28+
GH_TOKEN: ${{ env.GH_TOKEN }}
29+
PR_NUMBER: ${{ inputs.pr-number }}
30+
REPOSITORY: ${{ inputs.repository }}
31+
BASE_SHA: ${{ inputs.base-sha }}
32+
HEAD_SHA: ${{ inputs.head-sha }}
33+
run: |
34+
uv run --project "${{ github.action_path }}" python "${{ github.action_path }}/ruling_diff.py" \
35+
--pr-number "$PR_NUMBER" \
36+
--repository "$REPOSITORY" \
37+
--base-sha "$BASE_SHA" \
38+
--head-sha "$HEAD_SHA"
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[project]
2+
name = "ruling-diff-comment"
3+
version = "0.1.0"
4+
description = "GitHub Action helper for ruling diff comments"
5+
requires-python = ">=3.10"
6+
dependencies = []
7+
8+
[tool.uv]
9+
package = false
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
from __future__ import annotations
2+
3+
import argparse
4+
import logging
5+
import os
6+
import sys
7+
8+
from ruling_diff_core import build_rule_diffs, format_comment
9+
from ruling_diff_io import (
10+
GitHubActionIO,
11+
get_changed_ruling_files,
12+
post_or_update_comment,
13+
)
14+
15+
16+
def configure_logging() -> None:
17+
level = logging.DEBUG if os.environ.get("RUNNER_DEBUG") else logging.INFO
18+
logging.basicConfig(level=level, format="%(asctime)s %(levelname)s %(message)s")
19+
20+
21+
def parse_args() -> argparse.Namespace:
22+
parser = argparse.ArgumentParser(
23+
description="Generate and post ruling diff comment"
24+
)
25+
parser.add_argument("--pr-number", required=True)
26+
parser.add_argument("--repository", required=True)
27+
parser.add_argument("--base-sha", required=True)
28+
parser.add_argument("--head-sha", required=True)
29+
args = parser.parse_args()
30+
if "/" not in args.repository:
31+
raise ValueError("--repository must be in owner/repo format")
32+
return args
33+
34+
35+
def has_required_context(args: argparse.Namespace) -> bool:
36+
return bool(
37+
args.pr_number.strip() and args.base_sha.strip() and args.head_sha.strip()
38+
)
39+
40+
41+
def main() -> None:
42+
configure_logging()
43+
args = parse_args()
44+
if not has_required_context(args):
45+
logging.info("Missing pr/base/head arguments. Skipping ruling diff comment.")
46+
return
47+
48+
changed_files = get_changed_ruling_files(args.base_sha, args.head_sha)
49+
if not changed_files:
50+
logging.info("No changed ruling json files found. Nothing to do.")
51+
return
52+
53+
logging.info("Found %d changed ruling json files", len(changed_files))
54+
io = GitHubActionIO()
55+
rule_diffs = build_rule_diffs(
56+
changed_files,
57+
args.base_sha,
58+
args.head_sha,
59+
io,
60+
)
61+
if not rule_diffs:
62+
logging.info("Changed files have no issue deltas. No comment will be posted.")
63+
return
64+
65+
comment = format_comment(rule_diffs)
66+
post_or_update_comment(args.pr_number, args.repository, comment)
67+
68+
69+
if __name__ == "__main__":
70+
try:
71+
main()
72+
except Exception as exc:
73+
logging.error("Failed to generate ruling diff comment: %s", exc)
74+
sys.exit(1)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
from ruling_diff_core_lib.comment_rendering import format_comment
2+
from ruling_diff_core_lib.models_and_constants import (
3+
COMMENT_MARKER,
4+
COMMENT_SOFT_LIMIT,
5+
EXPECTED_RULING_ROOT,
6+
IssueDiff,
7+
RuleDiff,
8+
Snippet,
9+
)
10+
from ruling_diff_core_lib.ruling_diff_logic import (
11+
build_rule_diffs,
12+
diff_ruling_jsons,
13+
parse_ruling_path,
14+
parse_ruling_relative_path,
15+
parse_rule_filename,
16+
strip_project_key,
17+
)
18+
from ruling_diff_core_lib.snippet_generation import (
19+
render_file_level_snippet,
20+
render_line_snippet,
21+
render_snippet,
22+
)
23+
24+
__all__ = [
25+
"COMMENT_MARKER",
26+
"COMMENT_SOFT_LIMIT",
27+
"EXPECTED_RULING_ROOT",
28+
"IssueDiff",
29+
"RuleDiff",
30+
"Snippet",
31+
"build_rule_diffs",
32+
"diff_ruling_jsons",
33+
"format_comment",
34+
"parse_ruling_path",
35+
"parse_ruling_relative_path",
36+
"parse_rule_filename",
37+
"render_file_level_snippet",
38+
"render_line_snippet",
39+
"render_snippet",
40+
"strip_project_key",
41+
]
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
from ruling_diff_core_lib.comment_rendering import format_comment
2+
from ruling_diff_core_lib.models_and_constants import (
3+
COMMENT_MARKER,
4+
COMMENT_SOFT_LIMIT,
5+
EXPECTED_RULING_ROOT,
6+
IssueDiff,
7+
RuleDiff,
8+
Snippet,
9+
)
10+
from ruling_diff_core_lib.ruling_diff_logic import (
11+
build_rule_diffs,
12+
diff_ruling_jsons,
13+
parse_ruling_path,
14+
parse_ruling_relative_path,
15+
parse_rule_filename,
16+
strip_project_key,
17+
)
18+
from ruling_diff_core_lib.snippet_generation import (
19+
render_file_level_snippet,
20+
render_line_snippet,
21+
render_snippet,
22+
)
23+
24+
__all__ = [
25+
"COMMENT_MARKER",
26+
"COMMENT_SOFT_LIMIT",
27+
"EXPECTED_RULING_ROOT",
28+
"IssueDiff",
29+
"RuleDiff",
30+
"Snippet",
31+
"build_rule_diffs",
32+
"diff_ruling_jsons",
33+
"format_comment",
34+
"parse_ruling_path",
35+
"parse_ruling_relative_path",
36+
"parse_rule_filename",
37+
"render_file_level_snippet",
38+
"render_line_snippet",
39+
"render_snippet",
40+
"strip_project_key",
41+
]
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
from __future__ import annotations
2+
3+
from ruling_diff_core_lib.models_and_constants import (
4+
COMMENT_MARKER,
5+
COMMENT_SOFT_LIMIT,
6+
RuleDiff,
7+
Snippet,
8+
)
9+
10+
11+
def format_comment(
12+
rule_diffs: list[RuleDiff], soft_limit: int = COMMENT_SOFT_LIMIT
13+
) -> str:
14+
if not rule_diffs:
15+
return "\n".join(
16+
[
17+
COMMENT_MARKER,
18+
"## Ruling Diff Summary",
19+
"",
20+
"No issue deltas detected."
21+
]
22+
)
23+
comment = format_comment_header(rule_diffs)
24+
return append_rule_sections_with_soft_limit(comment, rule_diffs, soft_limit)
25+
26+
27+
def format_comment_header(rule_diffs: list[RuleDiff]) -> str:
28+
added = sum(len(diff.added_lines) for rule in rule_diffs for diff in rule.file_diffs)
29+
removed = sum(
30+
len(diff.removed_lines) for rule in rule_diffs for diff in rule.file_diffs
31+
)
32+
return "\n".join(
33+
[
34+
COMMENT_MARKER,
35+
"## Ruling Diff Summary",
36+
"",
37+
f"Detected changes in {len(rule_diffs)} rule files: {removed} issues removed, {added} issues added.",
38+
"",
39+
]
40+
)
41+
42+
43+
def append_rule_sections_with_soft_limit(
44+
comment: str, rule_diffs: list[RuleDiff], soft_limit: int
45+
) -> str:
46+
sections = [format_rule_section(rule_diff) for rule_diff in rule_diffs]
47+
accepted_sections: list[str] = []
48+
truncated_count = 0
49+
for index, section in enumerate(sections):
50+
candidate = comment + "\n\n".join(accepted_sections + [section])
51+
if len(candidate) > soft_limit:
52+
truncated_count = len(sections) - index
53+
break
54+
accepted_sections.append(section)
55+
if accepted_sections:
56+
comment += "\n\n".join(accepted_sections)
57+
if truncated_count:
58+
comment = append_truncation_notice(comment, truncated_count, bool(accepted_sections))
59+
return comment
60+
61+
62+
def append_truncation_notice(comment: str, count: int, has_sections: bool) -> str:
63+
separator = "\n\n" if has_sections else ""
64+
return (
65+
comment
66+
+ separator
67+
+ f"... and {count} more rules with changes (diff too large to display fully)"
68+
)
69+
70+
71+
def format_rule_section(rule_diff: RuleDiff) -> str:
72+
lines = ["<details>", f"<summary>{format_rule_summary(rule_diff)}</summary>", ""]
73+
if not rule_diff.snippets:
74+
lines.append("No source snippets available for this rule.")
75+
else:
76+
for snippet in rule_diff.snippets:
77+
lines.append(format_snippet_block(snippet))
78+
lines.append("")
79+
lines.append("</details>")
80+
return "\n".join(lines)
81+
82+
83+
def format_rule_summary(rule_diff: RuleDiff) -> str:
84+
removed = sum(len(diff.removed_lines) for diff in rule_diff.file_diffs)
85+
added = sum(len(diff.added_lines) for diff in rule_diff.file_diffs)
86+
summary_parts = [
87+
f"<b>{rule_diff.rule_key}</b> (<code>{rule_diff.repo}</code>) on <b>{rule_diff.project}</b>",
88+
f"{removed} issues removed, {added} issues added",
89+
]
90+
if rule_diff.is_new_file:
91+
summary_parts.append("new ruling file")
92+
if rule_diff.is_deleted_file:
93+
summary_parts.append("deleted ruling file")
94+
return " - ".join(summary_parts)
95+
96+
97+
def format_snippet_block(snippet: Snippet) -> str:
98+
return "\n".join([format_snippet_header(snippet), "```python", snippet.body, "```"])
99+
100+
101+
def format_snippet_header(snippet: Snippet) -> str:
102+
label = "Added" if snippet.change_kind == "added" else "Removed"
103+
location = "file-level" if snippet.line_number == 0 else f"line {snippet.line_number}"
104+
return f"**{label}** `{snippet.file_path}` ({location})"
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
from __future__ import annotations
2+
3+
from dataclasses import dataclass
4+
from typing import Protocol
5+
6+
RulingJson = dict[str, list[int]]
7+
OptionalRulingJson = RulingJson | None
8+
SourceLines = list[str]
9+
OptionalSourceLines = SourceLines | None
10+
SourceCache = dict[tuple[str, str], OptionalSourceLines]
11+
12+
13+
class RulingDiffIO(Protocol):
14+
def load_json_at_ref(self, path: str, ref: str) -> OptionalRulingJson:
15+
...
16+
17+
def load_text_at_ref(self, path: str, ref: str) -> str | None:
18+
...
19+
20+
def resolve_source_path(self, project: str, file_path: str) -> str:
21+
...
22+
23+
EXPECTED_RULING_ROOT = (
24+
"private/its-enterprise/ruling/src/test/resources/expected_ruling"
25+
)
26+
COMMENT_MARKER = "<!-- ruling-diff-comment -->"
27+
COMMENT_SOFT_LIMIT = 60000
28+
SNIPPET_CONTEXT = 5
29+
MAX_SNIPPETS_PER_FILE = 3
30+
MAX_SNIPPETS_PER_RULE = 15
31+
32+
PROJECT_SOURCE_OVERRIDES = {
33+
"buildbot": "private/its-enterprise/sources_ruling/buildbot-0.8.6p1",
34+
"buildbot-slave": "private/its-enterprise/sources_ruling/buildbot-slave-0.8.6p1",
35+
"django": "private/its-enterprise/sources_ruling/django-2.2.3",
36+
"django-cms": "private/its-enterprise/sources_ruling/django-cms-3.7.1",
37+
"docker-compose": "private/its-enterprise/sources_ruling/docker-compose-1.24.1",
38+
"mypy": "private/its-enterprise/sources_ruling/mypy-0.782",
39+
"numpy": "private/its-enterprise/sources_ruling/numpy-1.16.4",
40+
"tornado": "private/its-enterprise/sources_ruling/tornado-2.3",
41+
"twisted": "private/its-enterprise/sources_ruling/twisted-12.1.0",
42+
"sources_internal_ruling": "private/its-enterprise/sources_internal_ruling",
43+
"namespace_basic": "private/its-enterprise/sources_internal_namespace_ruling/basic_namespace",
44+
"namespace_mixed": "private/its-enterprise/sources_internal_namespace_ruling/mixed_namespace",
45+
}
46+
47+
48+
@dataclass(frozen=True)
49+
class IssueDiff:
50+
file_path: str
51+
added_lines: list[int]
52+
removed_lines: list[int]
53+
54+
55+
@dataclass(frozen=True)
56+
class Snippet:
57+
file_path: str
58+
line_number: int
59+
change_kind: str
60+
body: str
61+
62+
63+
@dataclass(frozen=True)
64+
class RuleDiff:
65+
project: str
66+
repo: str
67+
rule_key: str
68+
file_diffs: list[IssueDiff]
69+
snippets: list[Snippet]
70+
is_new_file: bool
71+
is_deleted_file: bool

0 commit comments

Comments
 (0)