WIP: diffguard-lsp: add # Errors section to run_server()

adr.md

@@ -0,0 +1,64 @@
+# ADR-034: Inline Format Arguments in diffguard CLI
+
+## Status
+Proposed
+
+## Context
+
+GitHub issue #416 reports ~20 instances of `clippy::uninlined_format_args` warnings in `crates/diffguard/src/main.rs`. These occur when a variable is passed as a standalone `{}` format argument (e.g., `format!("{}", e)`) instead of being inlined into the format string (e.g., `format!("{e}")`).
+
+The `diffguard` project enforces `cargo clippy --workspace --all-targets -- -D warnings` in CI, and a prior precedent (commit `d192d33`, issue #505) established inlining format arguments as the project standard when fixing `xtask`.
+
+The fix is purely syntactic — no logic changes, no behavioral changes — but the issue scope is limited to `crates/diffguard/src/main.rs` only (20 locations), while the same lint produces ~94 additional warnings across other crates.
+
+## Decision
+
+Fix all 20 `clippy::uninlined_format_args` warnings in `crates/diffguard/src/main.rs` by inlining format arguments using `cargo clippy --fix`, supplemented by manual fixes for any cases clippy cannot auto-fix.
+
+**Implementation approach:**
+1. Run `cargo clippy -p diffguard --fix --lib -- -W clippy::uninlined_format_args` for library target
+2. Run `cargo clippy -p diffguard --fix --tests -- -W clippy::uninlined_format_args` for test target  
+3. Manual patch for any remaining unfixable locations
+4. Verify with `cargo clippy -p diffguard --all-targets -- -W clippy::uninlined_format_args`
+
+**Scope boundaries:**
+- **In-scope**: 20 locations in `crates/diffguard/src/main.rs` (lib and test code within that file)
+- **Out-of-scope**: Warnings in `diffguard-core`, `diffguard-types`, `presets.rs`, and other files — these are separate work items
+
+**Semantic fix note**: Line 2880 (`bail!("No rules match filter '{}'", filter)`) has a latent bug — `'{}'` is a literal, so the `filter` variable was silently ignored. The fix will correct this to actually display the filter value in error messages.
+
+## Consequences
+
+### Benefits
+- **Readability**: `{e}` is more scannable than `format!("{}", e)` in CLI output code
+- **Consistency**: Aligns with the project's established precedent (commit `d192d33`)
+- **CI compliance**: Removes warnings that would break the lint gate if enforced later
+- **Correctness**: Fixes the latent semantic bug at line 2880 where `filter` was ignored
+
+### Tradeoffs
+- **Narrow scope**: The ~94 warnings in other crates remain unfixed, creating短期内 inconsistency
+- **Pre-existing test error**: `green_tests_work_d4a75f70.rs:119` has a compile error that blocks test target fixes — must be addressed separately
+- **Risk of scope creep**: Running `--fix --tests` also fixes `presets.rs` (7 warnings) — discipline required to stay within scope
+
+### Risks
+- Very low: The change is mechanical, clippy verifies correctness, and no logic changes occur
+
+## Alternatives Considered
+
+### 1. Manual-only fix
+Reject `cargo clippy --fix` and manually edit all 20 locations.
+- **Rejected because**: Too time-consuming for identical-pattern fixes; clippy auto-fix is well-tested and deterministic.
+
+### 2. Suppress lint with `#[allow()]`
+Add `#[allow(clippy::uninlined_format_args)]` to suppress warnings rather than fix them.
+- **Rejected because**: Goes against the issue intent (#416 specifically asks to fix them), leaves codebase inconsistent with established project standard.
+
+### 3. Fix entire workspace at once
+Extend scope to fix all ~114 warnings across all crates simultaneously.
+- **Rejected because**: Issue #416 is scoped specifically to main.rs; broader scope risks diffusion of responsibility and makes review harder.
+
+## References
+- GitHub Issue: #416
+- Prior precedent: commit `d192d33` (issue #505, xtask uninlined format args)
+- Lint: `clippy::uninlined_format_args`
+- File: `crates/diffguard/src/main.rs`

crates/diffguard-lsp/src/server.rs

@@ -161,6 +161,14 @@ impl ServerState {
     }
 }
 
+/// Run the LSP server main loop.
+///
+/// # Errors
+///
+/// Returns an error if:
+/// - LSP protocol initialization or message handling fails
+/// - LSP messages cannot be parsed as JSON
+/// - Sending LSP messages to the client fails
 pub fn run_server(connection: Connection) -> Result<()> {
     // Use the lower-level initialize_start/initialize_finish methods
     // to send a custom InitializeResult with server_info.

crates/diffguard-lsp/tests/run_server_doc_comment_test.rs

@@ -0,0 +1,157 @@
+//! Tests verifying that `run_server()` has proper documentation per Rust API Guidelines C409.
+//!
+//! These tests verify that the `run_server()` function in `server.rs` has:
+//! 1. A doc comment describing what the function does
+//! 2. An `# Errors` section enumerating all error categories
+
+use std::fs;
+use std::path::Path;
+
+/// Test that `run_server()` has a doc comment immediately preceding its declaration.
+#[test]
+fn test_run_server_has_doc_comment() {
+    let server_rs = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/server.rs");
+    let content = fs::read_to_string(&server_rs).expect("failed to read server.rs");
+
+    // Find the line with "pub fn run_server"
+    let lines: Vec<&str> = content.lines().collect();
+    let run_server_line = lines
+        .iter()
+        .position(|line| line.contains("pub fn run_server"))
+        .expect("pub fn run_server not found in server.rs");
+
+    // Check that there's a doc comment on the line(s) immediately before
+    // Count backwards from run_server_line to find the doc comment
+    let mut doc_line_idx = run_server_line;
+    while doc_line_idx > 0 {
+        let prev_line = lines[doc_line_idx - 1].trim();
+        if prev_line.is_empty() {
+            // Empty line before doc comment - that's the separator
+            break;
+        }
+        if prev_line.starts_with("///") || prev_line.starts_with("//!") {
+            doc_line_idx -= 1;
+        } else {
+            // No doc comment found before this non-doc, non-empty line
+            panic!(
+                "No doc comment found before `pub fn run_server` at line {}. \
+                 Found line: {:?}",
+                run_server_line + 1,
+                lines[doc_line_idx.saturating_sub(1)]
+            );
+        }
+    }
+
+    // Verify the doc comment exists (at least one line starting with ///)
+    let has_doc_comment = (0..=run_server_line).any(|i| lines[i].trim().starts_with("///"));
+    assert!(
+        has_doc_comment,
+        "run_server() at line {} must have a doc comment (/// ...)",
+        run_server_line + 1
+    );
+}
+
+/// Test that `run_server()` has an `# Errors` section in its doc comment.
+#[test]
+fn test_run_server_has_errors_section() {
+    let server_rs = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/server.rs");
+    let content = fs::read_to_string(&server_rs).expect("failed to read server.rs");
+
+    // Extract the doc comment for run_server
+    let lines: Vec<&str> = content.lines().collect();
+    let run_server_line = lines
+        .iter()
+        .position(|line| line.contains("pub fn run_server"))
+        .expect("pub fn run_server not found in server.rs");
+
+    // Collect all doc comment lines before run_server
+    let mut doc_lines = Vec::new();
+    for i in (0..run_server_line).rev() {
+        let line = lines[i].trim();
+        if line.starts_with("///") {
+            doc_lines.push(line);
+        } else if line.is_empty() {
+            // Skip empty lines in the doc comment
+            continue;
+        } else {
+            break;
+        }
+    }
+
+    // Reverse to get natural order
+    doc_lines.reverse();
+
+    // Join all doc lines to search for # Errors
+    let full_doc = doc_lines.join("\n");
+
+    assert!(
+        full_doc.contains("# Errors"),
+        "run_server() doc comment must contain '# Errors' section.\n\
+         Found doc comment:\n{}",
+        full_doc
+    );
+}
+
+/// Test that the `# Errors` section in `run_server()` documents all three error categories:
+/// 1. LSP protocol errors
+/// 2. JSON parse/serialization errors
+/// 3. LSP message send errors
+#[test]
+fn test_run_server_errors_section_has_three_categories() {
+    let server_rs = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/server.rs");
+    let content = fs::read_to_string(&server_rs).expect("failed to read server.rs");
+
+    // Extract the doc comment for run_server
+    let lines: Vec<&str> = content.lines().collect();
+    let run_server_line = lines
+        .iter()
+        .position(|line| line.contains("pub fn run_server"))
+        .expect("pub fn run_server not found in server.rs");
+
+    // Collect all doc comment lines before run_server
+    let mut doc_lines = Vec::new();
+    for i in (0..run_server_line).rev() {
+        let line = lines[i].trim();
+        if line.starts_with("///") {
+            doc_lines.push(line);
+        } else if line.is_empty() {
+            continue;
+        } else {
+            break;
+        }
+    }
+    doc_lines.reverse();
+    let full_doc = doc_lines.join("\n");
+
+    // Check for the three error categories
+    let has_lsp_protocol_errors = full_doc.to_lowercase().contains("lsp protocol")
+        || full_doc.to_lowercase().contains("initialize")
+        || full_doc.to_lowercase().contains("shutdown");
+    let has_json_errors = full_doc.to_lowercase().contains("json")
+        || full_doc.to_lowercase().contains("parse")
+        || full_doc.to_lowercase().contains("serialization")
+        || full_doc.to_lowercase().contains("serde");
+    let has_message_send_errors =
+        full_doc.to_lowercase().contains("send") || full_doc.to_lowercase().contains("message");
+
+    assert!(
+        has_lsp_protocol_errors,
+        "run_server() # Errors section must document LSP protocol errors (initialization, handling).\n\
+         Found doc comment:\n{}",
+        full_doc
+    );
+
+    assert!(
+        has_json_errors,
+        "run_server() # Errors section must document JSON parse/serialization errors.\n\
+         Found doc comment:\n{}",
+        full_doc
+    );
+
+    assert!(
+        has_message_send_errors,
+        "run_server() # Errors section must document LSP message send errors.\n\
+         Found doc comment:\n{}",
+        full_doc
+    );
+}

crates/diffguard-types/src/lib.rs

@@ -275,6 +275,16 @@ pub struct Defaults {
 
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub diff_context: Option<u32>,
+
+    /// Ignore comments when matching patterns.
+    /// When set to `true`, pattern matching skips comment lines.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub ignore_comments: Option<bool>,
+
+    /// Ignore string literals when matching patterns.
+    /// When set to `true`, pattern matching skips string literal content.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub ignore_strings: Option<bool>,
 }
 
 impl Default for Defaults {
@@ -286,6 +296,8 @@ impl Default for Defaults {
             fail_on: Some(FailOn::Error),
             max_findings: Some(200),
             diff_context: Some(0),
+            ignore_comments: None,
+            ignore_strings: None,
         }
     }
 }
@@ -517,7 +529,7 @@ pub struct CapabilityStatus {
 pub struct SensorFinding {
     /// Check identifier (constant: "diffguard.pattern").
     pub check_id: String,
-    /// Rule code (maps from rule_id, e.g., "rust.no_unwrap").
+    /// Rule code (maps from `rule_id`, e.g., "rust.no_unwrap").
     pub code: String,
     /// Finding severity.
     pub severity: Severity,
@@ -588,6 +600,8 @@ mod tests {
         assert_eq!(defaults.fail_on, Some(FailOn::Error));
         assert_eq!(defaults.max_findings, Some(200));
         assert_eq!(defaults.diff_context, Some(0));
+        assert_eq!(defaults.ignore_comments, None);
+        assert_eq!(defaults.ignore_strings, None);
     }
 
     #[test]