Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/fprime/util/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def skip_build_loading(parsed):
"""Determines if the build load step should be skipped. Commands that do not require a build object
should manually be added here by the developer.
"""
if parsed.command == "version-check":
if parsed.command in ["version-check", "format"]:
return True
return False

Expand All @@ -73,7 +73,6 @@ def skip_build_cache_validation(parsed):
if parsed.command in [
"purge",
"info",
"format",
]:
return True
if parsed.command == "new" and parsed.new_deployment:
Expand Down
3 changes: 2 additions & 1 deletion src/fprime/util/code_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ def execute(
args (Tuple[Dict[str, str], List[str]]): extra arguments to supply to the utility
"""
combined_env = os.environ.copy()
combined_env.update(builder.settings.get("environment", {}))
if builder is not None:
combined_env.update(builder.settings.get("environment", {}))

if len(self._files_to_format) == 0:
print("[INFO] No files were formatted.")
Expand Down
52 changes: 50 additions & 2 deletions src/fprime/util/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import importlib.metadata


from fprime.common.error import FprimeException
from fprime.fbuild.builder import Build, InvalidBuildCacheException
from fprime.fbuild.settings import IniSettings
from fprime.fbuild.types import UnableToDetectProjectException
from fprime.util.code_formatter import ClangFormatter
from .versioning import VersionException, FPRIME_PIP_PACKAGES
from fprime.util.cookiecutter_wrapper import (
Expand Down Expand Up @@ -154,6 +157,51 @@ def run_new(
)


def locate_clang_format_file(parsed: argparse.Namespace) -> Path:
"""Locate the .clang-format style file to use for formatting.

The `format` command must work both inside an F´ project (which declares a
`framework_path` in settings.ini) and inside a standalone F´ library (which
has neither a settings.ini nor a project root). Discovery proceeds as:

1. If a parent F´ project can be detected, use the `.clang-format` at the
root of the framework it points to (the historical behavior).
2. Otherwise, walk up the directory tree from the working path looking for
a `.clang-format` file. This mirrors clang-format's own discovery rules
and lets libraries supply their own style file.

Args:
parsed: parsed input arguments

Returns:
Path to the .clang-format file to use (may not exist; the caller
reports a clear error in that case).
"""
# Try to resolve the framework's .clang-format via project settings
try:
cmake_root = (
Path(parsed.root)
if parsed.root is not None
else Build.find_nearest_parent_project(Path.cwd())
)
settings = IniSettings.load(cmake_root / "settings.ini")
framework_path = settings.get("framework_path")
if framework_path is not None:
return Path(framework_path) / ".clang-format"
except (UnableToDetectProjectException, FprimeException):
pass # Not in a project (e.g. a standalone library); fall back to search

# Library fallback: walk up from the working path searching for a .clang-format

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default clang-format behavior so I think it'd be better to just delegate to it, and not rewrite the logic ourselves.

start = parsed.path if parsed.path is not None else Path.cwd()
for directory in [Path(start).resolve(), *Path(start).resolve().parents]:
candidate = directory / ".clang-format"
if candidate.is_file():
return candidate
# Nothing found: return the working-directory candidate so the error message
# points at a sensible location
return Path(start).resolve() / ".clang-format"


def run_code_format(
build: Build,
parsed: argparse.Namespace,
Expand All @@ -164,7 +212,7 @@ def run_code_format(
"""Runs code formatting using clang-format

Args:
build: used to retrieve .clang-format file
build: unused; format runs without a build cache (may be None)
parsed: parsed input arguments
__: unused cmake_args
___: unused make_args
Expand All @@ -179,7 +227,7 @@ def run_code_format(
}
clang_formatter = ClangFormatter(
"clang-format",
build.settings.get("framework_path", Path(".")) / ".clang-format",
locate_clang_format_file(parsed),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be useful to print out the found location in the case that --verbose is specified

options,
)
if not clang_formatter.is_supported():
Expand Down
84 changes: 84 additions & 0 deletions test/fprime/util/test_code_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
Tests for fprime.util.code_formatter
"""

import argparse
from pathlib import Path

import shutil
from unittest.mock import MagicMock

import pytest
from fprime.util.code_formatter import ClangFormatter
from fprime.util.commands import locate_clang_format_file, run_code_format


def test_init():
Expand Down Expand Up @@ -143,3 +145,85 @@ def test_execute_check_pass(tmp_path, mock_build, style_file):

result = formatter.execute(mock_build, tmp_path, ({}, []))
assert result == 0


def test_execute_no_build(tmp_path, style_file):
"""Test that execute works when no build object is provided (library case)"""
malformed_src = DATA_DIR / "malformed.cpp"
malformed_dst = tmp_path / "malformed.cpp"
shutil.copy(malformed_src, malformed_dst)

options = {"backup": False, "verbose": False, "quiet": True, "check": False}
formatter = ClangFormatter("clang-format", style_file, options)
formatter.stage_file(malformed_dst)

# build=None must not raise (libraries run format without a build cache)
result = formatter.execute(None, tmp_path, ({}, []))
assert result == 0


def test_locate_clang_format_file_in_library(tmp_path, monkeypatch):
"""A library's own .clang-format is found by walking up from the working path"""
library_root = tmp_path / "fprime-zephyr"
sub_dir = library_root / "Svc"
sub_dir.mkdir(parents=True)
style = library_root / ".clang-format"
style.write_text("BasedOnStyle: LLVM\n")

# Run from inside the library, with no project/settings.ini in any parent
monkeypatch.chdir(sub_dir)
parsed = argparse.Namespace(root=None, path=Path.cwd())

located = locate_clang_format_file(parsed)
assert located == style


def test_locate_clang_format_file_missing(tmp_path, monkeypatch):
"""When no .clang-format exists, a candidate path under the working dir is returned"""
work_dir = tmp_path / "lib-no-style"
work_dir.mkdir()
monkeypatch.chdir(work_dir)
parsed = argparse.Namespace(root=None, path=Path.cwd())

located = locate_clang_format_file(parsed)
# The returned path does not exist, so the caller surfaces a clear error
assert not located.is_file()
assert located.name == ".clang-format"


def test_run_code_format_in_library(tmp_path, monkeypatch):
"""End-to-end: format a library directory with no settings.ini, using its own style file"""
if shutil.which("clang-format") is None:
pytest.skip("clang-format executable not available")

library_root = tmp_path / "fprime-zephyr"
svc_dir = library_root / "Svc"
svc_dir.mkdir(parents=True)
(library_root / ".clang-format").write_text("BasedOnStyle: LLVM\n")

malformed = svc_dir / "Component.cpp"
shutil.copy(DATA_DIR / "malformed.cpp", malformed)

monkeypatch.chdir(library_root)
parsed = argparse.Namespace(
root=None,
path=Path.cwd(),
quiet=True,
verbose=False,
backup=False,
force=False,
check=False,
allow_extension=[],
stdin=False,
files=[],
dirs=[Path("./Svc")],
exclude=[],
pass_through=[],
)

# build is None: libraries run format without a build cache
result = run_code_format(None, parsed, {}, {}, [])
assert result == 0

well_formed_content = (DATA_DIR / "well-formed.cpp").read_text()
assert malformed.read_text() == well_formed_content