Skip to content

Commit 4c67db5

Browse files
haasonsaasclaude
andcommitted
fix: 2 TDD bugs — comment braces in function detection, duplicate LSP symbols
- function_chunker.rs: Skip braces inside line comments (// and #) when tracking brace depth. Previously `// }` in a comment would prematurely terminate function boundary detection. - symbol_index.rs: Use else-if between DocumentSymbol and SymbolInformation format detection to prevent pushing the same symbol twice when both selectionRange and location fields exist. 983 tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 026b45e commit 4c67db5

File tree

2 files changed

+110
-5
lines changed

2 files changed

+110
-5
lines changed

src/core/function_chunker.rs

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,30 @@ fn find_function_end(lines: &[&str], start: usize, language: &str) -> usize {
251251
lines.len().saturating_sub(1)
252252
}
253253
_ => {
254-
// Brace-based languages — skip braces inside string literals
254+
// Brace-based languages — skip braces inside string literals and comments
255255
let mut depth = 0i32;
256256
let mut found_open = false;
257257
for (i, line) in lines.iter().enumerate().skip(start) {
258258
let mut in_double_quote = false;
259259
let mut in_single_quote = false;
260260
let mut escaped = false;
261+
let mut prev_ch = '\0';
261262
for ch in line.chars() {
262263
if escaped {
263264
escaped = false;
264-
} else if (in_double_quote || in_single_quote) && ch == '\\' {
265+
prev_ch = ch;
266+
continue;
267+
}
268+
// Skip line comments: // in most languages, # in Python/Ruby/Shell
269+
if !in_double_quote && !in_single_quote {
270+
if ch == '/' && prev_ch == '/' {
271+
break; // rest of line is a comment
272+
}
273+
if ch == '#' && !in_double_quote && !in_single_quote {
274+
break; // rest of line is a comment
275+
}
276+
}
277+
if (in_double_quote || in_single_quote) && ch == '\\' {
265278
escaped = true;
266279
} else if ch == '"' && !in_single_quote {
267280
in_double_quote = !in_double_quote;
@@ -275,6 +288,7 @@ fn find_function_end(lines: &[&str], start: usize, language: &str) -> usize {
275288
depth -= 1;
276289
}
277290
}
291+
prev_ch = ch;
278292
}
279293
if found_open && depth <= 0 {
280294
return i;
@@ -967,4 +981,41 @@ fn next_func() {
967981
"Function should end at line 0 (braces balanced on same line with escaped backslash)"
968982
);
969983
}
984+
985+
// ── Bug: closing brace in line comment breaks function detection ──
986+
//
987+
// find_function_end tracks braces inside string literals but ignores
988+
// braces in line comments (// and #). A comment like `// }` causes
989+
// premature function termination.
990+
991+
#[test]
992+
fn test_find_function_end_brace_in_line_comment() {
993+
let lines: Vec<&str> = vec![
994+
"fn foo() {", // line 0: open brace
995+
" let x = 1; // }", // line 1: closing brace in comment
996+
" println!(\"ok\");", // line 2
997+
"}", // line 3: actual close
998+
];
999+
let end = find_function_end(&lines, 0, "rs");
1000+
assert_eq!(
1001+
end, 3,
1002+
"Brace in line comment should be ignored; function ends at line 3"
1003+
);
1004+
}
1005+
1006+
#[test]
1007+
fn test_find_function_end_hash_comment() {
1008+
let lines: Vec<&str> = vec![
1009+
"function foo() {", // line 0: opening brace
1010+
" x = 1 # }", // line 1: brace in hash comment
1011+
" y = 2", // line 2
1012+
"}", // line 3: actual close
1013+
];
1014+
// Use a brace-based language to test hash comment detection
1015+
let end = find_function_end(&lines, 0, "rb");
1016+
assert_eq!(
1017+
end, 3,
1018+
"Brace in # comment should be ignored; function ends at line 3"
1019+
);
1020+
}
9701021
}

src/core/symbol_index.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,7 @@ fn extract_lsp_symbols(result: &Value) -> Vec<LspSymbol> {
11401140

11411141
fn collect_lsp_symbol(value: &Value, symbols: &mut Vec<LspSymbol>) {
11421142
if let Some(obj) = value.as_object() {
1143+
// DocumentSymbol format: selectionRange or range on the object itself
11431144
if let (Some(name), Some(range)) = (
11441145
obj.get("name").and_then(|v| v.as_str()),
11451146
extract_range(obj.get("selectionRange").or_else(|| obj.get("range"))),
@@ -1148,9 +1149,8 @@ fn collect_lsp_symbol(value: &Value, symbols: &mut Vec<LspSymbol>) {
11481149
name: name.to_string(),
11491150
range,
11501151
});
1151-
}
1152-
1153-
if let Some(location) = obj.get("location") {
1152+
} else if let Some(location) = obj.get("location") {
1153+
// SymbolInformation format: range inside a location object
11541154
if let (Some(name), Some(range)) = (
11551155
obj.get("name").and_then(|v| v.as_str()),
11561156
extract_range(location.get("range")),
@@ -1272,4 +1272,58 @@ mod tests {
12721272
assert!(candidates.contains(&PathBuf::from("src/lib.rs")));
12731273
assert!(candidates.contains(&PathBuf::from("src/lib.py")));
12741274
}
1275+
1276+
// ── Bug: collect_lsp_symbol pushes duplicate when both formats present ──
1277+
//
1278+
// LSP responses can include both selectionRange (DocumentSymbol) and
1279+
// location (SymbolInformation) fields. The old code had two separate
1280+
// `if` blocks, causing the same symbol to be pushed twice.
1281+
1282+
#[test]
1283+
fn test_collect_lsp_symbol_no_duplicate() {
1284+
use serde_json::json;
1285+
let value = json!({
1286+
"name": "MyFunc",
1287+
"selectionRange": {
1288+
"start": { "line": 5, "character": 0 },
1289+
"end": { "line": 5, "character": 10 }
1290+
},
1291+
"location": {
1292+
"uri": "file:///test.rs",
1293+
"range": {
1294+
"start": { "line": 5, "character": 0 },
1295+
"end": { "line": 20, "character": 0 }
1296+
}
1297+
}
1298+
});
1299+
let mut symbols = Vec::new();
1300+
collect_lsp_symbol(&value, &mut symbols);
1301+
assert_eq!(
1302+
symbols.len(),
1303+
1,
1304+
"Symbol with both selectionRange and location should only be pushed once"
1305+
);
1306+
assert_eq!(symbols[0].name, "MyFunc");
1307+
}
1308+
1309+
#[test]
1310+
fn test_collect_lsp_symbol_location_fallback() {
1311+
use serde_json::json;
1312+
// SymbolInformation format: no selectionRange or range, only location
1313+
let value = json!({
1314+
"name": "MyVar",
1315+
"location": {
1316+
"uri": "file:///test.rs",
1317+
"range": {
1318+
"start": { "line": 10, "character": 0 },
1319+
"end": { "line": 10, "character": 5 }
1320+
}
1321+
}
1322+
});
1323+
let mut symbols = Vec::new();
1324+
collect_lsp_symbol(&value, &mut symbols);
1325+
assert_eq!(symbols.len(), 1);
1326+
assert_eq!(symbols[0].name, "MyVar");
1327+
assert_eq!(symbols[0].range, (11, 11)); // 0-based to 1-based
1328+
}
12751329
}

0 commit comments

Comments
 (0)