Skip to content

Commit f71b5aa

Browse files
committed
refactor: address code review findings
decorator_processor.py: - Add FormattedEntry, RequirementAnnotations, FormattedData TypedDicts for format_results return type (replaces bare dict) - Add str | os.PathLike to all path/file parameters for consistency - Tighten format_results body to use typed local variables test_processors.py: - Standardise all tests on tmp_path (drop legacy tmpdir fixture) - Remove stray inline `import os` from test body; use pathlib Path.exists() - Remove unused tmpdir parameter from test_format_results_implementations
1 parent 82afc71 commit f71b5aa

File tree

2 files changed

+79
-65
lines changed

2 files changed

+79
-65
lines changed

src/reqstool_python_decorators/processors/decorator_processor.py

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ class ElementResult(TypedDict):
3232
decorators: ReadOnly[list[DecoratorInfo]]
3333

3434

35+
class FormattedEntry(TypedDict):
36+
elementKind: str
37+
fullyQualifiedName: str
38+
39+
40+
class RequirementAnnotations(TypedDict):
41+
implementations: dict[str, list[FormattedEntry]]
42+
tests: dict[str, list[FormattedEntry]]
43+
44+
45+
class FormattedData(TypedDict):
46+
requirement_annotations: RequirementAnnotations
47+
48+
3549
type Results = list[ElementResult]
3650

3751

@@ -65,12 +79,12 @@ def __init__(self, *args, **kwargs):
6579
super().__init__(*args, **kwargs)
6680
self.req_svc_results: Results = []
6781

68-
def find_python_files(self, directory) -> list[str]:
82+
def find_python_files(self, directory: str | os.PathLike) -> list[str]:
6983
"""
7084
Find Python files in the given directory.
7185
7286
Parameters:
73-
- `directory` (str): The directory to search for Python files.
87+
- `directory` (str | PathLike): The directory to search for Python files.
7488
7589
Returns:
7690
- `python_files` (list): List of Python files found in the directory.
@@ -82,17 +96,16 @@ def find_python_files(self, directory) -> list[str]:
8296
python_files.append(os.path.join(root, file))
8397
return python_files
8498

85-
def get_functions_and_classes(self, file_path, decorator_names) -> None:
99+
def get_functions_and_classes(
100+
self, file_path: str | os.PathLike, decorator_names: list[str]
101+
) -> None:
86102
"""
87103
Get information about functions and classes, if annotated with "Requirements" or "SVCs":
88104
decorator filepath, elementKind, name and decorators is saved to list that is returned.
89105
90106
Parameters:
91-
- `file_path` (str): The path to the Python file.
92-
- `decorator_names` (list): List of decorator names to search for.
93-
94-
Returns:
95-
- `results` (list): List of dictionaries containing information about functions and classes.
107+
- `file_path` (str | PathLike): The path to the Python file.
108+
- `decorator_names` (list[str]): List of decorator names to search for.
96109
97110
Each dictionary includes:
98111
- `fullyQualifiedName` (str): The fully qualified name of the file.
@@ -101,7 +114,7 @@ def get_functions_and_classes(self, file_path, decorator_names) -> None:
101114
- `decorators` (list): List of dictionaries with decorator info including name and arguments e.g. "REQ_001".
102115
"""
103116
with open(file_path, "r") as file:
104-
tree = ast.parse(file.read(), filename=file_path)
117+
tree = ast.parse(file.read(), filename=str(file_path))
105118

106119
for node in ast.walk(tree):
107120
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
@@ -122,12 +135,12 @@ def get_functions_and_classes(self, file_path, decorator_names) -> None:
122135
}
123136
)
124137

125-
def write_to_yaml(self, output_file, formatted_data) -> None:
138+
def write_to_yaml(self, output_file: str | os.PathLike, formatted_data) -> None:
126139
"""
127140
Write formatted data to a YAML file.
128141
129142
Parameters:
130-
- `output_file` (str): The path to the output YAML file.
143+
- `output_file` (str | PathLike): The path to the output YAML file.
131144
- `formatted_data` (dict): The formatted data to be written to the YAML file.
132145
133146
Writes the formatted data to the specified YAML file.
@@ -151,25 +164,25 @@ def map_type(self, input_str) -> str:
151164
mapping = {item.from_value: item.to_value for item in DECORATOR_TYPES}
152165
return mapping.get(input_str, input_str)
153166

