Add code formatting command (#111)

* add build to .gitignore

* add format utility

* black formatter

* spelling

* add whitelist feature

* minor review changes

* return runner status code

.github/actions/spelling/expect.txt

@@ -1,3 +1,4 @@
+bak
 cargs
 cexc
 cfg
@@ -59,6 +60,7 @@ fh
 filecmp
 FILEID
 fileobj
+filepath
 firest
 floordiv
 FPGA
@@ -111,6 +113,7 @@ kevin
 kwargs
 len
 lld
+llvm
 locs
 lstrip
 lxml
@@ -136,7 +139,7 @@ pathlib
 Peet
 pexpect
 Popen
-
+postprocessed
 proj
 ptf
 py
@@ -186,6 +189,7 @@ stackoverflow
 startswith
 staticmethod
 stderr
+stdin
 stdint
 stdout
 strftime
@@ -229,6 +233,7 @@ venv
 versioning
 viewcode
 whitelist
+whitespaces
 workdir
 www
 wxgui

.gitignore

@@ -43,6 +43,7 @@ core
 py_dict
 .settings
 
+/build/
 /ci-venv/
 /ci-logs*
 .eggs

src/fprime/util/build_helper.py

@@ -250,7 +250,7 @@ def utility_entry(args):
             if parsed.command not in ["purge", "info"]:
                 raise
         validate_tools_from_requirements(build)
-        runners[parsed.command](
+        status = runners[parsed.command](
             build, parsed, cmake_args, make_args, getattr(parsed, "pass_through", [])
         )
     except GenerateException as genex:
@@ -265,4 +265,4 @@ def utility_entry(args):
     except Exception as exc:
         print(f"[ERROR] {exc}", file=sys.stderr)
         return 1
-    return 0
+    return 0 if status is None else status

src/fprime/util/cli.py

@@ -1,7 +1,7 @@
 """ fprime.util.cli: general CLI handling
 
 Sets up parsers and processors for general CLI targets under fprime-util that do not fit elsewhere. Includes parsers
-such as: hast-to-file, info, and others.
+such as: hast-to-file, info, format, and others.
 
 @author mstarch
 """
@@ -9,9 +9,10 @@ import argparse
 import sys
 
 from pathlib import Path
-from typing import Dict, Callable
+from typing import List, Dict, Callable
 from fprime.fbuild.builder import Build, InvalidBuildCacheException
 from fprime.fbuild.interaction import new_component, new_port
+from fprime.util.code_formatter import ClangFormatter
 
 
 def print_info(
@@ -121,6 +122,51 @@ def template(
     print("[ERROR] Use --component or --port, not both.", file=sys.stderr)
 
 
+def run_code_format(
+    build: Build,
+    parsed: argparse.Namespace,
+    _: Dict[str, str],
+    __: Dict[str, str],
+    ___: List[str],
+):
+    """Runs code formatting using clang-format
+
+    Args:
+        build: used to retrieve .clang-format file
+        parsed: parsed input arguments
+        __: unused cmake_args
+        ___: unused make_args
+        ____: unused pass-through arguments
+    """
+    options = {
+        "verbose": not parsed.quiet,
+        "backup": not parsed.no_backup,
+        "validate_extensions": not parsed.force,
+    }
+    clang_formatter = ClangFormatter(
+        "clang-format",
+        build.settings.get("framework_path", Path(".")) / ".clang-format",
+        options,
+    )
+    if not clang_formatter.is_supported():
+        print(
+            f"[ERROR] Cannot find executable: {clang_formatter.executable}. Unable to run formatting.",
+            file=sys.stderr,
+        )
+        return 1
+    # Allow requested file extensions
+    for file_ext in parsed.allow_extension:
+        clang_formatter.allow_extension(file_ext)
+    # Stage all files that are passed through stdin, if requested
+    if parsed.stdin:
+        for filename in sys.stdin.read().split():
+            clang_formatter.stage_file(Path(filename))
+    # Stage all files that are passed through --files
+    for filename in parsed.files:
+        clang_formatter.stage_file(Path(filename))
+    return clang_formatter.execute(build, parsed.path, ({}, parsed.pass_through))
+
+
 def add_special_parsers(
     subparsers, common: argparse.ArgumentParser, help_text: "HelpText"
 ) -> Dict[str, Callable]:
@@ -179,4 +225,54 @@ def add_special_parsers(
         action="store_true",
         help="Tells the new command to generate a port",
     )
-    return {"hash-to-file": hash_to_file, "info": print_info, "new": template}
+
+    # Code formatting with clang-format
+    format_parser = subparsers.add_parser(
+        "format",
+        help=help_text.short("format"),
+        description=help_text.long("format"),
+        parents=[common],
+        add_help=False,
+        formatter_class=argparse.RawDescriptionHelpFormatter,
+        conflict_handler="resolve",
+    ).add_argument_group("format utility arguments")
+    format_parser.add_argument(
+        "-x", "--no-backup", action="store_true", help="Disable backups"
+    )
+    format_parser.add_argument(
+        "-q", "--quiet", action="store_true", help="Disable clang-format verbose mode"
+    )
+    format_parser.add_argument(
+        "--force",
+        action="store_true",
+        help="Force all listed files to be passed to clang-format (no file extension check)",
+    )
+    format_parser.add_argument(
+        "--allow-extension",
+        action="append",
+        default=[],
+        help="Add a file extension to the allowed set",
+    )
+    format_parser.add_argument(
+        "-f",
+        "--files",
+        nargs="*",
+        default=[],
+        type=Path,
+        help="List of files to format",
+    )
+    format_parser.add_argument(
+        "--stdin", action="store_true", help="Read stdin for list of files to format"
+    )
+    format_parser.add_argument(
+        "--pass-through",
+        nargs=argparse.REMAINDER,
+        default=[],
+        help="If specified, --pass-through must be the last argument. Remaining arguments passed to underlying executable",
+    )
+    return {
+        "hash-to-file": hash_to_file,
+        "info": print_info,
+        "new": template,
+        "format": run_code_format,
+    }

src/fprime/util/code_formatter.py

@@ -0,0 +1,155 @@
+""" fprime.fbuild.code_formatter
+
+Wrapper for clang-format utility.
+
+@author thomas-bc
+"""
+
+from typing import Dict, List, Tuple
+
+from fprime.fbuild.target import ExecutableAction, TargetScope
+from pathlib import Path
+
+import subprocess
+import shutil
+import re
+
+
+# MARKER is needed to differentiate at postprocess between access specifiers
+# that were previously an uppercase MACRO, and those that were originally lowercase.
+# MARKER must be a comment for the formatting to behave - so might as well make it
+# a meaningful warning in case it's not postprocessed correctly
+MARKER = "// WARNING: fprime-util format mishap"
+
+# POST pattern is different because the formatting will likely introduce whitespaces
+PRIVATE_PRE_PATTERN = f"private:{MARKER}"
+PRIVATE_POST_PATTERN = f"private:[\s]*{MARKER}"
+PROTECTED_PRE_PATTERN = f"protected:{MARKER}"
+PROTECTED_POST_PATTERN = f"protected:[\s]*{MARKER}"
+STATIC_PRE_PATTERN = f"static:{MARKER}"
+STATIC_POST_PATTERN = f"static:[\s]*{MARKER}"
+
+# clang-format will try to format everything it is given - restrict for the time being
+ALLOWED_EXTENSIONS = [
+    ".cpp",
+    ".c++",
+    ".cxx",
+    ".cc",
+    ".c",
+    ".hpp",
+    ".h++",
+    ".hxx",
+    ".hh",
+    ".h",
+]
+
+
+class ClangFormatter(ExecutableAction):
+    """Class encapsulating the clang-format logic for fprime-util"""
+
+    def __init__(self, executable: str, style_file: "Path", options: Dict):
+        super().__init__(TargetScope.LOCAL)
+        self.executable = executable
+        self.style_file = style_file
+        self.backup = options["backup"]
+        self.verbose = options["verbose"]
+        self.validate_extensions = options["validate_extensions"]
+        self.allowed_extensions = ALLOWED_EXTENSIONS.copy()
+        self._files_to_format: List[Path] = []
+
+    def is_supported(self) -> bool:
+        return bool(shutil.which(self.executable))
+
+    def allow_extension(self, file_ext: str) -> None:
+        """Add a file extension str to the list of allowed extension"""
+        self.allowed_extensions.append(file_ext)
+
+    def stage_file(self, filepath: Path) -> None:
+        """Request ClangFormatter to consider the file for formatting.
+        If the file exists and its extension matches a known C/C++ format,
+        it will be passed to clang-format when the execute() function is called.
+
+        Args:
+            filepath (str): file path to file to be staged.
+        """
+        if not filepath.is_file():
+            if self.verbose:
+                print(f"[INFO] Skipping {filepath} : is not a file.")
+        elif self.validate_extensions and (
+            filepath.suffix not in self.allowed_extensions
+        ):
+            if self.verbose:
+                print(
+                    f"[INFO] Skipping {filepath} : unrecognized C/C++ file extension "
+                    f"('{filepath.suffix}'). Use --allow-extension or --force."
+                )
+        else:
+            self._files_to_format.append(filepath)
+
+    def _preprocess_files(self) -> None:
+        """Preprocess a file to ensure that clang-format behaves.
+        This is because of the access specifier macros (e.g. PROTECTED)
+        that are defined in F', which clang-format does not recognize
+        """
+        for filepath in self._files_to_format:
+            # It is unsafe to write to file while reading from it
+            # Better to read in memory, close the file, then re-open to write out from memory
+            with open(filepath, "r") as file:
+                content = file.read()
+            # Replace the strings in the file content
+            content = re.sub("PROTECTED:", PROTECTED_PRE_PATTERN, content)
+            content = re.sub("PRIVATE:", PRIVATE_PRE_PATTERN, content)
+            content = re.sub("STATIC:", STATIC_PRE_PATTERN, content)
+            # Write the file out to the same location, seemingly in-place
+            with open(filepath, "w") as file:
+                file.write(content)
+
+    def _postprocess_files(self) -> None:
+        """Postprocess a file to restore the access specifier macros."""
+        for filepath in self._files_to_format:
+            # Same logic as _preprocess_files()
+            with open(filepath, "r") as file:
+                content = file.read()
+            content = re.sub(PROTECTED_POST_PATTERN, "PROTECTED:", content)
+            content = re.sub(PRIVATE_POST_PATTERN, "PRIVATE:", content)
+            content = re.sub(STATIC_POST_PATTERN, "STATIC:", content)
+            with open(filepath, "w") as file:
+                file.write(content)
+
+    def execute(
+        self, builder: "Build", context: "Path", args: Tuple[Dict[str, str], List[str]]
+    ):
+        """Execute clang-format on the files that were staged.
+
+        Args:
+            builder (Build): build object to run the utility with
+            context (Path): context path of module clang-format can run on if --module is provided
+            args (Tuple[Dict[str, str], List[str]]): extra arguments to supply to the utility
+        """
+
+        if len(self._files_to_format) == 0:
+            print("[INFO] No files were formatted.")
+            return 0
+        if not self.style_file.is_file():
+            print(
+                f"[ERROR] No .clang-format file found in {self.style_file.parent}. "
+                "Override location with --pass-through --style=file:<path>."
+            )
+            return 1
+        # Backup files unless --no-backup is requested
+        if self.backup:
+            for file in self._files_to_format:
+                shutil.copy2(file, file.parent / (file.stem + ".bak" + file.suffix))
+        pass_through = args[1]
+        self._preprocess_files()
+        clang_args = [
+            self.executable,
+            "-i",
+            f"--style=file:{self.style_file}",
+            *(["--verbose"] if self.verbose else []),
+            *pass_through,
+            *self._files_to_format,
+        ]
+        status = subprocess.run(clang_args)
+        self._postprocess_files()
+        return status.returncode

src/fprime/util/help_text.py

@@ -281,6 +281,42 @@ Examples:
 
 WARNING: prototype code. Not recommended for inexperienced users. '{EXECUTABLE} new' runs a wizard to create new ports
 and components in fprime. The code has not been updated to use FPP models. Please check back later.
+""",
+    "format": f"""{EXECUTABLE} format ({VERSION}): Formats C/C++ files using clang-format
+
+'{EXECUTABLE} format' uses 'clang-format' to format C/C++ files. It uses the style specified in the .clang-format file
+found at the root of the F' framework used by the project (i.e. the 'framework_path' specified in settings.ini).
+Files are specified through stdin or by using the '--files [<path/to/file>]*' flag. When reading from stdin, file paths
+should be separated by whitespace characters.
+Because clang-format will try to format any text file it is fed, '{EXECUTABLE} format' restricts by default the files
+that are processed to commonly used C/C++ file extensions (cpp, c++, cxx, cc, c, and their 'h' equivalents). Backup
+copies of the formatted files are also created by default.
+
+Note: '{EXECUTABLE} format' requires that the 'clang-format' utility is installed and in the PATH.
+More information at https://clang.llvm.org/docs/ClangFormat.html
+
+Examples:
+
+  -- Manual usage --
+  {EXECUTABLE} format -f Main.cpp Main.hpp
+  {EXECUTABLE} format -f Imu/*
+  {EXECUTABLE} format -f *.hpp --pass-through --dry-run
+  
+  -- From stdin using Git | format all changed files --
+  git diff --name-only --relative | {EXECUTABLE} format --stdin
+
+  -- Format all C/C++ files within a module --
+  cd Ref/SignalGen
+  find . | {EXECUTABLE} format --stdin
+
+  -- With file list --
+  {EXECUTABLE} format --stdin < files-to-format.txt
+  {EXECUTABLE} format --files OtherFile.cpp --stdin < files-to-format.txt
+
+  -- Allow additional file extension -- 
+  {EXECUTABLE} format -f Main.cpp main.py --allow-extension .py
+
+
 """,
 }