Skip to content

Commit ab3a1cb

Browse files
committed
Fix anchoring of breakpoints inside embrace operator
And more generally inside elements of braces who don't start on their own line
1 parent 26e192f commit ab3a1cb

7 files changed

+219
-28
lines changed

crates/ark/src/console_annotate.rs

Lines changed: 149 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -557,19 +557,18 @@ impl SyntaxRewriter for AnnotationRewriter<'_> {
557557
let Some(last_info) = frame.expr_info.last() else {
558558
return self.fail(anyhow!("expr_info unexpectedly empty"), node);
559559
};
560-
let Some(first_info) = frame.expr_info.first() else {
561-
return self.fail(anyhow!("expr_info unexpectedly empty"), node);
562-
};
563560

564561
let brace_doc_start = self.to_doc_line(frame.brace_code_line);
565562
let brace_doc_end = self.to_doc_line(last_info.range.end);
566-
let first_expr_doc_start = self.to_doc_line(first_info.start);
567563

568564
// Annotate statements in the braced list
569565
let result = self.annotate_braced_list(node, frame.brace_code_line, frame.expr_info);
570566

571-
// Mark any remaining breakpoints in this brace range as invalid (closing braces)
572-
let invalidation_floor = breakpoint_floor(brace_doc_start, first_expr_doc_start);
567+
// Mark any remaining breakpoints in this brace range as invalid
568+
// (closing braces). Breakpoints on the brace line itself belong to
569+
// the parent scope, so we start invalidation from brace_doc_start +
570+
// 1.
571+
let invalidation_floor = brace_doc_start + 1;
573572
self.mark_breakpoints_invalid(
574573
Some(invalidation_floor),
575574
Some(brace_doc_end),
@@ -613,12 +612,11 @@ impl AnnotationRewriter<'_> {
613612
let mut result_slots: Vec<Option<RSyntaxElement>> = Vec::new();
614613
let mut needs_line_directive = false;
615614

616-
let first_expr_doc_start = expr_info
617-
.first()
618-
.map(|info| self.to_doc_line(info.start))
619-
.unwrap_or(brace_doc_start);
620-
let mut prev_doc_end: Option<i32> =
621-
Some(breakpoint_floor(brace_doc_start, first_expr_doc_start));
615+
// Braced expressions never claim breakpoints on their opening brace
616+
// line. This prevents injecting breakpoints inside `{ foo }` or `{{ foo
617+
// }}` when they appear on a single line - those breakpoints belong to
618+
// the parent scope.
619+
let mut prev_doc_end: Option<i32> = Some(brace_doc_start + 1);
622620

623621
for (i, expr) in elements.iter().enumerate() {
624622
// Use precomputed line info captured on the preorder visit, when
@@ -734,18 +732,6 @@ impl AnnotationRewriter<'_> {
734732
}
735733
}
736734

737-
/// Compute the floor line for breakpoint matching in a braced list. When
738-
/// content starts on a later line than the brace, we use `brace_doc_start + 1`
739-
/// to avoid claiming breakpoints on the brace line, as those belong to the
740-
/// parent scope.
741-
fn breakpoint_floor(brace_doc_start: i32, first_expr_doc_start: i32) -> i32 {
742-
if first_expr_doc_start > brace_doc_start {
743-
brace_doc_start + 1
744-
} else {
745-
brace_doc_start
746-
}
747-
}
748-
749735
/// Returns the code line of the node's first token.
750736
fn first_token_code_line(node: &RSyntaxNode, line_index: &LineIndex) -> Option<u32> {
751737
let token = node.first_token()?;
@@ -1893,14 +1879,16 @@ mod tests {
18931879
},
18941880
};
18951881

