make format.py clean up extra whitespace, also in .h files (#1991)

* make format.py clean up extra whitespace, also in .h files

* fixup

* cleanup whitespace msg

* Update check_format.py to also check non-src files touched by format.py

* format

format.py

@@ -4,6 +4,7 @@ import argparse
 import glob
 import multiprocessing
 import os
+from pathlib import Path
 import re
 import shutil
 import subprocess
@@ -94,14 +95,25 @@ def run_clang_apply_replacements(tmp_dir: str):
     subprocess.run(exec_str, shell=True)
 
 
-def add_final_new_line(file: str):
-    # https://backreference.org/2010/05/23/sanitizing-files-with-no-trailing-newline/index.html
-    # "gets the last character of the file pipes it into read, which will exit with a nonzero exit
-    # code if it encounters EOF before newline (so, if the last character of the file isn't a newline).
-    # If read exits nonzero, then append a newline onto the file using echo (if read exits 0,
-    # that satisfies the ||, so the echo command isn't run)." (https://stackoverflow.com/a/34865616)
-    exec_str = f"tail -c1 {file} | read -r _ || echo >> {file}"
-    subprocess.run(exec_str, shell=True)
+def cleanup_whitespace(file: str):
+    """
+    Remove whitespace at the end of lines,
+    ensure the file ends with an empty line.
+    """
+    file_p = Path(file)
+    contents = file_p.read_text(encoding="UTF-8")
+    modified = False
+
+    contents, n_subst = re.subn(r"[^\S\n]+\n", "\n", contents)
+    if n_subst != 0:
+        modified = True
+
+    if not contents.endswith("\n"):
+        contents += "\n"
+        modified = True
+
+    if modified:
+        file_p.write_text(contents, encoding="UTF-8")
 
 
 def format_files(src_files: List[str], extra_files: List[str], nb_jobs: int):
@@ -134,14 +146,24 @@ def format_files(src_files: List[str], extra_files: List[str], nb_jobs: int):
     else:
         run_clang_tidy(src_files)
 
-    print("Adding missing final new lines...")
-    # Adding final new lines is safe to do in parallel and can be applied to all types of files
+    print("Cleaning up whitespace...")
+    # Safe to do in parallel and can be applied to all types of files
     with multiprocessing.get_context("fork").Pool(nb_jobs) as pool:
-        pool.map(add_final_new_line, src_files + extra_files)
+        pool.map(cleanup_whitespace, src_files + extra_files)
 
     print("Done formatting files.")
 
 
+def list_files_to_format():
+    files = glob.glob("src/**/*.c", recursive=True)
+    extra_files = (
+        glob.glob("assets/**/*.xml", recursive=True)
+        + glob.glob("include/**/*.h", recursive=True)
+        + glob.glob("src/**/*.h", recursive=True)
+    )
+    return files, extra_files
+
+
 def main():
     parser = argparse.ArgumentParser(description="Format files in the codebase to enforce most style rules")
     parser.add_argument(
@@ -179,8 +201,7 @@ def main():
         files = args.files
         extra_files = []
     else:
-        files = glob.glob("src/**/*.c", recursive=True)
-        extra_files = glob.glob("assets/**/*.xml", recursive=True)
+        files, extra_files = list_files_to_format()
 
     format_files(files, extra_files, nb_jobs)
 

tools/check_format.py

@@ -23,14 +23,15 @@ def get_modified_files_to_format(compare_to):
     modified_files_str = subprocess.check_output(
         ["git", "diff", "--name-only", compare_to], text=True
     )
-    modified_files = modified_files_str.splitlines()
-    modified_c_files = [
-        f
-        for f in modified_files
-        if f.startswith("src" + os.path.sep) and f.endswith(".c")
-    ]
-    modified_c_files_existing = [f for f in modified_c_files if os.path.exists(f)]
-    return modified_c_files_existing
+    modified_files = set(modified_files_str.splitlines())
+
+    all_src_files, all_extra_files = format.list_files_to_format()
+    # Split modified_files between source files and extra files (see format.py)
+    # This also filters out deleted files that no logner exist
+    modified_src_files_existing = list(modified_files.intersection(all_src_files))
+    modified_extra_files_existing = list(modified_files.intersection(all_extra_files))
+
+    return modified_src_files_existing, modified_extra_files_existing
 
 
 def main():
@@ -40,21 +41,23 @@ def main():
     args = parser.parse_args()
 
     if args.compare_to:
-        src_files = get_modified_files_to_format(args.compare_to)
+        src_files, extra_files = get_modified_files_to_format(args.compare_to)
         if args.verbose:
-            print("Formatting specific files:", len(src_files), src_files)
-        if not src_files:
+            print("Formatting specific files:")
+            print(len(src_files), src_files)
+            print(len(extra_files), extra_files)
+        if not src_files and not extra_files:
             if args.verbose:
                 print("Nothing to format")
             exit(0)
     else:
-        src_files = glob.glob("src/**/*.c", recursive=True)
+        src_files, extra_files = format.list_files_to_format()
 
     nb_jobs = multiprocessing.cpu_count()
 
     git_status_pre = get_git_status()
 
-    format.format_files(src_files, [], nb_jobs)
+    format.format_files(src_files, extra_files, nb_jobs)
 
     git_status_post = get_git_status()