Skip to content

Commit f741439

Browse files
Pierre-SassoulasPierre-Sassoulas
committed
[primer] Classify new messages into false-positive categories
New messages are now sorted into subcategories: - useless-suppression → "fixed false positives" (🎉) - locally-disabled → "introduced false positives" (😞) - astroid-error → counted separately - everything else → "now emitted" Extract _ClassifiedMessages and _format_messages helpers, and use _details_section for consistent <details> blocks with the blank line required for GitHub markdown rendering.
1 parent b74faa8 commit f741439

15 files changed

Lines changed: 135 additions & 80 deletions

File tree

‎pylint/testutils/_primer/primer_compare_command.py‎

Lines changed: 99 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt
44
from __future__ import annotations
55

6+
import re
7+
from collections.abc import Callable
68
from pathlib import PurePosixPath
79

810
from pylint.reporters.json_reporter import JSONMessage
@@ -14,6 +16,70 @@
1416
from pylint.testutils._primer.primer_command import PrimerCommand
1517

1618
MAX_GITHUB_COMMENT_LENGTH = 65536
19+
USELESS_SUPPRESSION_RE = re.compile(r"Useless suppression of '(.+)'")
20+
21+
22+
def _format_messages(
23+
messages: list[JSONMessage],
24+
source_link: Callable[[JSONMessage], str],
25+
) -> str:
26+
"""Format a list of messages as a numbered body for a ``<details>`` block."""
27+
body = ""
28+
for count, msg in enumerate(messages, 1):
29+
body += (
30+
f"{count}) {msg['symbol']}:\n*{msg['message']}*\n" f"{source_link(msg)}\n"
31+
)
32+
return body
33+
34+
35+
class _ClassifiedMessages:
36+
"""All message categories for a single package, pre-formatted for display."""
37+
38+
__slots__ = (
39+
"astroid_errors",
40+
"changed_messages",
41+
"fixed_false_positives",
42+
"missing_messages",
43+
"new_false_positives",
44+
"new_messages",
45+
)
46+
47+
def __init__(
48+
self,
49+
new_messages: list[JSONMessage],
50+
missing_messages: list[JSONMessage],
51+
changed_messages: list[ChangedMessage],
52+
source_link: Callable[[JSONMessage], str],
53+
) -> None:
54+
astroid_errors = 0
55+
new_fp: list[JSONMessage] = []
56+
fixed_fp: list[JSONMessage] = []
57+
other_new: list[JSONMessage] = []
58+
for message in new_messages:
59+
if message["symbol"] == "astroid-error":
60+
astroid_errors += 1
61+
elif USELESS_SUPPRESSION_RE.match(message["message"]):
62+
fixed_fp.append(message)
63+
elif message["symbol"] == "locally-disabled":
64+
# A new locally-disabled means a maintainer had to add a
65+
# suppression comment — we introduced a false positive.
66+
new_fp.append(message)
67+
else:
68+
other_new.append(message)
69+
70+
self.astroid_errors = astroid_errors
71+
self.fixed_false_positives = _format_messages(fixed_fp, source_link)
72+
self.new_false_positives = _format_messages(new_fp, source_link)
73+
self.new_messages = _format_messages(other_new, source_link)
74+
self.missing_messages = _format_messages(missing_messages, source_link)
75+
76+
body = ""
77+
for count, (old, new) in enumerate(changed_messages, 1):
78+
body += (
79+
f"{count}) [{new['symbol']}]({source_link(new)}):\n"
80+
f"{message_diff(old, new)}\n"
81+
)
82+
self.changed_messages = body
1783

1884

1985
class CompareCommand(PrimerCommand):
@@ -66,63 +132,51 @@ def _source_link(msg: JSONMessage) -> str:
66132
filepath = str(PurePosixPath(msg["path"]).relative_to(clone_dir))
67133
return f"{url}/blob/{commit}/{filepath}#L{msg['line']}"
68134

135+
def _details_section(title: str, body: str) -> str:
136+
# Blank line after <details> required for GitHub markdown rendering.
137+
return f"{title}\n\n<details>\n\n{body}</details>\n\n"
138+
139+
classified = _ClassifiedMessages(
140+
new_messages, missing_messages, changed_messages, _source_link
141+
)
69142
comment = f"\n**Effect on [{package}]({url}):**\n\n"
70143

