Skip to content

Commit afaf91e

Browse files
haasonsaasclaude
andcommitted
TDD: fix hotspot line_range off-by-one and return type regex matching wrong arrow
- multi_pass.rs: max_line used new_start + new_lines (1 past end), now uses saturating_sub(1) - code_summary.rs: extract_return_type matched first -> (inside fn pointer params), now uses greedy .* to match the last -> (the actual return type arrow) 332 tests, 0 warnings, 0 clippy issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b834685 commit afaf91e

3 files changed

Lines changed: 56 additions & 3 deletions

File tree

src/core/code_summary.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,13 @@ fn extract_parameters(code: &str, language: &str) -> Vec<String> {
259259
}
260260

261261
static RUST_RETURN: Lazy<Regex> =
262-
Lazy::new(|| Regex::new(r"->\s*([^\{]+)").unwrap());
262+
Lazy::new(|| Regex::new(r".*->\s*([^\{]+)").unwrap());
263263

264264
fn extract_return_type(code: &str, language: &str) -> Option<String> {
265265
match language {
266266
"rs" => {
267+
// Greedy .* consumes everything up to the LAST "->", which is
268+
// the return type arrow (skips arrows inside fn pointer params)
267269
let first_lines: String = code.lines().take(3).collect::<Vec<_>>().join(" ");
268270
RUST_RETURN
269271
.captures(&first_lines)
@@ -743,6 +745,21 @@ mod tests {
743745
);
744746
}
745747

748+
// BUG: extract_return_type matches first "->" in the line, which hits the
749+
// arrow inside fn pointer params instead of the actual return type arrow
750+
#[test]
751+
fn test_extract_return_type_with_fn_pointer_param() {
752+
let code = "fn process(cb: fn(u32) -> bool) -> Result<()> {";
753+
let ret = extract_return_type(code, "rs");
754+
assert!(ret.is_some());
755+
let ret_str = ret.unwrap();
756+
// Should be "Result<()>", not "bool) -> Result<()>"
757+
assert!(
758+
ret_str.starts_with("Result"),
759+
"Return type should be 'Result<()>', got: '{ret_str}'"
760+
);
761+
}
762+
746763
// BUG: extract_return_type with multi-line complex types
747764
#[test]
748765
fn test_extract_return_type_complex() {

src/core/function_chunker.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ fn next_func() {
815815
);
816816
}
817817

818-
// BUG: find_enclosing_function panics on underflow if end_line < start_line
818+
// find_enclosing_function: verify no underflow if end_line < start_line
819819
#[test]
820820
fn test_find_enclosing_function_bad_boundary() {
821821
let boundaries = vec![

src/core/multi_pass.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ fn analyze_file_risk(diff: &UnifiedDiff) -> HotspotResult {
288288
let max_line = diff
289289
.hunks
290290
.iter()
291-
.map(|h| h.new_start + h.new_lines)
291+
.map(|h| h.new_start + h.new_lines.saturating_sub(1))
292292
.max()
293293
.unwrap_or(1);
294294

@@ -701,6 +701,42 @@ mod tests {
701701
assert!(hotspots[0].risk_score <= 1.0);
702702
}
703703

704+
// BUG: max_line uses new_start + new_lines instead of new_start + new_lines - 1
705+
// A hunk at line 10 with 3 lines (10,11,12) should give max_line=12, not 13
706+
#[test]
707+
fn test_hotspot_line_range_end() {
708+
let review = MultiPassReview::with_defaults();
709+
// Use "auth" path to trigger risk_score > 0.0 so the hotspot isn't filtered
710+
let diffs = vec![UnifiedDiff {
711+
file_path: PathBuf::from("src/auth.rs"),
712+
old_content: None,
713+
new_content: None,
714+
hunks: vec![crate::core::diff_parser::DiffHunk {
715+
old_start: 10,
716+
old_lines: 3,
717+
new_start: 10,
718+
new_lines: 3,
719+
context: String::new(),
720+
changes: vec![
721+
make_added_line(10, "line 10"),
722+
make_added_line(11, "line 11"),
723+
make_added_line(12, "line 12"),
724+
],
725+
}],
726+
is_binary: false,
727+
is_deleted: false,
728+
is_new: false,
729+
}];
730+
let hotspots = review.detect_hotspots(&diffs);
731+
assert_eq!(hotspots.len(), 1);
732+
// Lines are 10,11,12 → max should be 12, not 13
733+
assert_eq!(
734+
hotspots[0].line_range.1, 12,
735+
"max_line should be 12 (new_start + new_lines - 1), got {}",
736+
hotspots[0].line_range.1
737+
);
738+
}
739+
704740
#[test]
705741
fn test_multipass_config_default() {
706742
let config = MultiPassConfig::default();

0 commit comments

Comments
 (0)