Skip to content

Commit

Permalink
Preserve line endings when editing file in insert_license hook (#84)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yiftachkarkason
yiftachkarkason authored Aug 13, 2023
1 parent 9890e12 commit 27c596f
Show file tree
Hide file tree
Showing 2 changed files with 222 additions and 10 deletions.
13 changes: 7 additions & 6 deletions pre_commit_hooks/insert_license.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def get_license_info(args) -> LicenseInfo:
)
if "|" in comment_prefix:
comment_start, comment_prefix, comment_end = comment_prefix.split("|")
with open(args.license_filepath, encoding="utf8") as license_file:
with open(args.license_filepath, encoding="utf8", newline="") as license_file:
plain_license = license_file.readlines()

if args.use_current_year:
Expand Down Expand Up @@ -268,7 +268,7 @@ def _read_file_content(src_filepath):
"ISO-8859-1",
): # we could use the chardet library to support more encodings
try:
with open(src_filepath, encoding=encoding) as src_file:
with open(src_filepath, encoding=encoding, newline="") as src_file:
return src_file.readlines(), encoding
except UnicodeDecodeError as error:
last_error = error
Expand Down Expand Up @@ -324,7 +324,7 @@ def license_not_found( # pylint: disable=too-many-arguments
+ [license_info.eol]
+ src_file_content[index:]
)
with open(src_filepath, "w", encoding=encoding) as src_file:
with open(src_filepath, "w", encoding=encoding, newline="") as src_file:
src_file.write("".join(src_file_content))
return True
return False
Expand Down Expand Up @@ -458,7 +458,7 @@ def license_found(
)

if updated:
with open(src_filepath, "w", encoding=encoding) as src_file:
with open(src_filepath, "w", encoding=encoding, newline="") as src_file:
src_file.write("".join(src_file_content))

return updated
Expand Down Expand Up @@ -494,7 +494,7 @@ def fuzzy_license_found(
]
+ src_file_content[fuzzy_match_header_index:]
)
with open(src_filepath, "w", encoding=encoding) as src_file:
with open(src_filepath, "w", encoding=encoding, newline="") as src_file:
src_file.write("".join(src_file_content))
return True

Expand Down Expand Up @@ -661,7 +661,8 @@ def get_license_candidate_string(candidate_array, license_info):
)
else:
in_license = True
found_license_offset = current_offset # We have no data :(. We start license immediately
# We have no data :(. We start license immediately
found_license_offset = current_offset
else:
if stripped_comment_end and stripped_line.startswith(stripped_comment_end):
break
Expand Down
219 changes: 215 additions & 4 deletions tests/insert_license_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,40 @@

from .utils import chdir_to_test_resources, capture_stdout


# pylint: disable=too-many-arguments


def _convert_line_ending(file_path, new_line_endings):
for encoding in (
"utf8",
"ISO-8859-1",
): # we could use the chardet library to support more encodings
last_error = None
try:
with open(file_path, encoding=encoding, newline="") as f_in:
content = f_in.read()

with open(
file_path, "w", encoding=encoding, newline=new_line_endings
) as f_out:
f_out.write(content)

return
except UnicodeDecodeError as error:
last_error = error
print(
f"Error while processing: {file_path} - file encoding is probably not supported"
)
if last_error is not None: # Avoid mypy message
raise last_error
raise RuntimeError("Unexpected branch taken (_convert_line_ending)")