154-
def format_results(self, results: Results) -> dict:
167+
def format_results(self, results: Results) -> FormattedData:
155168
"""
156169
Format the collected results into a structured data format for YAML.
157170
158171
Parameters:
159172
- `results` (list): List of dictionaries containing information about functions and classes.
160173
161174
Returns:
162-
- `formatted_data` (dict): Formatted data in a structured `yaml_language_server` compatible format.
175+
- `formatted_data` (FormattedData): Formatted data in a structured `yaml_language_server` compatible format.
163176
164177
This function formats a list of decorated data into the structure required by the `yaml_language_server`.
165178
It includes version information, requirement annotations, and relevant element information.
166179
"""
167180

168-
formatted_data = {}
169-
implementations = {}
170-
tests = {}
171-
requirement_annotations = {"implementations": implementations, "tests": tests}
172-
formatted_data["requirement_annotations"] = requirement_annotations
181+
implementations: dict[str, list[FormattedEntry]] = {}
182+
tests: dict[str, list[FormattedEntry]] = {}
183+
formatted_data: FormattedData = {
184+
"requirement_annotations": {"implementations": implementations, "tests": tests}
185+
}
173186

174187
for result in results:
175188
for decorator_info in result["decorators"]:
@@ -190,26 +203,28 @@ def format_results(self, results: Results) -> dict:
190203

191204
return formatted_data
192205

193-
def create_dir_from_path(self, filepath: str) -> None:
206+
def create_dir_from_path(self, filepath: str | os.PathLike) -> None:
194207
"""
195208
Creates directory of provided filepath if it does not exists
196209
197210
Parameters:
198-
- `filepath` (str): Filepath to check and create directory from.
211+
- `filepath` (str | PathLike): Filepath to check and create directory from.
199212
"""
200213
directory = os.path.dirname(filepath)
201214
if not os.path.exists(directory):
202215
os.makedirs(directory)
203216

