diff --git a/Cargo.toml b/Cargo.toml index 2863eb1..d39617a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "diffscope" -version = "0.5.26" +version = "0.5.27" edition = "2021" rust-version = "1.88" authors = ["Jonathan Haas "] diff --git a/src/config.rs b/src/config.rs index f49c792..3380c74 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2184,6 +2184,25 @@ triage_skip_deletion_only: true assert!(config.triage_skip_deletion_only); } + #[test] + fn test_config_default_triage_skip_deletion_only_false() { + let config = Config::default(); + assert!( + !config.triage_skip_deletion_only, + "default: deletions get review unless explicitly enabled" + ); + } + + #[test] + fn test_config_deserialize_triage_skip_deletion_only_false() { + let yaml = r#" +model: claude-sonnet-4-6 +triage_skip_deletion_only: false +"#; + let config: Config = serde_yaml::from_str(yaml).unwrap(); + assert!(!config.triage_skip_deletion_only); + } + #[test] fn test_default_frontier_role_models_match_requested_pair() { let config = Config::default(); diff --git a/src/parsing/llm_response.rs b/src/parsing/llm_response.rs index ac07e57..48901a8 100644 --- a/src/parsing/llm_response.rs +++ b/src/parsing/llm_response.rs @@ -436,19 +436,23 @@ fn convert_single_quoted_json_to_double(s: &str) -> String { } if c == '\'' { // Start of single-quoted string: emit " and copy until unescaped ', escaping " and \. + // Inside single-quoted: \' → one quote in JSON (emit \"); \\ → emit \\; \" → emit \" out.push('"'); + let mut single_escape = false; for c in chars.by_ref() { - if c == '\\' { - escape_next = true; - out.push(c); - } else if c == '\'' { - if escape_next { - escape_next = false; - out.push('\''); + if single_escape { + single_escape = false; + if c == '\'' { + out.push('\''); // apostrophe in JSON string needs no escape } else { - out.push('"'); - break; + out.push('\\'); + out.push(c); } + } else if c == '\\' { + single_escape = true; + } else if c == '\'' { + out.push('"'); + break; } else if c == '"' { out.push('\\'); out.push('"'); @@ -456,8 +460,8 @@ fn convert_single_quoted_json_to_double(s: &str) -> String { out.push(c); } } - if escape_next { - escape_next = false; + if single_escape { + out.push('\\'); } continue; } @@ -1349,6 +1353,43 @@ let data = &input; assert!(comments[0].content.contains("deprecated")); } + #[test] + fn parse_json_single_quoted_object_wrapped_in_findings() { + // Outer object with "findings" key; inner array uses single quotes — raw bracket span + repair. + let input = r#"{"findings": [{'line': 2, 'issue': 'Minor bug'}]}"#; + let file_path = PathBuf::from("src/lib.rs"); + let comments = parse_llm_response(input, &file_path).unwrap(); + assert_eq!(comments.len(), 1); + assert_eq!(comments[0].line_number, 2); + assert!(comments[0].content.contains("Minor bug")); + } + + #[test] + fn parse_json_single_quoted_value_with_escaped_apostrophe() { + // Single-quoted value containing escaped apostrophe (e.g. "don't") — converter preserves it. + let input = r#"[{'line': 1, 'issue': 'don\'t forget'}]"#; + let file_path = PathBuf::from("src/lib.rs"); + let comments = parse_llm_response(input, &file_path).unwrap(); + assert_eq!(comments.len(), 1); + assert_eq!(comments[0].line_number, 1); + assert!( + comments[0].content.contains("don't"), + "content should contain apostrophe: {:?}", + comments[0].content + ); + } + + #[test] + fn parse_json_double_quoted_value_with_apostrophe_unchanged() { + // Valid JSON with apostrophe in double-quoted string — no repair; parses as-is. + let input = r#"[{"line": 3, "issue": "don't use deprecated API"}]"#; + let file_path = PathBuf::from("src/lib.rs"); + let comments = parse_llm_response(input, &file_path).unwrap(); + assert_eq!(comments.len(), 1); + assert_eq!(comments[0].line_number, 3); + assert!(comments[0].content.contains("don't")); + } + // ── Bug: find_json_array uses mismatched brackets ────────────────── // // `find_json_array` uses `find('[')` (first) + `rfind(']')` (last). diff --git a/src/review/pipeline/guidance.rs b/src/review/pipeline/guidance.rs index 98faff1..4dfdf66 100644 --- a/src/review/pipeline/guidance.rs +++ b/src/review/pipeline/guidance.rs @@ -115,4 +115,53 @@ mod tests { assert!(guidance.contains("Always use parameterized queries")); assert!(guidance.contains("No direct SQL string concatenation")); } + + #[test] + fn build_review_guidance_no_prose_section_when_rules_none() { + let config = config::Config { + review_rules_prose: None, + ..config::Config::default() + }; + let guidance = build_review_guidance(&config, None).unwrap(); + assert!( + !guidance.contains("Custom rules (natural language)"), + "guidance should not include prose section when review_rules_prose is None" + ); + } + + #[test] + fn build_review_guidance_no_prose_section_when_rules_empty() { + let config = config::Config { + review_rules_prose: Some(vec![]), + ..config::Config::default() + }; + let guidance = build_review_guidance(&config, None).unwrap(); + assert!( + !guidance.contains("Custom rules (natural language)"), + "guidance should not include prose section when review_rules_prose is empty" + ); + } + + #[test] + fn build_review_guidance_single_prose_rule() { + let config = config::Config { + review_rules_prose: Some(vec!["Single rule.".to_string()]), + ..config::Config::default() + }; + let guidance = build_review_guidance(&config, None).unwrap(); + assert!(guidance.contains("Custom rules (natural language)")); + assert!(guidance.contains("Single rule.")); + } + + #[test] + fn build_review_guidance_prose_rule_with_special_chars() { + let config = config::Config { + review_rules_prose: Some(vec!["Check for