@pytest.mark.parametrize(
(
"license_file_path",
"line_ending",
"src_file_path",
"comment_prefix",
"new_src_file_expected",
Expand All @@ -22,13 +50,14 @@
"extra_args",
),
map(
lambda a: a[:1] + a[1],
lambda a: a[:2] + a[2],
chain(
product( # combine license files with other args
(
"LICENSE_with_trailing_newline.txt",
"LICENSE_without_trailing_newline.txt",
),
("\n", "\r\n"),
(
(
"module_without_license.py",
Expand Down Expand Up @@ -209,10 +238,182 @@
True,
["--use-current-year"],
),
(
"module_without_license.py",
"#",
"module_with_license.py",
"",
True,
None,
),
("module_without_license_skip.py", "#", None, "", False, None),
("module_with_license.py", "#", None, "", False, None),
("module_with_license_todo.py", "#", None, "", True, None),
(
"module_without_license.jinja",
"{#||#}",
"module_with_license.jinja",
"",
True,
None,
),
(
"module_without_license_skip.jinja",
"{#||#}",
None,
"",
False,
None,
),
("module_with_license.jinja", "{#||#}", None, "", False, None),
("module_with_license_todo.jinja", "{#||#}", None, "", True, None),
(
"module_without_license_and_shebang.py",
"#",
"module_with_license_and_shebang.py",
"",
True,
None,
),
(
"module_without_license_and_shebang_skip.py",
"#",
None,
"",
False,
None,
),
("module_with_license_and_shebang.py", "#", None, "", False, None),
(
"module_with_license_and_shebang_todo.py",
"#",
None,
"",
True,
None,
),
(
"module_without_license.groovy",
"//",
"module_with_license.groovy",
"",
True,
None,
),
("module_without_license_skip.groovy", "//", None, "", False, None),
("module_with_license.groovy", "//", None, "", False, None),
("module_with_license_todo.groovy", "//", None, "", True, None),
(
"module_without_license.css",
"/*| *| */",
"module_with_license.css",
"",
True,
None,
),
(
"module_without_license_and_few_words.css",
"/*| *| */",
"module_with_license_and_few_words.css",
"",
True,
None,
), # Test fuzzy match does not match greedily
(
"module_without_license_skip.css",
"/*| *| */",
None,
"",
False,
None,
),
("module_with_license.css", "/*| *| */", None, "", False, None),
("module_with_license_todo.css", "/*| *| */", None, "", True, None),
(
"main_without_license.cpp",
"/*|\t| */",
"main_with_license.cpp",
"",
True,
None,
),
(
"main_iso8859_without_license.cpp",
"/*|\t| */",
"main_iso8859_with_license.cpp",
"",
True,
None,
),
(
"module_without_license.txt",
"",
"module_with_license_noprefix.txt",
"",
True,
None,
),
(
"module_without_license.py",
"#",
"module_with_license_nospace.py",
"",
True,
["--no-space-in-comment-prefix"],
),
(
"module_without_license.php",
"/*| *| */",
"module_with_license.php",
"",
True,
["--insert-license-after-regex", "^<\\?php$"],
),
(
"module_without_license.py",
"#",
"module_with_license_noeol.py",
"",
True,
["--no-extra-eol"],
),
(
"module_without_license.groovy",
"//",
"module_with_license.groovy",
"",
True,
["--use-current-year"],
),
(
"module_with_stale_year_in_license.py",
"#",
"module_with_year_range_in_license.py",
"",
True,
["--use-current-year"],
),
(
"module_with_stale_year_range_in_license.py",
"#",
"module_with_year_range_in_license.py",
"",
True,
["--use-current-year"],
),
(
"module_with_badly_formatted_stale_year_range_in_license.py",
"#",
"module_with_badly_formatted_stale_year_range_in_license.py",
"module_with_badly_formatted_stale_year_range_in_license.py",
True,
["--use-current-year"],
),
),
),
product(
("LICENSE_with_year_range_and_trailing_newline.txt",),
("\n", "\r\n"),
(
(
"module_without_license.groovy",
Expand All @@ -229,6 +430,7 @@
)
def test_insert_license(
license_file_path,
line_ending,
src_file_path,
comment_prefix,
new_src_file_expected,
Expand All @@ -241,6 +443,7 @@ def test_insert_license(
with chdir_to_test_resources():
path = tmpdir.join(src_file_path)
shutil.copy(src_file_path, path.strpath)
_convert_line_ending(path.strpath, line_ending)
args = [
"--license-filepath",
license_file_path,
Expand All @@ -257,7 +460,7 @@ def test_insert_license(

if new_src_file_expected:
with open(
new_src_file_expected, encoding=encoding
new_src_file_expected, encoding=encoding, newline=line_ending
) as expected_content_file:
expected_content = expected_content_file.read()
if "--use-current-year" in args:
Expand Down Expand Up @@ -315,18 +518,20 @@ def test_insert_license_current_year_already_there(
@pytest.mark.parametrize(
(
"license_file_path",
"line_ending",
"src_file_path",
"comment_style",
"new_src_file_expected",
"fail_check",
),
map(
lambda a: a[:1] + a[1],
lambda a: a[:2] + a[2],
product( # combine license files with other args
(
"LICENSE_with_trailing_newline.txt",
"LICENSE_without_trailing_newline.txt",
),
("\n", "\r\n"),
(
(
"module_without_license.jinja",
Expand Down Expand Up @@ -393,6 +598,7 @@ def test_insert_license_current_year_already_there(
)
def test_fuzzy_match_license(
license_file_path,
line_ending,
src_file_path,
comment_style,
new_src_file_expected,
Expand All @@ -402,6 +608,7 @@ def test_fuzzy_match_license(
with chdir_to_test_resources():
path = tmpdir.join("src_file_path")
shutil.copy(src_file_path, path.strpath)
_convert_line_ending(path.strpath, line_ending)
args = [
"--license-filepath",
license_file_path,
Expand Down Expand Up @@ -470,6 +677,7 @@ def test_is_license_present(src_file_content, expected_index, match_years_strict
@pytest.mark.parametrize(
(
"license_file_path",
"line_ending",
"src_file_path",
"comment_style",
"fuzzy_match",
Expand All @@ -478,12 +686,13 @@ def test_is_license_present(src_file_content, expected_index, match_years_strict
"use_current_year",
),
map(
lambda a: a[:1] + a[1],
lambda a: a[:2] + a[2],
product( # combine license files with other args
(
"LICENSE_with_trailing_newline.txt",
"LICENSE_without_trailing_newline.txt",
),
("\n", "\r\n"),
(
(
"module_with_license.css",
Expand Down Expand Up @@ -625,6 +834,7 @@ def test_is_license_present(src_file_content, expected_index, match_years_strict
)
def test_remove_license(
license_file_path,
line_ending,
src_file_path,
comment_style,
fuzzy_match,
Expand All @@ -636,6 +846,7 @@ def test_remove_license(
with chdir_to_test_resources():
path = tmpdir.join("src_file_path")
shutil.copy(src_file_path, path.strpath)
_convert_line_ending(path.strpath, line_ending)
argv = [
"--license-filepath",
license_file_path,
Expand Down

0 comments on commit 27c596f

Please sign in to comment.