From a2eaf7ce261a133f305467656a51a67dc8b0e42b Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Wed, 8 Oct 2025 14:23:07 -0400 Subject: [PATCH] [ty_test] Calculate hover column at parse time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the column calculation from generate_hover_outputs into HoverAssertion::from_str() by passing LineIndex and SourceText to the parse() method. This simplifies the code and centralizes the column calculation logic in one place during parsing, rather than spreading it across multiple locations. The HoverAssertion now directly stores the final column position in the line, ready to use. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/ty_test/src/assertion.rs | 27 ++++++++++++++++++--------- crates/ty_test/src/hover.rs | 13 +++---------- crates/ty_test/src/matcher.rs | 2 +- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/crates/ty_test/src/assertion.rs b/crates/ty_test/src/assertion.rs index a82a682d0b..6c3dd648b0 100644 --- a/crates/ty_test/src/assertion.rs +++ b/crates/ty_test/src/assertion.rs @@ -272,7 +272,11 @@ impl<'a> UnparsedAssertion<'a> { } /// Parse the attempted assertion into a [`ParsedAssertion`] structured representation. - pub(crate) fn parse(&self) -> Result, PragmaParseError<'a>> { + pub(crate) fn parse( + &self, + line_index: &ruff_source_file::LineIndex, + source: &ruff_db::source::SourceText, + ) -> Result, PragmaParseError<'a>> { match self { Self::Revealed(revealed) => { if revealed.is_empty() { @@ -285,7 +289,7 @@ impl<'a> UnparsedAssertion<'a> { .map(ParsedAssertion::Error) .map_err(PragmaParseError::ErrorAssertionParseError), Self::Hover(expected_type, full_comment, range) => { - HoverAssertion::from_str(expected_type, full_comment, *range) + HoverAssertion::from_str(expected_type, full_comment, *range, line_index, source) .map(ParsedAssertion::Hover) .map_err(PragmaParseError::HoverAssertionParseError) } @@ -364,12 +368,9 @@ impl std::fmt::Display for ErrorAssertion<'_> { /// A parsed and validated `# hover:` assertion comment. #[derive(Debug)] pub(crate) struct HoverAssertion<'a> { - /// The zero-based column offset within the comment where the down arrow appears. + /// The zero-based column in the line where the down arrow appears. /// This indicates the character position in the target line where we should hover. - pub(crate) arrow_offset_in_comment: usize, - - /// The range of the comment in the source file. - pub(crate) comment_range: TextRange, + pub(crate) column: usize, /// The expected type at the hover position. pub(crate) expected_type: &'a str, @@ -380,6 +381,8 @@ impl<'a> HoverAssertion<'a> { expected_type: &'a str, full_comment: &'a str, comment_range: TextRange, + line_index: &ruff_source_file::LineIndex, + source: &ruff_db::source::SourceText, ) -> Result { if expected_type.is_empty() { return Err(HoverAssertionParseError::EmptyType); @@ -390,9 +393,15 @@ impl<'a> HoverAssertion<'a> { .find('↓') .ok_or(HoverAssertionParseError::MissingDownArrow)?; + // Calculate the column within the comment's line + // First, get the line and column of the comment's start + let comment_line_col = line_index.line_column(comment_range.start(), source); + + // The hover column is the comment's column plus the arrow offset within the comment + let column = comment_line_col.column.to_zero_indexed() + arrow_offset_in_comment; + Ok(Self { - arrow_offset_in_comment, - comment_range, + column, expected_type, }) } diff --git a/crates/ty_test/src/hover.rs b/crates/ty_test/src/hover.rs index 95adb1510b..ad1a074fac 100644 --- a/crates/ty_test/src/hover.rs +++ b/crates/ty_test/src/hover.rs @@ -101,23 +101,16 @@ pub(crate) fn generate_hover_outputs( }; // Parse the assertion to get the hover information - let Ok(ParsedAssertion::Hover(hover)) = assertion.parse() else { + let Ok(ParsedAssertion::Hover(hover)) = assertion.parse(&lines, &source) else { // Invalid hover assertion - will be caught as error by matcher continue; }; - // Calculate the column within the comment's line - // First, get the line and column of the comment's start - let comment_line_col = lines.line_column(hover.comment_range.start(), &source); - - // The hover column is the comment's column plus the arrow offset within the comment - let hover_column_in_line = comment_line_col.column.to_zero_indexed() + hover.arrow_offset_in_comment; - // Get the start offset of the target line let target_line_start = lines.line_start(target_line, &source); - // Calculate the hover position: start of target line + column in line - let hover_offset = target_line_start + TextSize::try_from(hover_column_in_line).unwrap(); + // Calculate the hover position: start of target line + column + let hover_offset = target_line_start + TextSize::try_from(hover.column).unwrap(); // Get the inferred type at that position let Some(inferred_type) = infer_type_at_position(db, file, hover_offset) else { diff --git a/crates/ty_test/src/matcher.rs b/crates/ty_test/src/matcher.rs index 0e1cee9821..cf987d1311 100644 --- a/crates/ty_test/src/matcher.rs +++ b/crates/ty_test/src/matcher.rs @@ -276,7 +276,7 @@ impl Matcher { let mut failures = vec![]; let mut unmatched: Vec<&CheckOutput> = outputs.outputs.iter().collect(); for assertion in assertions { - match assertion.parse() { + match assertion.parse(&self.line_index, &self.source) { Ok(assertion) => { if !self.matches(&assertion, &mut unmatched) { failures.push(assertion.unmatched());