From 75c1a0ae55093dc0ea6e996641857c75c03c229e Mon Sep 17 00:00:00 2001 From: Hugo Date: Sun, 16 Nov 2025 09:34:54 +0100 Subject: [PATCH] [ty] Provide proper error on dangling revealed (#21416) Co-authored-by: Micha Reiser --- crates/ty_test/src/lib.rs | 84 ++++++++++++++++++++++++++++++------ crates/ty_test/src/parser.rs | 52 +++++++++++++++++++--- 2 files changed, 117 insertions(+), 19 deletions(-) diff --git a/crates/ty_test/src/lib.rs b/crates/ty_test/src/lib.rs index 9460992312..04eda62a2f 100644 --- a/crates/ty_test/src/lib.rs +++ b/crates/ty_test/src/lib.rs @@ -14,7 +14,7 @@ use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf}; use ruff_db::testing::{setup_logging, setup_logging_with_filter}; use ruff_source_file::{LineIndex, OneIndexed}; use std::backtrace::BacktraceStatus; -use std::fmt::Write; +use std::fmt::{Display, Write}; use ty_python_semantic::pull_types::pull_types; use ty_python_semantic::types::{UNDEFINED_REVEAL, check_types}; use ty_python_semantic::{ @@ -86,21 +86,34 @@ pub fn run( EmbeddedFileSourceMap::new(&md_index, test_failures.backtick_offsets); for (relative_line_number, failures) in test_failures.by_line.iter() { - let absolute_line_number = - source_map.to_absolute_line_number(relative_line_number); + let file = match output_format { + OutputFormat::Cli => relative_fixture_path.as_str(), + OutputFormat::GitHub => absolute_fixture_path.as_str(), + }; + + let absolute_line_number = match source_map + .to_absolute_line_number(relative_line_number) + { + Ok(line_number) => line_number, + Err(last_line_number) => { + print!("{}", + output_format.display_error( + file, + last_line_number, + "Found a trailing assertion comment (e.g., `# revealed:` or `# error:`) \ + not followed by any statement." + ) + ); + + continue; + } + }; for failure in failures { - match output_format { - OutputFormat::Cli => { - let line_info = - format!("{relative_fixture_path}:{absolute_line_number}") - .cyan(); - println!(" {line_info} {failure}"); - } - OutputFormat::GitHub => println!( - "::error file={absolute_fixture_path},line={absolute_line_number}::{failure}" - ), - } + print!( + "{}", + output_format.display_error(file, absolute_line_number, failure) + ); } } } @@ -153,6 +166,49 @@ impl OutputFormat { const fn is_cli(self) -> bool { matches!(self, OutputFormat::Cli) } + + fn display_error(self, file: &str, line: OneIndexed, failure: impl Display) -> impl Display { + struct Display<'a, T> { + format: OutputFormat, + file: &'a str, + line: OneIndexed, + failure: T, + } + + impl std::fmt::Display for Display<'_, T> + where + T: std::fmt::Display, + { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let Display { + format, + file, + line, + failure, + } = self; + + match format { + OutputFormat::Cli => { + writeln!( + f, + " {file_line} {failure}", + file_line = format!("{file}:{line}").cyan() + ) + } + OutputFormat::GitHub => { + writeln!(f, "::error file={file},line={line}::{failure}") + } + } + } + } + + Display { + format: self, + file, + line, + failure, + } + } } fn run_test( diff --git a/crates/ty_test/src/parser.rs b/crates/ty_test/src/parser.rs index bc039eece6..ceba7285be 100644 --- a/crates/ty_test/src/parser.rs +++ b/crates/ty_test/src/parser.rs @@ -273,20 +273,36 @@ impl EmbeddedFileSourceMap { } } - pub(crate) fn to_absolute_line_number(&self, relative_line_number: OneIndexed) -> OneIndexed { - let mut absolute_line_number = 0; + /// Returns the absolute line number in the markdown file for a given line number + /// relative to the concatenated code blocks. + /// + /// Returns an `Err` if the relative line number is out of bounds where + /// the returned value is the absolute line number of the last code block. + /// + /// # Panics + /// If called when the markdown file has no code blocks. + pub(crate) fn to_absolute_line_number( + &self, + relative_line_number: OneIndexed, + ) -> std::result::Result { let mut relative_line_number = relative_line_number.get(); for (start_line, line_count) in &self.start_line_and_line_count { if relative_line_number > *line_count { relative_line_number -= *line_count; } else { - absolute_line_number = start_line + relative_line_number; - break; + let absolute_line_number = start_line + relative_line_number; + return Ok(OneIndexed::new(absolute_line_number) + .expect("absolute line number must be >= 1")); } } - OneIndexed::new(absolute_line_number).expect("Relative line number out of bounds") + let last_line_number = self + .start_line_and_line_count + .last() + .and_then(|(start_line, line_count)| OneIndexed::new(start_line + line_count)); + + Err(last_line_number.expect("markdown file to have at least one code block")) } } @@ -919,6 +935,7 @@ impl MdtestDirectives { mod tests { use ruff_python_ast::PySourceType; use ruff_python_trivia::textwrap::dedent; + use ruff_source_file::OneIndexed; use insta::assert_snapshot; @@ -931,6 +948,31 @@ mod tests { assert!(mf.tests().next().is_none()); } + #[test] + fn source_map_to_absolute_line_number() { + let map = super::EmbeddedFileSourceMap { + start_line_and_line_count: vec![(10, 5), (25, 3)], + }; + + let absolute = map + .to_absolute_line_number(OneIndexed::new(6).unwrap()) + .unwrap(); + assert_eq!(absolute.get(), 26); + } + + #[test] + fn source_map_reports_invalid_relative_line() { + let map = super::EmbeddedFileSourceMap { + start_line_and_line_count: vec![(9, 2)], + }; + + let error = map + .to_absolute_line_number(OneIndexed::new(3).unwrap()) + .unwrap_err(); + + assert_eq!(error.get(), 11); + } + #[test] fn single_file_test() { let source = dedent(