1896-
// Test 1: Breakpoint on line 0 (valid - anchors to inner {} expression)
1882+
// Test 1: Breakpoint on line 0 stays Unverified - braces never claim breakpoints
1883+
// on their opening line, and at top level there's no parent to claim it either
18971884
let mut breakpoints = vec![Breakpoint::new(1, 0, BreakpointState::Unverified)];
18981885
let result = annotate_input(code, location.clone(), Some(&mut breakpoints)).unwrap();
18991886
assert!(
1900-
!matches!(breakpoints[0].state, BreakpointState::Invalid(_)),
1901-
"Breakpoint on {{ line should be valid"
1887+
matches!(breakpoints[0].state, BreakpointState::Unverified),
1888+
"Breakpoint on {{ line should remain Unverified at top level"
19021889
);
1903-
assert!(result.contains(".ark_breakpoint"));
1890+
// No breakpoint injected (left for potential parent scope)
1891+
assert!(!result.contains(".ark_breakpoint"));
19041892

19051893
// Test 2: Breakpoint on line 1 (}} line) is invalid - inner {} is empty
19061894
let mut breakpoints = vec![Breakpoint::new(2, 1, BreakpointState::Unverified)];
@@ -1914,6 +1902,139 @@ mod tests {
19141902
assert!(!result.contains(".ark_breakpoint"));
19151903
}
19161904

