Allow comment-heavy YAML in check-yaml (#2554)

Closes #2501

Author: j178

Cargo.lock

@@ -86,13 +86,13 @@ serde_json = { version = "1.0.132", features = [
   "unbounded_depth"
 ] }
 serde_stacker = { version = "0.1.12" }
-serde-saphyr = { version = "1.0.0", default-features = false, features = [
+serde-saphyr = { version = "1.1.0", default-features = false, features = [
   "deserialize",
   "serialize"
 ] }
 shlex = { version = "2.0.0" }
 globset = { version = "0.4.18" }
-granit-parser = { version = "1.0.1" }
+granit-parser = { version = "1.1.0" }
 similar = { version = "3.0.0" }
 ssl-certs = { version = "0.1.1" }
 strum = { version = "0.28.0", features = ["derive"] }

Cargo.toml

@@ -71,6 +71,8 @@ fn check_loaded(filename: &Path, content: &[u8], allow_multi_docs: bool) -> Hook
             // high-reuse anchors that are common in compose-style files. See #1838.
             enforce_alias_anchor_ratio: false,
         },
+        // Comments do not affect syntax or structure validation. See #2501.
+        emit_comments: false,
         // Do not require `!!binary` scalars to decode as UTF-8. See #1102.
         ignore_binary_tag_for_string: true,
         // The scalar values are discarded, so only validate whether they are
@@ -102,9 +104,15 @@ fn check_syntax(filename: &Path, content: &[u8]) -> HookOutput {
         }
     };
 
+    let options = granit_parser::options! {
+        // Comments do not affect syntax validation. See #2501.
+        emit_comments: false,
+    };
+
     // TODO: `granit-parser` resolves aliases while producing parser events, so this is stricter
     // than upstream's syntax-only mode for aliases that reference an undefined anchor.
-    for event in granit_parser::Parser::new_from_str(content) {
+    for event in granit_parser::Parser::with_options(granit_parser::StrInput::new(content), options)
+    {
         if let Err(error) = event {
             let error_message = format!("{}: Failed to yaml parse ({error})\n", filename.display());
             return HookOutput::unchanged(1, error_message.into_bytes());
@@ -346,4 +354,27 @@ response:
         assert!(result.output.is_empty());
         Ok(())
     }
+
+    #[tokio::test]
+    async fn test_yaml_with_many_consecutive_comments() -> Result<()> {
+        let dir = tempdir()?;
+        let mut content = "items:\n".to_string();
+        for index in 0..100 {
+            let _ = writeln!(content, "  # comment {index}");
+        }
+        content.push_str("  - value\n");
+
+        let file_path = create_test_file(&dir, "many-comments.yaml", content.as_bytes()).await?;
+        for mode in [LOAD_SINGLE_DOCUMENT, CheckMode::SyntaxOnly] {
+            let result = check_file(Path::new(""), &file_path, mode).await?;
+            assert_eq!(
+                result.exit_status,
+                0,
+                "{}",
+                String::from_utf8_lossy(&result.output)
+            );
+            assert!(result.output.is_empty());
+        }
+        Ok(())
+    }
 }

crates/prek/src/hooks/pre_commit_hooks/check_yaml.rs

@@ -210,19 +210,19 @@ fn yaml_to_toml_rejects_invalid_config() -> anyhow::Result<()> {
       context
         .command()
         .args(["util", "yaml-to-toml", "config.yaml"]),
-      @"
+      @r#"
     success: false
     exit_code: 2
     ----- stdout -----
 
     ----- stderr -----
     error: Failed to parse `config.yaml`
-      caused by: error: line 1 column 8: unexpected event: expected sequence start
+      caused by: error: line 1 column 8: expected sequence start
      --> <input>:1:8
       |
     1 | repos: 123
-      |        ^ unexpected event: expected sequence start
-    "
+      |        ^ expected sequence start
+    "#
     );
 
     Ok(())