Skip to content

Commit

Permalink
Catch panics in formatter (#7377)
Browse files Browse the repository at this point in the history
## 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 #7247.

## Test Plan

I created a file `foo.py` with a syntax error, and a file `bar.py` with
an intentional panic.

<img width="1624" alt="Screen Shot 2023-09-13 at 10 25 22 PM"
src="https://github.com/astral-sh/ruff/assets/1309177/605c2839-ad02-4376-a2e9-d5a593ab660f">

<img width="1624" alt="Screen Shot 2023-09-13 at 10 25 24 PM"
src="https://github.com/astral-sh/ruff/assets/1309177/b1381909-157c-48cb-9630-d0bbfcb1b640">
  • Loading branch information
charliermarsh authored Sep 14, 2023
1 parent ec2f229 commit 5d21b9c
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 32 deletions.
27 changes: 12 additions & 15 deletions crates/ruff/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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
);
Expand All @@ -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,
Expand All @@ -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,
);
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_cli/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
Expand Down
46 changes: 38 additions & 8 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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))),
}
Expand All @@ -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)]
Expand Down Expand Up @@ -255,6 +261,7 @@ pub(crate) enum FormatCommandError {
Read(Option<PathBuf>, io::Error),
Write(Option<PathBuf>, io::Error),
FormatModule(Option<PathBuf>, FormatModuleError),
Panic(Option<PathBuf>, PanicError),
}

impl Display for FormatCommandError {
Expand Down Expand Up @@ -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()
)
}
}
}
}
}
8 changes: 5 additions & 3 deletions crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl From<ParseError> 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,
Expand Down

0 comments on commit 5d21b9c

Please sign in to comment.