Fix silent statement/data loss in root render/parse path

src/formatter/mod.rs

@@ -36,6 +36,9 @@ pub(crate) struct StyleConfig {
     pub strip_inner_join: bool,
     /// Wrap CASE expressions when ELSE is present (AWeber, mattmc3).
     pub wrap_case_else: bool,
+    /// Use the dedicated pg_dump/ruleutils renderer instead of the standard
+    /// statement formatter.
+    pub pg_dump: bool,
 }
 
 impl StyleConfig {
@@ -54,6 +57,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: false,
             },
             Style::Mozilla => Self {
                 upper_keywords: true,
@@ -68,6 +72,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: false,
             },
             Style::Aweber => Self {
                 upper_keywords: true,
@@ -82,6 +87,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: true,
+                pg_dump: false,
             },
             Style::Dbt => Self {
                 upper_keywords: false,
@@ -96,6 +102,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: false,
             },
             Style::Gitlab => Self {
                 upper_keywords: true,
@@ -110,6 +117,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: true,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: false,
             },
             Style::Kickstarter => Self {
                 upper_keywords: true,
@@ -124,6 +132,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: false,
             },
             Style::Mattmc3 => Self {
                 upper_keywords: false,
@@ -138,6 +147,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: true,
                 wrap_case_else: true,
+                pg_dump: false,
             },
             // PgDump uses a dedicated renderer (see formatter::pgdump); these
             // values are placeholders and are not consulted on that path.
@@ -154,6 +164,7 @@ impl StyleConfig {
                 blank_lines_in_ctes: false,
                 strip_inner_join: false,
                 wrap_case_else: false,
+                pg_dump: true,
             },
         }
     }
@@ -180,14 +191,24 @@ impl<'a> Formatter<'a> {
         let mut results = Vec::new();
         let mut cursor = root.walk();
         for child in root.named_children(&mut cursor) {
-            if child.kind() == "toplevel_stmt"
-                && let Some(stmt) = child.find_child("stmt")
-            {
-                if self.style == Style::PgDump {
+            if child.kind() == "toplevel_stmt" {
+                // Most statements are wrapped in a `stmt` node, but some parse
+                // as a direct child of `toplevel_stmt` (e.g. the legacy
+                // transaction statement `BEGIN`, which is a
+                // `TransactionStmtLegacy`). Format the inner statement in
+                // either case so it is not silently dropped.
+                let stmt = child.find_child("stmt").unwrap_or(child);
+                if self.config.pg_dump {
                     results.push(self.format_pgdump_stmt(stmt)?);
                 } else {
                     results.push(self.format_stmt(stmt)?);
                 }
+            } else if child.kind() == "comment" {
+                // Standalone comments between/around top-level statements are
+                // direct children of `source_file`; preserve them verbatim in
+                // source order so they are not silently discarded. (Comments
+                // embedded within a statement's clauses are not yet preserved.)
+                results.push(self.text(child).trim_end().to_string());
             }
         }
         if results.is_empty() {

src/lib.rs

@@ -179,11 +179,27 @@ fn has_structural_error(root: &tree_sitter::Node) -> bool {
     let has_valid_stmt = root
         .named_children(&mut cursor)
         .any(|c| c.kind() == "toplevel_stmt");
-    if has_valid_stmt {
-        return false;
+    if !has_valid_stmt {
+        // No valid statements at all — this is genuinely broken input.
+        return true;
     }
-    // No valid statements at all — this is genuinely broken input.
-    true
+    // At least one statement parsed, but a sibling statement may still be
+    // broken (tree-sitter emits an ERROR/MISSING node rather than a
+    // toplevel_stmt for it). Small ERROR leaf nodes (< 5 bytes, e.g. decimal
+    // fractions) are grammar limitations for otherwise-valid SQL and are
+    // tolerated; a MISSING node or a substantial ERROR node signals a
+    // genuinely unparseable statement that must not be silently dropped.
+    has_significant_error(root)
+}
+
+/// Recursively check for a MISSING node or a non-trivial ERROR node.
+fn has_significant_error(node: &tree_sitter::Node) -> bool {
+    if node.is_missing() || (node.is_error() && node.byte_range().len() >= 5) {
+        return true;
+    }
+    let mut cursor = node.walk();
+    node.children(&mut cursor)
+        .any(|child| has_significant_error(&child))
 }
 
 fn find_error_message(node: &tree_sitter::Node, source: &str) -> String {

tests/fixtures/river/begin.expected

@@ -0,0 +1 @@
+BEGIN;

tests/fixtures/river/begin.sql

@@ -0,0 +1 @@
+BEGIN;

tests/fixtures/river/begin_insert_commit.expected

@@ -0,0 +1,6 @@
+BEGIN;
+
+INSERT INTO t
+     VALUES (1);
+
+COMMIT;

tests/fixtures/river/begin_insert_commit.sql

@@ -0,0 +1 @@
+BEGIN; INSERT INTO t VALUES (1); COMMIT;

tests/fixtures/river/comment_between_statements.expected

@@ -0,0 +1,5 @@
+SELECT 1;
+
+-- note
+
+SELECT 2;

tests/fixtures/river/comment_between_statements.sql

@@ -0,0 +1,3 @@
+SELECT 1;
+-- note
+SELECT 2;

tests/smoke_test.rs

@@ -1,3 +1,4 @@
+use libpgfmt::error::FormatError;
 use libpgfmt::{format, style::Style};
 
 #[test]
@@ -110,6 +111,18 @@ fn typed_literal_constants_river() {
     }
 }
 
+// Regression for https://github.com/gmr/libpgfmt/issues/16: a broken
+// statement in multi-statement input must surface as a syntax error rather
+// than being silently dropped while the parseable statements are returned.
+#[test]
+fn broken_statement_errors_instead_of_dropping() {
+    let result = format("SELECT 1; THIS IS NOT SQL @@@;", Style::River);
+    assert!(
+        matches!(result, Err(FormatError::Syntax(_))),
+        "expected Err(FormatError::Syntax(..)), got {result:?}"
+    );
+}
+
 #[test]
 fn cast_multiword_type_names() {
     // Regression: multi-word type names in :: casts must not be truncated