Skip to content

Commit 0ee281d

Browse files
committed
fix: end every written file with a newline
Whole-file rewrites come back from the API without their final newline, so a unit-test script that tests on a linter's final-newline rule (RuboCop Layout/TrailingEmptyLines, ESLint eol-last, Ruff W292) would fail every fix round.
1 parent 2f721a4 commit 0ee281d

2 files changed

Lines changed: 73 additions & 2 deletions

File tree

‎file_utils.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,20 @@ def get_existing_files_content(build_folder, existing_files):
142142
return existing_files_content
143143

144144

145+
def ensure_final_newline(content: str) -> str:
146+
"""Ends non-empty text with a newline, as linters expect (e.g. RuboCop, ESLint eol-last, Ruff W292)."""
147+
if content and not content.endswith("\n"):
148+
return content + "\n"
149+
return content
150+
151+
145152
def store_response_files(target_folder, response_files, existing_files):
153+
"""Writes the files returned by the API to target_folder. None content deletes the file.
154+
155+
Every written file ends with a newline - whole-file rewrites come back from the API without one. The
156+
added newline is also written back into response_files, so callers that diff response_files against
157+
the folder afterwards see exactly what is on disk.
158+
"""
146159
for file_name in response_files:
147160
full_file_name = os.path.join(target_folder, file_name)
148161

@@ -156,6 +169,8 @@ def store_response_files(target_folder, response_files, existing_files):
156169

157170
continue
158171

172+
response_files[file_name] = ensure_final_newline(response_files[file_name])
173+
159174
os.makedirs(os.path.dirname(full_file_name), exist_ok=True)
160175

161176
with open(full_file_name, "w", encoding="utf-8") as f:

‎tests/test_file_utils.py‎

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

44
import pytest
55

6-
from file_utils import load_linked_resources, store_response_files
6+
from diff_utils import get_code_diff
7+
from file_utils import get_existing_files_content, load_linked_resources, store_response_files
78
from plain2code_exceptions import UnsupportedBase64Content, UnsupportedResourceType
89
from plain2code_utils import MAX_BASE64_BLOB_LENGTH
910

@@ -84,9 +85,64 @@ def test_load_linked_resources_real_base64_image_raises(template_dir):
8485
def test_store_response_files_writes_unicode_as_utf8(template_dir):
8586
# Content with a non-cp1252 character (📍 U+1F4CD) must be written as UTF-8
8687
# regardless of the platform's default text encoding (e.g. cp1252 on Windows).
87-
content = "Location 📍 marker"
88+
content = "Location 📍 marker\n"
8889
store_response_files(template_dir, {"notes.md": content}, [])
8990

9091
file_path = os.path.join(template_dir, "notes.md")
9192
with open(file_path, "rb") as f:
9293
assert f.read().decode("utf-8") == content
94+
95+
96+
def _read_bytes(folder, file_name):
97+
with open(os.path.join(folder, file_name), "rb") as f:
98+
return f.read().decode("utf-8")
99+
100+
101+
@pytest.mark.parametrize(
102+
"content, expected",
103+
[
104+
("class Foo\nend", "class Foo\nend\n"),
105+
("class Foo\nend\n", "class Foo\nend\n"),
106+
("class Foo\nend\n\n", "class Foo\nend\n\n"),
107+
("class Foo\r\nend\r\n", "class Foo\r\nend\r\n"),
108+
("", ""),
109+
],
110+
)
111+
def test_store_response_files_ends_every_text_file_with_a_newline(template_dir, content, expected):
112+
# Linters gate on a final newline (RuboCop Layout/TrailingEmptyLines, ESLint eol-last, Ruff W292), so a
113+
# whole-file rewrite that comes back without one must not reach the disk that way.
114+
store_response_files(template_dir, {"app/foo.rb": content}, [])
115+
116+
assert _read_bytes(template_dir, "app/foo.rb") == expected
117+
118+
119+
def test_store_response_files_hands_back_the_content_it_wrote(template_dir):
120+
# Callers diff the response files against the build folder after storing them (fix journal, conformance
121+
# fixes handed to the unit tests fixer), so the response files must match what is on disk.
122+
response_files = {"app/foo.rb": "class Foo\nend", "app/bar.rb": "class Bar\nend\n"}
123+
124+
store_response_files(template_dir, response_files, [])
125+
126+
assert response_files == {"app/foo.rb": "class Foo\nend\n", "app/bar.rb": "class Bar\nend\n"}
127+
128+
129+
def test_rewriting_a_file_without_its_final_newline_does_not_change_it(template_dir):
130+
existing_files = store_response_files(template_dir, {"app/foo.rb": "class Foo\nend\n"}, [])
131+
existing_files_content = get_existing_files_content(template_dir, existing_files)
132+
133+
rewrite = {"app/foo.rb": "class Foo\nend"}
134+
store_response_files(template_dir, rewrite, existing_files)
135+
136+
assert get_existing_files_content(template_dir, existing_files) == existing_files_content
137+
assert get_code_diff(rewrite, existing_files_content) == {}
138+
139+
140+
def test_store_response_files_still_deletes_a_file_whose_content_is_none(template_dir):
141+
existing_files = store_response_files(template_dir, {"app/foo.rb": "class Foo\nend\n"}, [])
142+
143+
response_files = {"app/foo.rb": None}
144+
store_response_files(template_dir, response_files, existing_files)
145+
146+
assert not os.path.exists(os.path.join(template_dir, "app/foo.rb"))
147+
assert existing_files == []
148+
assert response_files == {"app/foo.rb": None}

0 commit comments

Comments
 (0)