Skip to content

Commit fba6601

Browse files
evanyeungmeta-codesync[bot]
authored andcommitted
Fix is_sub_range flawed multi-line range comparison
Reviewed By: captbaritone Differential Revision: D95419957 fbshipit-source-id: d956b4a39eb0e1257e772f0823daea9bae5a4df2
1 parent f571ad8 commit fba6601

1 file changed

Lines changed: 31 additions & 7 deletions

File tree

compiler/crates/relay-lsp/src/diagnostic_reporter.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -330,14 +330,14 @@ impl DiagnosticReporter {
330330
}
331331

332332
/// Checks if `inner` range is within the `outer` range.
333-
/// First, we need to make sure that the start character of the outer range
334-
/// is before (or the same) character of the inner range.
335-
/// Then we need to make sure that end character of the outer range is after
336-
/// (or the same) as the end character of the inner range, or outer line
337-
/// is more than inner end line.
333+
/// A position A is before or equal to position B if A's line is less than B's,
334+
/// or they are on the same line and A's character is less than or equal to B's.
338335
fn is_sub_range(inner: Range, outer: Range) -> bool {
339-
(outer.start.character <= inner.start.character && outer.start.line <= inner.start.line)
340-
&& (outer.end.character >= inner.end.character && outer.end.line >= inner.end.line)
336+
let outer_start_before_inner_start = outer.start.line < inner.start.line
337+
|| (outer.start.line == inner.start.line && outer.start.character <= inner.start.character);
338+
let outer_end_after_inner_end = outer.end.line > inner.end.line
339+
|| (outer.end.line == inner.end.line && outer.end.character >= inner.end.character);
340+
outer_start_before_inner_start && outer_end_after_inner_end
341341
}
342342

343343
/// Publish diagnostics to the client
@@ -423,4 +423,28 @@ mod tests {
423423
let diagnostic = Range::new(Position::new(92, 6), Position::new(92, 17));
424424
assert!(!is_sub_range(cursor, diagnostic));
425425
}
426+
427+
#[test]
428+
fn sub_range_multiline_outer_contains_inner_with_smaller_char() {
429+
// inner is on a line within the outer range but at a smaller character
430+
// offset than the outer's start character — this should still be contained
431+
let inner = Range::new(Position::new(6, 2), Position::new(6, 2));
432+
let outer = Range::new(Position::new(5, 10), Position::new(8, 5));
433+
assert!(is_sub_range(inner, outer));
434+
}
435+
436+
#[test]
437+
fn sub_range_multiline_outer_contains_inner_with_larger_end_char() {
438+
// inner end is on a line within the outer range but at a larger character
439+
// offset than the outer's end character — this should still be contained
440+
let inner = Range::new(Position::new(6, 2), Position::new(7, 20));
441+
let outer = Range::new(Position::new(5, 10), Position::new(8, 5));
442+
assert!(is_sub_range(inner, outer));
443+
}
444+
445+
#[test]
446+
fn sub_range_single_line_exact_match() {
447+
let range = Range::new(Position::new(5, 3), Position::new(5, 10));
448+
assert!(is_sub_range(range, range));
449+
}
426450
}

0 commit comments

Comments
 (0)