From 5d21b9c22ebd50dd9cc9f2479b67985329df07cc Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 14 Sep 2023 11:44:16 -0400 Subject: [PATCH] Catch panics in formatter (#7377) ## Summary This PR ensures that we catch and render panics in the formatter identically to other kinds of errors. It also improves the consistency in error rendering throughout and makes a few stylistic changes to the messages. Closes https://github.com/astral-sh/ruff/issues/7247. ## Test Plan I created a file `foo.py` with a syntax error, and a file `bar.py` with an intentional panic. Screen Shot 2023-09-13 at 10 25 22 PM Screen Shot 2023-09-13 at 10 25 24 PM --- crates/ruff/src/linter.rs | 27 +++++++-------- crates/ruff_cli/src/commands/check.rs | 10 +++--- crates/ruff_cli/src/commands/format.rs | 46 ++++++++++++++++++++----- crates/ruff_cli/src/lib.rs | 8 +++-- crates/ruff_python_formatter/src/lib.rs | 2 +- 5 files changed, 61 insertions(+), 32 deletions(-) diff --git a/crates/ruff/src/linter.rs b/crates/ruff/src/linter.rs index 19612ab968da1..2b08fec96b279 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -36,9 +36,6 @@ use crate::settings::{flags, Settings}; use crate::source_kind::SourceKind; use crate::{directives, fs}; -const CARGO_PKG_NAME: &str = env!("CARGO_PKG_NAME"); -const CARGO_PKG_REPOSITORY: &str = env!("CARGO_PKG_REPOSITORY"); - /// A [`Result`]-like type that returns both data and an error. Used to return /// diagnostics even in the face of parse errors, since many diagnostics can be /// generated without a full AST. @@ -543,8 +540,9 @@ fn report_failed_to_converge_error(path: &Path, transformed: &str, diagnostics: let codes = collect_rule_codes(diagnostics.iter().map(|diagnostic| diagnostic.kind.rule())); if cfg!(debug_assertions) { eprintln!( - "{}: Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---", + "{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---", "debug error".red().bold(), + ":".bold(), MAX_ITERATIONS, fs::relativize_path(path), codes, @@ -553,18 +551,17 @@ fn report_failed_to_converge_error(path: &Path, transformed: &str, diagnostics: } else { eprintln!( r#" -{}: Failed to converge after {} iterations. +{}{} Failed to converge after {} iterations. -This indicates a bug in `{}`. If you could open an issue at: +This indicates a bug in Ruff. If you could open an issue at: - {}/issues/new?title=%5BInfinite%20loop%5D + https://github.com/astral-sh/ruff/issues/new?title=%5BInfinite%20loop%5D ...quoting the contents of `{}`, the rule codes {}, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, "error".red().bold(), + ":".bold(), MAX_ITERATIONS, - CARGO_PKG_NAME, - CARGO_PKG_REPOSITORY, fs::relativize_path(path), codes ); @@ -581,8 +578,9 @@ fn report_autofix_syntax_error( let codes = collect_rule_codes(rules); if cfg!(debug_assertions) { eprintln!( - "{}: Autofix introduced a syntax error in `{}` with rule codes {}: {}\n---\n{}\n---", + "{}{} Autofix introduced a syntax error in `{}` with rule codes {}: {}\n---\n{}\n---", "error".red().bold(), + ":".bold(), fs::relativize_path(path), codes, error, @@ -591,17 +589,16 @@ fn report_autofix_syntax_error( } else { eprintln!( r#" -{}: Autofix introduced a syntax error. Reverting all changes. +{}{} Autofix introduced a syntax error. Reverting all changes. -This indicates a bug in `{}`. If you could open an issue at: +This indicates a bug in Ruff. If you could open an issue at: - {}/issues/new?title=%5BAutofix%20error%5D + https://github.com/astral-sh/ruff/issues/new?title=%5BAutofix%20error%5D ...quoting the contents of `{}`, the rule codes {}, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, "error".red().bold(), - CARGO_PKG_NAME, - CARGO_PKG_REPOSITORY, + ":".bold(), fs::relativize_path(path), codes, ); diff --git a/crates/ruff_cli/src/commands/check.rs b/crates/ruff_cli/src/commands/check.rs index f760a5fb58948..493e4117f0715 100644 --- a/crates/ruff_cli/src/commands/check.rs +++ b/crates/ruff_cli/src/commands/check.rs @@ -211,16 +211,16 @@ fn lint_path( match result { Ok(inner) => inner, Err(error) => { - let message = r#"This indicates a bug in `ruff`. If you could open an issue at: + let message = r#"This indicates a bug in Ruff. If you could open an issue at: -https://github.com/astral-sh/ruff/issues/new?title=%5BLinter%20panic%5D + https://github.com/astral-sh/ruff/issues/new?title=%5BLinter%20panic%5D -with the relevant file contents, the `pyproject.toml` settings, and the following stack trace, we'd be very appreciative! +...with the relevant file contents, the `pyproject.toml` settings, and the following stack trace, we'd be very appreciative! "#; - warn!( + error!( "{}{}{} {message}\n{error}", - "Linting panicked ".bold(), + "Panicked while linting ".bold(), fs::relativize_path(path).bold(), ":".bold() ); diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index e278f54edfc0f..e150d733086af 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -6,6 +6,7 @@ use std::time::Instant; use anyhow::Result; use colored::Colorize; +use log::error; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use thiserror::Error; @@ -22,6 +23,7 @@ use ruff_source_file::{find_newline, LineEnding}; use ruff_workspace::resolver::python_files_in_path; use crate::args::{FormatArguments, Overrides}; +use crate::panic::{catch_unwind, PanicError}; use crate::resolve::resolve; use crate::ExitStatus; @@ -85,7 +87,13 @@ pub(crate) fn format( .with_line_width(LineWidth::from(NonZeroU16::from(line_length))) .with_preview(preview); debug!("Formatting {} with {:?}", path.display(), options); - Some(format_path(path, options, mode)) + + Some(match catch_unwind(|| format_path(path, options, mode)) { + Ok(inner) => inner, + Err(error) => { + Err(FormatCommandError::Panic(Some(path.to_path_buf()), error)) + } + }) } Err(err) => Some(Err(FormatCommandError::Ignore(err))), } @@ -95,22 +103,20 @@ pub(crate) fn format( Err(err) => Right(err), }); let duration = start.elapsed(); + debug!( "Formatted {} files in {:.2?}", results.len() + errors.len(), duration ); - let summary = FormatResultSummary::new(results, mode); - // Report on any errors. - if !errors.is_empty() { - warn!("Encountered {} errors while formatting:", errors.len()); - for error in &errors { - warn!("{error}"); - } + for error in &errors { + error!("{error}"); } + let summary = FormatResultSummary::new(results, mode); + // Report on the formatting changes. if log_level >= LogLevel::Default { #[allow(clippy::print_stdout)] @@ -255,6 +261,7 @@ pub(crate) enum FormatCommandError { Read(Option, io::Error), Write(Option, io::Error), FormatModule(Option, FormatModuleError), + Panic(Option, PanicError), } impl Display for FormatCommandError { @@ -320,6 +327,29 @@ impl Display for FormatCommandError { write!(f, "{}{} {err}", "Failed to format".bold(), ":".bold()) } } + Self::Panic(path, err) => { + let message = r#"This indicates a bug in Ruff. If you could open an issue at: + + https://github.com/astral-sh/ruff/issues/new?title=%5BFormatter%20panic%5D + +...with the relevant file contents, the `pyproject.toml` settings, and the following stack trace, we'd be very appreciative! +"#; + if let Some(path) = path { + write!( + f, + "{}{}{} {message}\n{err}", + "Panicked while formatting ".bold(), + fs::relativize_path(path).bold(), + ":".bold() + ) + } else { + write!( + f, + "{} {message}\n{err}", + "Panicked while formatting.".bold() + ) + } + } } } } diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index 08057794b15e3..8e1b9d6cbe8b1 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -111,13 +111,15 @@ pub fn run( { eprintln!( r#" -{}: `ruff` crashed. This indicates a bug in `ruff`. If you could open an issue at: +{}{} {} If you could open an issue at: -https://github.com/astral-sh/ruff/issues/new?title=%5BPanic%5D + https://github.com/astral-sh/ruff/issues/new?title=%5BPanic%5D -quoting the executed command, along with the relevant file contents and `pyproject.toml` settings, we'd be very appreciative! +...quoting the executed command, along with the relevant file contents and `pyproject.toml` settings, we'd be very appreciative! "#, "error".red().bold(), + ":".bold(), + "Ruff crashed.".bold(), ); } default_panic_hook(info); diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index 4d96a9374eda8..4eb54f1b348f6 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -120,7 +120,7 @@ impl From for FormatModuleError { } } -#[tracing::instrument(level=Level::TRACE, skip_all, err)] +#[tracing::instrument(level=Level::TRACE, skip_all)] pub fn format_module( contents: &str, options: PyFormatOptions,