Skip to content

Commit

Permalink
Gracefully handle lint panics
Browse files Browse the repository at this point in the history
Create a diagnostics for files that panic during linting rather than crashing Ruff.
  • Loading branch information
MichaReiser committed Mar 14, 2023
1 parent d5700d7 commit 75456f4
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 10 deletions.
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ textwrap = { version = "0.16.0" }
toml = { version = "0.7.2" }

[profile.release]
panic = "abort"
lto = "thin"
lto = "fat"
codegen-units = 1
opt-level = 3

Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_cli/src/commands/linter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::fmt::Write;
use std::io;
use std::io::BufWriter;
use std::io::Write;

use anyhow::Result;
use itertools::Itertools;
Expand Down Expand Up @@ -41,7 +41,7 @@ pub fn linter(format: HelpFormat) -> Result<()> {
.join("/"),
prefix => prefix.to_string(),
};
output.push_str(&format!("{:>4} {}\n", prefix, linter.name()));
writeln!(output, "{:>4} {}", prefix, linter.name()).unwrap();
}
}

Expand All @@ -65,7 +65,7 @@ pub fn linter(format: HelpFormat) -> Result<()> {
}
}

write!(stdout, "{output}")?;
io::Write::write_fmt(&mut stdout, format_args!("{output}"))?;

Ok(())
}
46 changes: 42 additions & 4 deletions crates/ruff_cli/src/commands/run.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
use std::io;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::time::Instant;

use anyhow::Result;
use colored::Colorize;
use ignore::Error;
use log::{debug, error};
use log::{debug, error, warn};
#[cfg(not(target_family = "wasm"))]
use rayon::prelude::*;

use crate::panic::catch_unwind;
use ruff::message::{Location, Message};
use ruff::registry::Rule;
use ruff::resolver::PyprojectDiscovery;
use ruff::settings::flags;
use ruff::settings::{flags, AllSettings};
use ruff::{fix, fs, packaging, resolver, warn_user_once, IOError, Range};
use ruff_diagnostics::Diagnostic;

use crate::args::Overrides;
use crate::cache;
use crate::diagnostics::{lint_path, Diagnostics};
use crate::diagnostics::Diagnostics;

/// Run the linter over a collection of files.
pub fn run(
Expand Down Expand Up @@ -83,6 +84,7 @@ pub fn run(
.and_then(|parent| package_roots.get(parent))
.and_then(|package| *package);
let settings = resolver.resolve_all(path, pyproject_strategy);

lint_path(path, package, settings, cache, noqa, autofix)
.map_err(|e| (Some(path.to_owned()), e.to_string()))
}
Expand Down Expand Up @@ -135,3 +137,39 @@ pub fn run(

Ok(diagnostics)
}

/// Wraps [`lint_path`](crate::diagnostics::lint_path) in a [`catch_unwind`](std::panic::catch_unwind) and emits
/// a diagnostic if the linting the file panics.
fn lint_path(
path: &Path,
package: Option<&Path>,
settings: &AllSettings,
cache: flags::Cache,
noqa: flags::Noqa,
autofix: fix::FixMode,
) -> Result<Diagnostics> {
let result = catch_unwind(|| {
crate::diagnostics::lint_path(path, package, settings, cache, noqa, autofix)
});

match result {
Ok(inner) => inner,
Err(error) => {
let message = r#"This indicates a bug in `ruff`. If you could open an issue at:
https://github.com/charliermarsh/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!
"#;

warn!(
"{}{}{} {message}\n{error}",
"Linting panicked ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);

Ok(Diagnostics::default())
}
}
}
2 changes: 1 addition & 1 deletion crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub mod args;
mod cache;
mod commands;
mod diagnostics;
mod panic;
mod printer;
mod resolve;

Expand Down Expand Up @@ -46,7 +47,6 @@ pub fn run(
log_level_args,
}: Args,
) -> Result<ExitStatus> {
#[cfg(not(debug_assertions))]
{
use colored::Colorize;

Expand Down
46 changes: 46 additions & 0 deletions crates/ruff_cli/src/panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#[derive(Default, Debug)]
pub(crate) struct PanicError {
pub info: String,
pub backtrace: Option<std::backtrace::Backtrace>,
}

impl std::fmt::Display for PanicError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{}", self.info)?;
if let Some(backtrace) = &self.backtrace {
writeln!(f, "Backtrace: {backtrace}")
} else {
Ok(())
}
}
}

thread_local! {
static LAST_PANIC: std::cell::Cell<Option<PanicError>> = std::cell::Cell::new(None);
}

/// [`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, R>(f: F) -> Result<R, PanicError>
where
F: FnOnce() -> R + std::panic::UnwindSafe,
{
let prev = std::panic::take_hook();
std::panic::set_hook(Box::new(|info| {
let info = info.to_string();
let backtrace = std::backtrace::Backtrace::force_capture();
LAST_PANIC.with(|cell| {
cell.set(Some(PanicError {
info,
backtrace: Some(backtrace),
}));
});
}));

let result = std::panic::catch_unwind(f)
.map_err(|_| LAST_PANIC.with(std::cell::Cell::take).unwrap_or_default());

std::panic::set_hook(prev);

result
}

0 comments on commit 75456f4

Please sign in to comment.