diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 0e13c04..44fa936 100644 --- a/.github/actions/spelling/expect.txt +++ b/.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 diff --git a/.gitignore b/.gitignore index 44d0cae..9797ce9 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ core py_dict .settings +/build/ /ci-venv/ /ci-logs* .eggs diff --git a/src/fprime/util/build_helper.py b/src/fprime/util/build_helper.py index dba568e..d9066d7 100644 --- a/src/fprime/util/build_helper.py +++ b/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 diff --git a/src/fprime/util/cli.py b/src/fprime/util/cli.py index bbb2bc8..8cd301d 100644 --- a/src/fprime/util/cli.py +++ b/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, + } diff --git a/src/fprime/util/code_formatter.py b/src/fprime/util/code_formatter.py new file mode 100644 index 0000000..9d5ada2 --- /dev/null +++ b/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:." + ) + 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 diff --git a/src/fprime/util/help_text.py b/src/fprime/util/help_text.py index 1aeb442..a464066 100644 --- a/src/fprime/util/help_text.py +++ b/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 []*' 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 + + """, }