cfengine format: Don't reformat policy files with syntax errors

src/cfengine_cli/commands.py

@@ -4,7 +4,7 @@
 import json
 from cfengine_cli.profile import profile_cfengine, generate_callstack
 from cfengine_cli.dev import dispatch_dev_subcommand
-from cfengine_cli.lint import lint_args
+from cfengine_cli.lint import lint_args, PolicySyntaxError
 from cfengine_cli.shell import user_command
 from cfengine_cli.paths import bin
 from cfengine_cli.version import cfengine_cli_version_string
@@ -51,6 +51,9 @@ def deploy() -> int:
 
 
 def _format_filename(filename: str, line_length: int, check: bool) -> int:
+    """Format a single file.
+
+    Raises PolicySyntaxError for .cf files with syntax errors."""
     if filename.startswith("./."):
         return 0
     if filename.endswith(".json"):
@@ -70,6 +73,14 @@ def _format_dirname(directory: str, line_length: int, check: bool) -> int:
 
 
 def format(names, line_length, check) -> int:
+    try:
+        return _format_inner(names, line_length, check)
+    except PolicySyntaxError as e:
+        print(f"Error: {e}")
+        return 1
+
+
+def _format_inner(names, line_length, check) -> int:
     if not names:
         return _format_dirname(".", line_length, check)
     if len(names) == 1 and names[0] == "-":

src/cfengine_cli/format.py

@@ -7,6 +7,7 @@
 import tree_sitter_cfengine as tscfengine
 from tree_sitter import Language, Parser, Node
 from cfbs.pretty import pretty_file, pretty_check_file
+from cfengine_cli.lint import check_policy_syntax
 
 # Node types that increase indentation by 2 when entered
 INDENTED_TYPES = {
@@ -778,19 +779,23 @@ def format_policy_file(filename: str, line_length: int, check: bool) -> int:
     """Format a .cf policy file in place, writing only if content changed.
 
     Returns 0 in case of successful reformat or no reformat needed.
-    Returns 1 when check is True and reformat is needed."""
+    Returns 1 when check is True and reformat is needed.
+    Raises PolicySyntaxError when the file has syntax errors."""
     assert filename.endswith(".cf")
 
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
 
-    fmt = Formatter()
     with open(filename, "rb") as f:
         original_data = f.read()
     tree = parser.parse(original_data)
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
+
+    check_policy_syntax(tree, filename)
+
+    fmt = Formatter()
     autoformat(root_node, fmt, line_length)
 
     new_data = fmt.buffer + "\n"
@@ -808,16 +813,21 @@ def format_policy_file(filename: str, line_length: int, check: bool) -> int:
 def format_policy_fin_fout(
     fin: IO[str], fout: IO[str], line_length: int, check: bool
 ) -> int:
-    """Format CFEngine policy read from fin, writing the result to fout."""
+    """Format CFEngine policy read from fin, writing the result to fout.
+
+    Raises PolicySyntaxError when the input has syntax errors."""
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
 
-    fmt = Formatter()
     original_data = fin.read().encode("utf-8")
     tree = parser.parse(original_data)
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
+
+    check_policy_syntax(tree, "<stdin>")
+
+    fmt = Formatter()
     autoformat(root_node, fmt, line_length)
 
     new_data = fmt.buffer + "\n"

src/cfengine_cli/lint.py

@@ -1035,6 +1035,48 @@ def _lint_json_selector(file: str) -> int:
     return _lint_json_plain(file)
 
 
+# ---------------------------------------------------------------------------
+# Syntax error detection (used by both linter and formatter)
+# ---------------------------------------------------------------------------
+
+
+def _find_first_error(node: Node) -> Node | None:
+    """Find the first ERROR node in the tree, or None if the tree is valid."""
+    if node.type == "ERROR":
+        return node
+    for child in node.children:
+        found = _find_first_error(child)
+        if found:
+            return found
+    return None
+
+
+class PolicySyntaxError(Exception):
+    """Raised when a policy file has syntax errors and cannot be formatted."""
+
+    def __init__(self, filename: str, line: int, column: int):
+        self.filename = filename
+        self.line = line
+        self.column = column
+        super().__init__(f"Syntax error in '{filename}' at {filename}:{line}:{column}")
+
+
+def check_policy_syntax(tree: Tree, filename: str) -> None:
+    """Check a parsed tree for syntax errors.
+
+    Raises PolicySyntaxError if an ERROR node is found.
+
+    Only checks for ERROR nodes, not MISSING nodes — missing tokens like
+    semicolons are handled gracefully by the formatter."""
+    root_node = tree.root_node
+    error_node = _find_first_error(root_node)
+    if not error_node:
+        return
+    line = error_node.range.start_point[0] + 1
+    column = error_node.range.start_point[1] + 1
+    raise PolicySyntaxError(filename, line, column)
+
+
 # Interface: These are the functions we want to be called from outside
 # They create State() and should not be called recursively inside lint.py
 

tests/shell/004-format-check.sh

@@ -39,3 +39,29 @@ if cfengine format --check "$tmpdir/bad2.cf"; then
 fi
 # Verify the file was NOT modified
 diff "$tmpdir/bad2_orig.cf" "$tmpdir/bad2.cf"
+
+# Case 5: format a file with syntax errors -> exit 1, file NOT modified
+printf 'bundle agent { invalid syntax\n' > "$tmpdir/syntax_error.cf"
+cp "$tmpdir/syntax_error.cf" "$tmpdir/syntax_error_orig.cf"
+if cfengine format "$tmpdir/syntax_error.cf"; then
+	echo "FAIL: expected exit code 1 for file with syntax errors"
+	exit 1
+fi
+# Verify the file was NOT modified
+diff "$tmpdir/syntax_error_orig.cf" "$tmpdir/syntax_error.cf"
+
+# Case 6: format from stdin with syntax errors -> exit 1
+if printf 'bundle agent { invalid syntax\n' | cfengine format -; then
+	echo "FAIL: expected exit code 1 for stdin with syntax errors"
+	exit 1
+fi
+
+# Case 7: --check on file with syntax errors -> exit 1, file NOT modified
+printf 'bundle agent { invalid syntax\n' > "$tmpdir/syntax_error2.cf"
+cp "$tmpdir/syntax_error2.cf" "$tmpdir/syntax_error2_orig.cf"
+if cfengine format --check "$tmpdir/syntax_error2.cf"; then
+	echo "FAIL: expected exit code 1 for --check on file with syntax errors"
+	exit 1
+fi
+# Verify the file was NOT modified
+diff "$tmpdir/syntax_error2_orig.cf" "$tmpdir/syntax_error2.cf"