From 75015b0ed91b728f22897c3bf12af5e6451e163a Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Fri, 3 Jan 2025 13:45:56 -0500 Subject: [PATCH] Attribute panics to the mdtests that cause them (#15241) This updates the mdtest harness to catch any panics that occur during type checking, and to display the panic message as an mdtest failure. (We don't know which specific line causes the failure, so we attribute panics to the first line of the test case.) --- crates/red_knot_test/src/lib.rs | 18 ++++++++++++++++-- crates/red_knot_test/src/matcher.rs | 2 +- crates/ruff/src/commands/check.rs | 2 +- crates/ruff/src/commands/format.rs | 2 +- crates/ruff/src/lib.rs | 1 - crates/ruff_db/src/lib.rs | 1 + crates/{ruff => ruff_db}/src/panic.rs | 8 ++++---- 7 files changed, 24 insertions(+), 10 deletions(-) rename crates/{ruff => ruff_db}/src/panic.rs (87%) diff --git a/crates/red_knot_test/src/lib.rs b/crates/red_knot_test/src/lib.rs index d91a5e9b69a35..001bd2a70df4f 100644 --- a/crates/red_knot_test/src/lib.rs +++ b/crates/red_knot_test/src/lib.rs @@ -6,10 +6,11 @@ use red_knot_python_semantic::types::check_types; use red_knot_python_semantic::Program; use ruff_db::diagnostic::{Diagnostic, ParseDiagnostic}; use ruff_db::files::{system_path_to_file, File, Files}; +use ruff_db::panic::catch_unwind; use ruff_db::parsed::parsed_module; use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; use ruff_db::testing::{setup_logging, setup_logging_with_filter}; -use ruff_source_file::LineIndex; +use ruff_source_file::{LineIndex, OneIndexed}; use ruff_text_size::TextSize; use salsa::Setter; @@ -136,7 +137,20 @@ fn run_test(db: &mut db::Db, test: &parser::MarkdownTest) -> Result<(), Failures }) .collect(); - let type_diagnostics = check_types(db, test_file.file); + let type_diagnostics = match catch_unwind(|| check_types(db, test_file.file)) { + Ok(type_diagnostics) => type_diagnostics, + Err(info) => { + let mut by_line = matcher::FailuresByLine::default(); + by_line.push( + OneIndexed::from_zero_indexed(0), + info.info.split('\n').map(String::from).collect(), + ); + return Some(FileFailures { + backtick_offset: test_file.backtick_offset, + by_line, + }); + } + }; diagnostics.extend(type_diagnostics.into_iter().map(|diagnostic| { let diagnostic: Box = Box::new((*diagnostic).clone()); diagnostic diff --git a/crates/red_knot_test/src/matcher.rs b/crates/red_knot_test/src/matcher.rs index 40a5b790be7ad..65d8409957ede 100644 --- a/crates/red_knot_test/src/matcher.rs +++ b/crates/red_knot_test/src/matcher.rs @@ -27,7 +27,7 @@ impl FailuresByLine { }) } - fn push(&mut self, line_number: OneIndexed, messages: Vec) { + pub(super) fn push(&mut self, line_number: OneIndexed, messages: Vec) { let start = self.failures.len(); self.failures.extend(messages); self.lines.push(LineFailures { diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index 0a547b3551828..56436fed52129 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -11,6 +11,7 @@ use log::{debug, error, warn}; use rayon::prelude::*; use rustc_hash::FxHashMap; +use ruff_db::panic::catch_unwind; use ruff_diagnostics::Diagnostic; use ruff_linter::message::Message; use ruff_linter::package::PackageRoot; @@ -27,7 +28,6 @@ use ruff_workspace::resolver::{ use crate::args::ConfigArguments; use crate::cache::{Cache, PackageCacheMap, PackageCaches}; use crate::diagnostics::Diagnostics; -use crate::panic::catch_unwind; /// Run the linter over a collection of files. #[allow(clippy::too_many_arguments)] diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 2d49f0d5e0539..74ea1035fb350 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -15,6 +15,7 @@ use rustc_hash::FxHashSet; use thiserror::Error; use tracing::debug; +use ruff_db::panic::{catch_unwind, PanicError}; use ruff_diagnostics::SourceMap; use ruff_linter::fs; use ruff_linter::logging::{DisplayParseError, LogLevel}; @@ -32,7 +33,6 @@ use ruff_workspace::FormatterSettings; use crate::args::{ConfigArguments, FormatArguments, FormatRange}; use crate::cache::{Cache, FileCacheKey, PackageCacheMap, PackageCaches}; -use crate::panic::{catch_unwind, PanicError}; use crate::resolve::resolve; use crate::{resolve_default_files, ExitStatus}; diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index b45e588416106..8469c577d95b3 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -29,7 +29,6 @@ pub mod args; mod cache; mod commands; mod diagnostics; -mod panic; mod printer; pub mod resolve; mod stdin; diff --git a/crates/ruff_db/src/lib.rs b/crates/ruff_db/src/lib.rs index d7d85a0db975d..d98418e7c0e78 100644 --- a/crates/ruff_db/src/lib.rs +++ b/crates/ruff_db/src/lib.rs @@ -10,6 +10,7 @@ pub mod diagnostic; pub mod display; pub mod file_revision; pub mod files; +pub mod panic; pub mod parsed; pub mod source; pub mod system; diff --git a/crates/ruff/src/panic.rs b/crates/ruff_db/src/panic.rs similarity index 87% rename from crates/ruff/src/panic.rs rename to crates/ruff_db/src/panic.rs index 5947873ef775d..c4983ee5a766f 100644 --- a/crates/ruff/src/panic.rs +++ b/crates/ruff_db/src/panic.rs @@ -1,7 +1,7 @@ #[derive(Default, Debug)] -pub(crate) struct PanicError { - pub(crate) info: String, - pub(crate) backtrace: Option, +pub struct PanicError { + pub info: String, + pub backtrace: Option, } impl std::fmt::Display for PanicError { @@ -21,7 +21,7 @@ thread_local! { /// [`catch_unwind`](std::panic::catch_unwind) wrapper that sets a custom [`set_hook`](std::panic::set_hook) /// to extract the backtrace. The original panic-hook gets restored before returning. -pub(crate) fn catch_unwind(f: F) -> Result +pub fn catch_unwind(f: F) -> Result where F: FnOnce() -> R + std::panic::UnwindSafe, {