71-
# Changed messages
72144
if changed_messages:
73145
print("Changed:")
74-
comment += "The following messages have been changed:\n\n<details>\n\n"
75-
for count, (old, new) in enumerate(changed_messages, 1):
76-
comment += (
77-
f"{count}) [{new['symbol']}]({_source_link(new)}):\n"
78-
f"{message_diff(old, new)}\n"
79-
)
146+
for _, new in changed_messages:
80147
print(new)
81-
comment += "</details>\n\n"
82-
83-
# New messages
84-
count = 1
85-
astroid_errors = 0
86-
new_non_astroid_messages = ""
87148
if new_messages:
88149
print("Now emitted:")
89-
for message in new_messages:
90-
if message["symbol"] == "astroid-error":
91-
astroid_errors += 1
92-
else:
93-
new_non_astroid_messages += (
94-
f"{count}) {message['symbol']}:\n*{message['message']}*\n"
95-
f"{_source_link(message)}\n"
96-
)
97-
print(message)
98-
count += 1
99-
100-
if astroid_errors:
150+
for msg in new_messages:
151+
print(msg)
152+
if missing_messages:
153+
print("No longer emitted:")
154+
for msg in missing_messages:
155+
print(msg)
156+
157+
if classified.changed_messages:
158+
comment += _details_section(
159+
"Changed messages:", classified.changed_messages
160+
)
161+
if classified.astroid_errors:
101162
comment += (
102-
f'{astroid_errors} "astroid error(s)" were found. '
163+
f'{classified.astroid_errors} "astroid error(s)" were found. '
103164
"Please open the GitHub Actions log to see what failed or crashed.\n\n"
104165
)
105-
if new_non_astroid_messages:
106-
comment += (
107-
"The following messages are now emitted:\n\n<details>\n\n"
108-
+ new_non_astroid_messages
109-
+ "</details>\n\n"
166+
if classified.fixed_false_positives:
167+
comment += _details_section(
168+
"🎉 Fixed false positives:", classified.fixed_false_positives
110169
)
111-
112-
# Missing messages
113-
count = 1
114-
if missing_messages:
115-
comment += "The following messages are no longer emitted:\n\n<details>\n\n"
116-
print("No longer emitted:")
117-
for message in missing_messages:
118-
comment += (
119-
f"{count}) {message['symbol']}:\n*{message['message']}*\n"
120-
f"{_source_link(message)}\n"
170+
if classified.new_false_positives:
171+
comment += _details_section(
172+
"😞 New false positives:", classified.new_false_positives
173+
)
174+
if classified.new_messages:
175+
comment += _details_section("New messages:", classified.new_messages)
176+
if classified.missing_messages:
177+
comment += _details_section(
178+
"Removed messages:", classified.missing_messages
121179
)
122-
count += 1
123-
print(message)
124-
if missing_messages:
125-
comment += "</details>\n\n"
126180
return comment
127181

128182
def _truncate_comment(self, comment: str) -> str:

‎tests/testutils/_primer/batched_cases/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages are no longer emitted:
6+
Removed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/confidence_changed/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/line_moved/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/message_changed/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/message_changed/expected_truncated.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,15 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

1010
1) [locally-disabled](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/__init__.py#L91):
1111
```diff
12-
- Locally disabling redefined-builtin (W0622)
13-
+ Locally disabling...
12+
- Locally disabling...
1413
</details>
1514

16-
*This comment was truncated because GitHub allows only 525 characters in a comment.*
15+
*This comment was truncated because GitHub allows only 450 characters in a comment.*
1716

1817
*This comment was generated for commit v2.14.2*

‎tests/testutils/_primer/cases/message_changed/expected_truncated_in_details.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

1010
1)...
1111
</details>
1212

13-
*This comment was truncated because GitHub allows only 420 characters in a comment.*
13+
*This comment was truncated because GitHub allows only 400 characters in a comment.*
1414

1515
*This comment was generated for commit v2.14.2*

‎tests/testutils/_primer/cases/message_changed_line/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/mixed_changes/expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages have been changed:
6+
Changed messages:
77

88
<details>
99

‎tests/testutils/_primer/cases/multi_package/expected.txt‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,24 @@
33

44
**Effect on [astroid](https://github.com/pylint-dev/astroid):**
55

6-
The following messages are now emitted:
6+
Changed messages:
77

88
<details>
99

10-
1) too-complex:
11-
*'my_function' is too complex. The McCabe rating is 17*
12-
https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L105
13-
</details>
14-
15-
The following messages are no longer emitted:
16-
17-
<details>
10+
1) [too-complex](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L105):
11+
Was raised from `100:0` to `100:15`, now from `105:0` to `105:15`.
1812

19-
1) too-complex:
20-
*'my_function' is too complex. The McCabe rating is 18*
21-
https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L100
13+
```diff
14+
- 'my_function' is too complex. The McCabe rating is 18
15+
+ 'my_function' is too complex. The McCabe rating is 17
16+
^
17+
```
2218
</details>
2319

2420

2521
**Effect on [astropy](https://github.com/astropy/astropy):**
2622

27-
😞 The following false positives have been introduced:
23+
😞 New false positives:
2824

2925
<details>
3026

@@ -33,7 +29,7 @@ https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.
3329
https://github.com/astropy/astropy/blob/1fb40bc1f22f176254ef583065aa155f53f3b414/astropy/coordinates/angles/angle_parsetab.py#L14
3430
</details>
3531

36-
The following messages are no longer emitted:
32+
Removed messages:
3733

3834
<details>
3935

0 commit comments

Comments
 (0)