Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

Commit 01986b6

Browse files
giovanni-guidinigiovanni-guidini
authored
fix: Fix python analysis for function docstrings (#267)
* fix: Fix python analysis for function docstrings context: https://l.codecov.dev/hfGkNI This is a fix for a bug that I stumbled upon today. After much investigation it boils down to this: - treesitter count's docstrings as strings, not comments (which is to be expected I suppose) - pytest counts them as comments. They won't appear in the coverage report (if function docstring) - if you select a docstring as the line of surety ancestorship, because it's not in the report, it has no labels associated with it, so ATS selects no tests for that line We were doing that (I believe) and selected a function docstring as the line in base to look for tests. No tests were found, and so ATS passed, but all tests failed. These changes fix this behavior by doing 2 things: 1. Making sure function docstrings are not in `statements` on the static analysis 2. Making sure we don't select function docstrings as the line of surety ancestorship for the actual fist line of code in a funciton. It should be `null` instead. * addressing style comments
1 parent 7673f84 commit 01986b6

File tree

4 files changed

+298
-2
lines changed

4 files changed

+298
-2
lines changed

‎codecov_cli/services/staticanalysis/analyzers/python/node_wrappers.py‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,43 @@ def visit(self, node):
1010
for c in node.children:
1111
self.visit(c)
1212

13+
def _is_function_docstring(self, node):
14+
"""Skips docstrings for funtions, such as this one.
15+
Pytest doesn't include them in the report, so I don't think we should either,
16+
at least for now.
17+
"""
18+
# Docstrings have type 'expression_statement
19+
if node.type != "expression_statement":
20+
return False
21+
# Docstrings for a module are OK - they show up in pytest result
22+
# Docstrings for a class are OK - they show up in pytest result
23+
# Docstrings for functions are NOT OK - they DONT show up in pytest result
24+
# Check if it's docstring
25+
has_single_child = len(node.children) == 1
26+
only_child_is_string = node.children[0].type == "string"
27+
# Check if is the first line of a function
28+
parent_is_block = node.parent.type == "block"
29+
first_exp_in_block = node.prev_named_sibling is None
30+
is_in_function_context = (
31+
parent_is_block and node.parent.parent.type == "function_definition"
32+
)
33+
34+
return (
35+
has_single_child
36+
and only_child_is_string
37+
and parent_is_block
38+
and first_exp_in_block
39+
and is_in_function_context
40+
)
41+
42+
def _get_previous_sibling_that_is_not_comment_not_func_docstring(self, node):
43+
curr = node.prev_named_sibling
44+
while curr is not None and (
45+
curr.type == "comment" or self._is_function_docstring(curr)
46+
):
47+
curr = curr.prev_named_sibling
48+
return curr
49+
1350
def do_visit(self, node):
1451
if node.is_named:
1552
current_line_number = node.start_point[0] + 1
@@ -20,9 +57,20 @@ def do_visit(self, node):
2057
"for_statement",
2158
"while_statement",
2259
):
23-
if node.prev_named_sibling:
60+
if self._is_function_docstring(node):
61+
# We ignore these
62+
return
63+
closest_named_sibling_not_comment_that_is_in_statements = (
64+
self._get_previous_sibling_that_is_not_comment_not_func_docstring(
65+
node
66+
)
67+
)
68+
if closest_named_sibling_not_comment_that_is_in_statements:
2469
self.analyzer.line_surety_ancestorship[current_line_number] = (
25-
node.prev_named_sibling.start_point[0] + 1
70+
closest_named_sibling_not_comment_that_is_in_statements.start_point[
71+
0
72+
]
73+
+ 1
2674
)
2775
self.analyzer.statements.append(
2876
{

‎samples/inputs/sample_005.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"""An important module with some documentation on top.
2+
"When I created this code only me and God knew what it does. Now only God knows"
3+
"""
4+
5+
# Random comment
6+
# Random comment
7+
# Random comment
8+
9+
x = "some string"
10+
y = 'some string'
11+
w = """some string"""
12+
13+
def well_documented_function(a, b):
14+
""" Returns the value of a + b, a - b and a * b
15+
As a tuple, in that order
16+
"""
17+
plus = a + b
18+
minus = a - b
19+
times = a * b
20+
return (plus, minus, times)
21+
22+
def less_documented_function(a, b):
23+
""" Returns tuple(a + b, a - b, a * b)"""
24+
return (a + b, a - b, a * b)
25+
26+
def commented_function(a, b):
27+
# Returns tuple(a + b, a - b, a * b)
28+
return (a + b, a - b, a * b)
29+
30+
class MyClass:
31+
""" This is my class, not yours u.u
32+
"""
33+
34+
def __init__(self) -> None:
35+
self.owner = 'me'

‎samples/outputs/sample_005.json‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
{
2+
"result": {
3+
"language": "python",
4+
"empty_lines": [
5+
4,
6+
8,
7+
12,
8+
21,
9+
25,
10+
29,
11+
33
12+
],
13+
"functions": [
14+
{
15+
"identifier": "well_documented_function",
16+
"start_line": 13,
17+
"end_line": 20,
18+
"code_hash": "a3dd128ed3a7f6b87dff831df9536041",
19+
"complexity_metrics": {
20+
"conditions": 0,
21+
"mccabe_cyclomatic_complexity": 1,
22+
"returns": 1,
23+
"max_nested_conditional": 0
24+
}
25+
},
26+
{
27+
"identifier": "less_documented_function",
28+
"start_line": 22,
29+
"end_line": 24,
30+
"code_hash": "5b39073ac5cd6ae87194ea607c355836",
31+
"complexity_metrics": {
32+
"conditions": 0,
33+
"mccabe_cyclomatic_complexity": 1,
34+
"returns": 1,
35+
"max_nested_conditional": 0
36+
}
37+
},
38+
{
39+
"identifier": "commented_function",
40+
"start_line": 26,
41+
"end_line": 28,
42+
"code_hash": "26a5e6a370792d61352f402e98066508",
43+
"complexity_metrics": {
44+
"conditions": 0,
45+
"mccabe_cyclomatic_complexity": 1,
46+
"returns": 1,
47+
"max_nested_conditional": 0
48+
}
49+
},
50+
{
51+
"identifier": "MyClass::__init__",
52+
"start_line": 34,
53+
"end_line": 35,
54+
"code_hash": "da07aae9b4c31039150d1eebbf835f4b",
55+
"complexity_metrics": {
56+
"conditions": 0,
57+
"mccabe_cyclomatic_complexity": 1,
58+
"returns": 0,
59+
"max_nested_conditional": 0
60+
}
61+
}
62+
],
63+
"hash": "0623dd2b794a6f7399865865b127a7ee",
64+
"number_lines": 35,
65+
"statements": [
66+
[
67+
1,
68+
{
69+
"line_surety_ancestorship": null,
70+
"start_column": 0,
71+
"line_hash": "b6ac9155701928c76d7de0f40c023b2a",
72+
"len": 2,
73+
"extra_connected_lines": []
74+
}
75+
],
76+
[
77+
9,
78+
{
79+
"line_surety_ancestorship": 1,
80+
"start_column": 0,
81+
"line_hash": "968a672046ffebf7efa7d9298b53223b",
82+
"len": 0,
83+
"extra_connected_lines": []
84+
}
85+
],
86+
[
87+
10,
88+
{
89+
"line_surety_ancestorship": 9,
90+
"start_column": 0,
91+
"line_hash": "7e2ea08683844b89b362b9b920aeb1f4",
92+
"len": 0,
93+
"extra_connected_lines": []
94+
}
95+
],
96+
[
97+
11,
98+
{
99+
"line_surety_ancestorship": 10,
100+
"start_column": 0,
101+
"line_hash": "72bcafea9cfcf4995233dee3004c5830",
102+
"len": 0,
103+
"extra_connected_lines": []
104+
}
105+
],
106+
[
107+
17,
108+
{
109+
"line_surety_ancestorship": null,
110+
"start_column": 4,
111+
"line_hash": "9237152e7a4e9da3a61c59a992ee313b",
112+
"len": 0,
113+
"extra_connected_lines": []
114+
}
115+
],
116+
[
117+
18,
118+
{
119+
"line_surety_ancestorship": 17,
120+
"start_column": 4,
121+
"line_hash": "78fd33f477009fb9dec82c435b6049fd",
122+
"len": 0,
123+
"extra_connected_lines": []
124+
}
125+
],
126+
[
127+
19,
128+
{
129+
"line_surety_ancestorship": 18,
130+
"start_column": 4,
131+
"line_hash": "3a8f5fe6d0cd2995e78a189e004c46cb",
132+
"len": 0,
133+
"extra_connected_lines": []
134+
}
135+
],
136+
[
137+
20,
138+
{
139+
"line_surety_ancestorship": 19,
140+
"start_column": 4,
141+
"line_hash": "1de7b03608e8c92e83a094f044863159",
142+
"len": 0,
143+
"extra_connected_lines": []
144+
}
145+
],
146+
[
147+
24,
148+
{
149+
"line_surety_ancestorship": null,
150+
"start_column": 4,
151+
"line_hash": "26a5e6a370792d61352f402e98066508",
152+
"len": 0,
153+
"extra_connected_lines": []
154+
}
155+
],
156+
[
157+
28,
158+
{
159+
"line_surety_ancestorship": null,
160+
"start_column": 4,
161+
"line_hash": "26a5e6a370792d61352f402e98066508",
162+
"len": 0,
163+
"extra_connected_lines": []
164+
}
165+
],
166+
[
167+
31,
168+
{
169+
"line_surety_ancestorship": null,
170+
"start_column": 4,
171+
"line_hash": "9185d2f3d96e27b9446aa4361a60c996",
172+
"len": 1,
173+
"extra_connected_lines": []
174+
}
175+
],
176+
[
177+
35,
178+
{
179+
"line_surety_ancestorship": null,
180+
"start_column": 8,
181+
"line_hash": "da07aae9b4c31039150d1eebbf835f4b",
182+
"len": 0,
183+
"extra_connected_lines": []
184+
}
185+
]
186+
],
187+
"definition_lines": [
188+
[
189+
13,
190+
7
191+
],
192+
[
193+
22,
194+
2
195+
],
196+
[
197+
26,
198+
2
199+
],
200+
[
201+
30,
202+
5
203+
],
204+
[
205+
34,
206+
1
207+
]
208+
],
209+
"import_lines": []
210+
},
211+
"error": null
212+
}

‎tests/services/static_analysis/test_analyse_file.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
("samples/inputs/sample_002.py", "samples/outputs/sample_002.json"),
1919
("samples/inputs/sample_003.js", "samples/outputs/sample_003.json"),
2020
("samples/inputs/sample_004.js", "samples/outputs/sample_004.json"),
21+
("samples/inputs/sample_005.py", "samples/outputs/sample_005.json"),
2122
],
2223
)
2324
def test_sample_analysis(input_filename, output_filename):

0 commit comments

Comments
 (0)