1905+
#[test]
1906+
fn test_inject_breakpoints_double_brace_injection_bug() {
1907+
// Test that breakpoints on a line with {{ foo }} don't inject inside the inner braces.
1908+
// The inner {{ }} is an rlang interpolation construct and shouldn't have breakpoints.
1909+
// Breakpoint on line 1 should inject BEFORE identity(), not inside {{ }}
1910+
let code = "{\n identity({{ foo }})\n}";
1911+
let location = CodeLocation {
1912+
uri: Url::parse("file:///test.R").unwrap(),
1913+
start: Position {
1914+
line: 0,
1915+
character: 0,
1916+
},
1917+
end: Position {
1918+
line: 2,
1919+
character: 1,
1920+
},
1921+
};
1922+
let mut breakpoints = vec![Breakpoint::new(1, 1, BreakpointState::Unverified)];
1923+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
1924+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
1925+
insta::assert_snapshot!(result);
1926+
}
1927+
1928+
#[test]
1929+
fn test_inject_breakpoints_call_with_braced_arg() {
1930+
// Test that breakpoints on a line with `foo({ bar })` don't inject inside the braces.
1931+
// Breakpoint on line 1 should inject BEFORE foo(), not inside { }
1932+
let code = "{\n foo({ bar })\n}";
1933+
let location = CodeLocation {
1934+
uri: Url::parse("file:///test.R").unwrap(),
1935+
start: Position {
1936+
line: 0,
1937+
character: 0,
1938+
},
1939+
end: Position {
1940+
line: 2,
1941+
character: 1,
1942+
},
1943+
};
1944+
let mut breakpoints = vec![Breakpoint::new(1, 1, BreakpointState::Unverified)];
1945+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
1946+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
1947+
insta::assert_snapshot!(result);
1948+
}
1949+
1950+
#[test]
1951+
fn test_inject_breakpoints_multiline_braced_arg_bp_on_opening_line() {
1952+
// Test breakpoint on opening line of multi-line braced arg `foo({ foo\n bar })`.
1953+
// Breakpoint on line 1 should inject BEFORE foo(), at outer level
1954+
let code = "{\n foo({ foo\n bar\n })\n}";
1955+
let location = CodeLocation {
1956+
uri: Url::parse("file:///test.R").unwrap(),
1957+
start: Position {
1958+
line: 0,
1959+
character: 0,
1960+
},
1961+
end: Position {
1962+
line: 4,
1963+
character: 1,
1964+
},
1965+
};
1966+
let mut breakpoints = vec![Breakpoint::new(1, 1, BreakpointState::Unverified)];
1967+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
1968+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
1969+
insta::assert_snapshot!(result);
1970+
}
1971+
1972+
#[test]
1973+
fn test_inject_breakpoints_multiline_braced_arg_bp_on_inner_line() {
1974+
// Test breakpoint on inner line of multi-line braced arg `foo({ foo\n bar })`.
1975+
// Breakpoint on line 2 should inject INSIDE the braces, before bar
1976+
let code = "{\n foo({ foo\n bar\n })\n}";
1977+
let location = CodeLocation {
1978+
uri: Url::parse("file:///test.R").unwrap(),
1979+
start: Position {
1980+
line: 0,
1981+
character: 0,
1982+
},
1983+
end: Position {
1984+
line: 4,
1985+
character: 1,
1986+
},
1987+
};
1988+
let mut breakpoints = vec![Breakpoint::new(1, 2, BreakpointState::Unverified)];
1989+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
1990+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
1991+
insta::assert_snapshot!(result);
1992+
}
1993+
1994+
#[test]
1995+
fn test_inject_breakpoints_braced_arg_on_separate_line() {
1996+
// Test breakpoint on a braced arg `{ foo }` that's on its own line.
1997+
// Breakpoint anchors to outer foo() call, not inside braces.
1998+
let code = "{\n foo(\n { foo }\n )\n}";
1999+
let location = CodeLocation {
2000+
uri: Url::parse("file:///test.R").unwrap(),
2001+
start: Position {
2002+
line: 0,
2003+
character: 0,
2004+
},
2005+
end: Position {
2006+
line: 4,
2007+
character: 1,
2008+
},
2009+
};
2010+
let mut breakpoints = vec![Breakpoint::new(1, 2, BreakpointState::Unverified)];
2011+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
2012+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
2013+
insta::assert_snapshot!(result);
2014+
}
2015+
2016+
#[test]
2017+
fn test_inject_breakpoints_double_braced_arg_on_separate_line() {
2018+
// Test breakpoint on a double-braced arg `{{ foo }}` that's on its own line.
2019+
// Breakpoint anchors to outer foo() call, not inside braces.
2020+
let code = "{\n foo(\n {{ foo }}\n )\n}";
2021+
let location = CodeLocation {
2022+
uri: Url::parse("file:///test.R").unwrap(),
2023+
start: Position {
2024+
line: 0,
2025+
character: 0,
2026+
},
2027+
end: Position {
2028+
line: 4,
2029+
character: 1,
2030+
},
2031+
};
2032+
let mut breakpoints = vec![Breakpoint::new(1, 2, BreakpointState::Unverified)];
2033+
let result = annotate_input(code, location, Some(&mut breakpoints)).unwrap();
2034+
assert!(!matches!(breakpoints[0].state, BreakpointState::Invalid(_)));
2035+
insta::assert_snapshot!(result);
2036+
}
2037+
19172038
#[test]
19182039
fn test_inject_breakpoints_inside_multiline_call() {
19192040
// Test breakpoint placed on a line inside a multi-line call expression.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
8+
#line 2 "file:///test.R"
9+
foo(
10+
{ foo }
11+
)
12+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
8+
#line 2 "file:///test.R"
9+
foo({ bar })
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
8+
#line 2 "file:///test.R"
9+
identity({{ foo }})
10+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
8+
#line 2 "file:///test.R"
9+
foo(
10+
{{ foo }}
11+
)
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
foo({ foo
8+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
9+
#line 3 "file:///test.R"
10+
bar
11+
})
12+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
source: crates/ark/src/console_annotate.rs
3+
expression: result
4+
---
5+
#line 1 "file:///test.R"
6+
{
7+
base::.ark_auto_step(base::.ark_breakpoint(browser(), "file:///test.R", "1"))
8+
#line 2 "file:///test.R"
9+
foo({ foo
10+
base::.ark_auto_step(base::.ark_verify_breakpoints_range("file:///test.R", 2L, 3L))
11+
#line 3 "file:///test.R"
12+
bar
13+
})
14+
}

0 commit comments

Comments
 (0)