Skip to content

Commit d41747d

Browse files
debontedebonte
authored
Fix issue grouping bug that gave different results based on ordering (#70)
1 parent 509f7c7 commit d41747d

File tree

3 files changed

+172
-2
lines changed

3 files changed

+172
-2
lines changed

‎sarif/issues_report.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,15 @@ def _group_records_by_key(self):
5454
key_and_count["count"] += 1
5555
common_desc_stem = key_and_count["common_desc"]
5656
desc = record["Description"]
57-
if common_desc_stem != desc:
57+
if not desc.startswith(common_desc_stem):
5858
for char_pos, (char1, char2) in enumerate(
5959
zip(common_desc_stem, desc)
6060
):
6161
if char1 != char2:
62+
new_desc_stem = common_desc_stem[0:char_pos]
63+
key_and_count["common_desc"] = new_desc_stem
6264
key_and_count["key"] = combine_code_and_description(
63-
code, common_desc_stem[0:char_pos] + " ..."
65+
code, new_desc_stem + " ..."
6466
)
6567
break
6668
sorted_codes = sorted(

‎tests/diff/test_diff_issues_reordered.py‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import datetime
2+
import json
3+
import os
4+
import tempfile
5+
6+
from sarif.operations import diff_op
7+
from sarif import sarif_file
8+
9+
SARIF = {
10+
"$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
11+
"version": "2.1.0",
12+
"runs": [
13+
{
14+
"tool": {"driver": {"name": "unit test"}},
15+
"results": [
16+
{
17+
"ruleId": "core.NullDereference",
18+
"ruleIndex": 2,
19+
"message": {
20+
"text": "Access to field 'type' results in a dereference of a null pointer (loaded from variable 'json')"
21+
},
22+
"locations": [
23+
{
24+
"physicalLocation": {
25+
"artifactLocation": {
26+
"uri": "file:///C:/Code/main.c",
27+
"index": 0,
28+
},
29+
"region": {"startLine": 24, "startColumn": 9},
30+
}
31+
}
32+
],
33+
},
34+
{
35+
"ruleId": "core.NullDereference",
36+
"ruleIndex": 2,
37+
"message": {
38+
"text": "Dereference of null pointer (loaded from variable 's')"
39+
},
40+
"locations": [
41+
{
42+
"physicalLocation": {
43+
"artifactLocation": {
44+
"uri": "file:///C:/Code/main.c",
45+
"index": 0,
46+
},
47+
"region": {"startLine": 24, "startColumn": 9},
48+
}
49+
}
50+
],
51+
},
52+
{
53+
"ruleId": "core.NullDereference",
54+
"ruleIndex": 2,
55+
"message": {
56+
"text": "Access to field 'other' results in a dereference of a null pointer (loaded from variable 'json')"
57+
},
58+
"locations": [
59+
{
60+
"physicalLocation": {
61+
"artifactLocation": {
62+
"uri": "file:///C:/Code/main.c",
63+
"index": 0,
64+
},
65+
"region": {"startLine": 24, "startColumn": 9},
66+
}
67+
}
68+
],
69+
},
70+
],
71+
}
72+
],
73+
}
74+
75+
SARIF_WITH_ISSUES_REORDERED = {
76+
"$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
77+
"version": "2.1.0",
78+
"runs": [
79+
{
80+
"tool": {"driver": {"name": "unit test"}},
81+
"results": [
82+
{
83+
"ruleId": "core.NullDereference",
84+
"ruleIndex": 2,
85+
"message": {
86+
"text": "Access to field 'type' results in a dereference of a null pointer (loaded from variable 'json')"
87+
},
88+
"locations": [
89+
{
90+
"physicalLocation": {
91+
"artifactLocation": {
92+
"uri": "file:///C:/Code/main.c",
93+
"index": 0,
94+
},
95+
"region": {"startLine": 24, "startColumn": 9},
96+
}
97+
}
98+
],
99+
},
100+
{
101+
"ruleId": "core.NullDereference",
102+
"ruleIndex": 2,
103+
"message": {
104+
"text": "Access to field 'other' results in a dereference of a null pointer (loaded from variable 'json')"
105+
},
106+
"locations": [
107+
{
108+
"physicalLocation": {
109+
"artifactLocation": {
110+
"uri": "file:///C:/Code/main.c",
111+
"index": 0,
112+
},
113+
"region": {"startLine": 24, "startColumn": 9},
114+
}
115+
}
116+
],
117+
},
118+
{
119+
"ruleId": "core.NullDereference",
120+
"ruleIndex": 2,
121+
"message": {
122+
"text": "Dereference of null pointer (loaded from variable 's')"
123+
},
124+
"locations": [
125+
{
126+
"physicalLocation": {
127+
"artifactLocation": {
128+
"uri": "file:///C:/Code/main.c",
129+
"index": 0,
130+
},
131+
"region": {"startLine": 24, "startColumn": 9},
132+
}
133+
}
134+
],
135+
},
136+
],
137+
}
138+
],
139+
}
140+
141+
142+
def test_diff_issues_reordered():
143+
mtime = datetime.datetime.now()
144+
sarif = sarif_file.SarifFile(
145+
"SARIF", SARIF, mtime=mtime
146+
)
147+
sarif_reordered = sarif_file.SarifFile(
148+
"SARIF_WITH_ISSUES_REORDERED", SARIF_WITH_ISSUES_REORDERED, mtime=mtime
149+
)
150+
verify_no_diffs(sarif, sarif_reordered)
151+
verify_no_diffs(sarif_reordered, sarif)
152+
153+
154+
def verify_no_diffs(old_sarif: sarif_file.SarifFile, new_sarif: sarif_file.SarifFile):
155+
with tempfile.TemporaryDirectory() as tmp:
156+
file_path = os.path.join(tmp, "diff.json")
157+
result = diff_op.print_diff(
158+
old_sarif, new_sarif, file_path, check_level="warning"
159+
)
160+
with open(file_path, "rb") as f_in:
161+
diff_dict = json.load(f_in)
162+
assert result == 0
163+
assert diff_dict == {
164+
"all": {"+": 0, "-": 0},
165+
"error": {"+": 0, "-": 0, "codes": {}},
166+
"warning": {"+": 0, "-": 0, "codes": {}},
167+
"note": {"+": 0, "-": 0, "codes": {}},
168+
}

tests/test_diff_op.py renamed to tests/diff/test_diff_op.py

File renamed without changes.

0 commit comments

Comments
 (0)