204217
def process_decorated_data(
205-
self, path_to_python_files: str, output_file: str = "build/reqstool/annotations.yml"
218+
self,
219+
path_to_python_files: list[str | os.PathLike],
220+
output_file: str | os.PathLike = "build/reqstool/annotations.yml",
206221
) -> None:
207222
"""
208223
"Main" function, runs all functions resulting in a yaml file containing decorated data.
209224
210225
Parameters:
211226
- `path_to_python_files` (list): List of directories containing Python files.
212-
- `output_file` (str): Set path for output file, defaults to build/annotations.yml
227+
- `output_file` (str | PathLike): Set path for output file, defaults to build/annotations.yml
213228
214229
This method takes a list of directories containing Python files, collects decorated data from these files,
215230
formats the collected data, and writes the formatted results to YAML file for Requirements and SVCs annotations.

tests/unit/reqstool_decorators/processors/test_processors.py

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,28 @@ def process_decorator_instance():
1313
# ---------------------------------------------------------------------------
1414

1515

16-
def test_find_python_files(process_decorator_instance: DecoratorProcessor, tmpdir):
17-
tmpdir.join("pythonfile.py").write("content")
18-
result = process_decorator_instance.find_python_files(tmpdir)
19-
assert result == [str(tmpdir.join("pythonfile.py"))]
16+
def test_find_python_files(process_decorator_instance: DecoratorProcessor, tmp_path):
17+
(tmp_path / "pythonfile.py").write_text("content")
18+
result = process_decorator_instance.find_python_files(tmp_path)
19+
assert result == [str(tmp_path / "pythonfile.py")]
2020

2121

22-
def test_find_python_files_empty_dir(process_decorator_instance: DecoratorProcessor, tmpdir):
23-
result = process_decorator_instance.find_python_files(tmpdir)
22+
def test_find_python_files_empty_dir(process_decorator_instance: DecoratorProcessor, tmp_path):
23+
result = process_decorator_instance.find_python_files(tmp_path)
2424
assert result == []
2525

2626

27-
def test_find_python_files_nested(process_decorator_instance: DecoratorProcessor, tmpdir):
28-
sub = tmpdir.mkdir("sub")
29-
sub.join("nested.py").write("content")
30-
result = process_decorator_instance.find_python_files(tmpdir)
31-
assert str(sub.join("nested.py")) in result
27+
def test_find_python_files_nested(process_decorator_instance: DecoratorProcessor, tmp_path):
28+
sub = tmp_path / "sub"
29+
sub.mkdir()
30+
(sub / "nested.py").write_text("content")
31+
result = process_decorator_instance.find_python_files(tmp_path)
32+
assert str(sub / "nested.py") in result
3233

3334

34-
def test_find_python_files_ignores_non_py(process_decorator_instance: DecoratorProcessor, tmpdir):
35-
tmpdir.join("readme.txt").write("content")
36-
result = process_decorator_instance.find_python_files(tmpdir)
35+
def test_find_python_files_ignores_non_py(process_decorator_instance: DecoratorProcessor, tmp_path):
36+
(tmp_path / "readme.txt").write_text("content")
37+
result = process_decorator_instance.find_python_files(tmp_path)
3738
assert result == []
3839

3940

@@ -42,39 +43,39 @@ def test_find_python_files_ignores_non_py(process_decorator_instance: DecoratorP
4243
# ---------------------------------------------------------------------------
4344

4445

45-
def test_get_functions_and_classes(process_decorator_instance: DecoratorProcessor, tmpdir):
46-
file_path = str(tmpdir.join("test_file.py"))
47-
tmpdir.join("test_file.py").write('@SVCs("SVC_001")\nclass Test:\n pass')
46+
def test_get_functions_and_classes(process_decorator_instance: DecoratorProcessor, tmp_path):
47+
file_path = tmp_path / "test_file.py"
48+
file_path.write_text('@SVCs("SVC_001")\nclass Test:\n pass')
4849
process_decorator_instance.get_functions_and_classes(file_path, ["SVCs"])
4950
assert process_decorator_instance.req_svc_results[0]["name"] == "Test"
5051
assert process_decorator_instance.req_svc_results[0]["decorators"][0]["args"][0] == "SVC_001"
5152
assert process_decorator_instance.req_svc_results[0]["decorators"][0]["name"] == "SVCs"
5253
assert process_decorator_instance.req_svc_results[0]["elementKind"] == "CLASS"
5354

5455

55-
def test_get_functions_and_classes_function_def(process_decorator_instance: DecoratorProcessor, tmpdir):
56-
file_path = str(tmpdir.join("f.py"))
57-
tmpdir.join("f.py").write('@Requirements("REQ_001")\ndef my_func():\n pass')
56+
def test_get_functions_and_classes_function_def(process_decorator_instance: DecoratorProcessor, tmp_path):
57+
file_path = tmp_path / "f.py"
58+
file_path.write_text('@Requirements("REQ_001")\ndef my_func():\n pass')
5859
process_decorator_instance.get_functions_and_classes(file_path, ["Requirements"])
5960
assert len(process_decorator_instance.req_svc_results) == 1
6061
result = process_decorator_instance.req_svc_results[0]
6162
assert result["name"] == "my_func"
6263
assert result["elementKind"] == "FUNCTION"
6364

6465

65-
def test_get_functions_and_classes_async_function_def(process_decorator_instance: DecoratorProcessor, tmpdir):
66-
file_path = str(tmpdir.join("af.py"))
67-
tmpdir.join("af.py").write('@Requirements("REQ_001")\nasync def my_async():\n pass')
66+
def test_get_functions_and_classes_async_function_def(process_decorator_instance: DecoratorProcessor, tmp_path):
67+
file_path = tmp_path / "af.py"
68+
file_path.write_text('@Requirements("REQ_001")\nasync def my_async():\n pass')
6869
process_decorator_instance.get_functions_and_classes(file_path, ["Requirements"])
6970
assert len(process_decorator_instance.req_svc_results) == 1
7071
result = process_decorator_instance.req_svc_results[0]
7172
assert result["name"] == "my_async"
7273
assert result["elementKind"] == "ASYNCFUNCTION"
7374

7475

75-
def test_get_functions_and_classes_multiple_args(process_decorator_instance: DecoratorProcessor, tmpdir):
76-
file_path = str(tmpdir.join("m.py"))
77-
tmpdir.join("m.py").write('@Requirements("A", "B")\ndef func():\n pass')
76+
def test_get_functions_and_classes_multiple_args(process_decorator_instance: DecoratorProcessor, tmp_path):
77+
file_path = tmp_path / "m.py"
78+
file_path.write_text('@Requirements("A", "B")\ndef func():\n pass')
7879
process_decorator_instance.get_functions_and_classes(file_path, ["Requirements"])
7980
args = process_decorator_instance.req_svc_results[0]["decorators"][0]["args"]
8081
assert args == ["A", "B"]
@@ -89,28 +90,28 @@ def test_get_functions_and_classes_multiple_args(process_decorator_instance: Dec
8990
],
9091
)
9192
def test_get_functions_and_classes_element_kind(
92-
process_decorator_instance: DecoratorProcessor, tmpdir, code, expected_kind
93+
process_decorator_instance: DecoratorProcessor, tmp_path, code, expected_kind
9394
):
9495
"""Fix 4: elementKind must be derived from __class__.__name__, not CPython repr."""
95-
f = tmpdir.join("f.py")
96-
f.write(code)
97-
process_decorator_instance.get_functions_and_classes(str(f), ["Requirements"])
96+
f = tmp_path / "f.py"
97+
f.write_text(code)
98+
process_decorator_instance.get_functions_and_classes(f, ["Requirements"])
9899
assert process_decorator_instance.req_svc_results[0]["elementKind"] == expected_kind
99100

100101

101-
def test_get_functions_and_classes_no_match(process_decorator_instance: DecoratorProcessor, tmpdir):
102-
file_path = str(tmpdir.join("n.py"))
103-
tmpdir.join("n.py").write('@OtherDecorator("X")\ndef func():\n pass')
102+
def test_get_functions_and_classes_no_match(process_decorator_instance: DecoratorProcessor, tmp_path):
103+
file_path = tmp_path / "n.py"
104+
file_path.write_text('@OtherDecorator("X")\ndef func():\n pass')
104105
process_decorator_instance.get_functions_and_classes(file_path, ["Requirements"])
105106
assert process_decorator_instance.req_svc_results == []
106107

107108

108109
def test_get_functions_and_classes_multiple_decorators_on_func(
109-
process_decorator_instance: DecoratorProcessor, tmpdir
110+
process_decorator_instance: DecoratorProcessor, tmp_path
110111
):
111-
file_path = str(tmpdir.join("md.py"))
112+
file_path = tmp_path / "md.py"
112113
code = '@Requirements("REQ_001")\n@SVCs("SVC_001")\ndef func():\n pass'
113-
tmpdir.join("md.py").write(code)
114+
file_path.write_text(code)
114115
process_decorator_instance.get_functions_and_classes(file_path, ["Requirements", "SVCs"])
115116
assert len(process_decorator_instance.req_svc_results) == 1
116117
names = [d["name"] for d in process_decorator_instance.req_svc_results[0]["decorators"]]
@@ -140,7 +141,7 @@ def test_map_type_unknown_type(process_decorator_instance: DecoratorProcessor):
140141
# ---------------------------------------------------------------------------
141142

142143

143-
def test_format_results_implementations(process_decorator_instance: DecoratorProcessor, tmpdir):
144+
def test_format_results_implementations(process_decorator_instance: DecoratorProcessor):
144145
results = [
145146
{
146147
"fullyQualifiedName": "my.module.py",
@@ -252,22 +253,20 @@ def test_process_decorated_data_produces_yaml(process_decorator_instance: Decora
252253
src_file.parent.mkdir()
253254
src_file.write_text('@Requirements("REQ_001")\ndef my_func():\n pass\n')
254255

255-
output_file = str(tmp_path / "out" / "annotations.yml")
256+
output_file = tmp_path / "out" / "annotations.yml"
256257
process_decorator_instance.process_decorated_data(
257258
path_to_python_files=[str(src_file.parent)], output_file=output_file
258259
)
259260

260-
import os
261-
262-
assert os.path.exists(output_file)
261+
assert output_file.exists()
263262

264263

265264
def test_process_decorated_data_correct_structure(process_decorator_instance: DecoratorProcessor, tmp_path):
266265
src_file = tmp_path / "src" / "app.py"
267266
src_file.parent.mkdir()
268267
src_file.write_text('@Requirements("REQ_001")\ndef my_func():\n pass\n')
269268

270-
output_file = str(tmp_path / "out" / "annotations.yml")
269+
output_file = tmp_path / "out" / "annotations.yml"
271270
process_decorator_instance.process_decorated_data(
272271
path_to_python_files=[str(src_file.parent)], output_file=output_file
273272
)
@@ -285,7 +284,7 @@ def test_process_decorated_data_no_state_accumulation(process_decorator_instance
285284
src_file.parent.mkdir()
286285
src_file.write_text('@Requirements("REQ_001")\ndef my_func():\n pass\n')
287286

288-
output_file = str(tmp_path / "out" / "annotations.yml")
287+
output_file = tmp_path / "out" / "annotations.yml"
289288

290289
# Call twice
291290
process_decorator_instance.process_decorated_data(

0 commit comments

Comments
 (0)