Better handling for panics during linting

- Convert panics to diagnostics with id `Panic`, severity `Fatal`, and
  the error as the diagnostic message, annotated with a `Span` with
  empty code block and no range.
- Updates the post-linting message diagnostic handling to track the
  maximum severity seen, and then prints the "report a bug in ruff"
  message only if the max severity was `Fatal`
- Adds a new test rule to exercise panics in lint rules

This depends on the sorting changes since it creates diagnostics with no
range specified.

Author: amyreese

crates/ruff/src/commands/check.rs

@@ -6,12 +6,12 @@ use std::time::Instant;
 use anyhow::Result;
 use colored::Colorize;
 use ignore::Error;
-use log::{debug, error, warn};
+use log::{debug, warn};
 #[cfg(not(target_family = "wasm"))]
 use rayon::prelude::*;
 use rustc_hash::FxHashMap;
 
-use ruff_db::diagnostic::Diagnostic;
+use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, Span};
 use ruff_db::panic::catch_unwind;
 use ruff_linter::package::PackageRoot;
 use ruff_linter::registry::Rule;
@@ -193,21 +193,20 @@ 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:
-
-    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!
-";
-
-            error!(
-                "{}{}{} {message}\n{error}",
+            let message = format!(
+                "{}{}{}\n{error}",
                 "Panicked while linting ".bold(),
                 fs::relativize_path(path).bold(),
                 ":".bold()
             );
-
-            Ok(Diagnostics::default())
+            let mut diagnostic = Diagnostic::new(
+                DiagnosticId::Panic,
+                ruff_db::diagnostic::Severity::Fatal,
+                message,
+            );
+            let span = Span::from(SourceFileBuilder::new(path.to_string_lossy(), "").finish());
+            diagnostic.annotate(Annotation::primary(span));
+            Ok(Diagnostics::new(vec![diagnostic], FxHashMap::default()))
         }
     }
 }

crates/ruff/src/lib.rs

@@ -9,10 +9,11 @@ use std::sync::mpsc::channel;
 use anyhow::Result;
 use clap::CommandFactory;
 use colored::Colorize;
-use log::warn;
+use log::{error, warn};
 use notify::{RecursiveMode, Watcher, recommended_watcher};
 
 use args::{GlobalConfigArgs, ServerCommand};
+use ruff_db::diagnostic::{Diagnostic, Severity};
 use ruff_linter::logging::{LogLevel, set_up_logging};
 use ruff_linter::settings::flags::FixMode;
 use ruff_linter::settings::types::OutputFormat;
@@ -444,6 +445,22 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
         }
 
         if !cli.exit_zero {
+            let max_severity = diagnostics
+                .inner
+                .iter()
+                .map(Diagnostic::severity)
+                .max()
+                .unwrap_or(Severity::Info);
+            if max_severity.is_fatal() {
+                let message = "Panic during linting indicates a bug in Ruff. If you could open an issue at:
+
+https://github.com/astral-sh/ruff/issues/new?title=%5BLinter%20panic%5D
+
+...with the relevant file contents, the `pyproject.toml` settings, and the stack trace above, we'd be very appreciative!
+";
+                error!("{message}");
+                return Ok(ExitStatus::Error);
+            }
             if cli.diff {
                 // If we're printing a diff, we always want to exit non-zero if there are
                 // any fixable violations (since we've printed the diff, but not applied the

crates/ruff_linter/src/codes.rs

@@ -1080,6 +1080,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "950") => (RuleGroup::Stable, rules::ruff::rules::RedirectedToTestRule),
         #[cfg(any(feature = "test-rules", test))]
         (Ruff, "960") => (RuleGroup::Removed, rules::ruff::rules::RedirectedFromPrefixTestRule),
+        #[cfg(any(feature = "test-rules", test))]
+        (Ruff, "990") => (RuleGroup::Preview, rules::ruff::rules::PanicyTestRule),
 
 
         // flake8-django

crates/ruff_linter/src/linter.rs

@@ -319,6 +319,9 @@ pub fn check_path(
                         &context,
                     );
                 }
+                Rule::PanicyTestRule => {
+                    test_rules::PanicyTestRule::diagnostic(locator, comment_ranges, &context);
+                }
                 _ => unreachable!("All test rules must have an implementation"),
             }
         }

crates/ruff_linter/src/rules/ruff/rules/test_rules.rs

@@ -46,6 +46,7 @@ pub(crate) const TEST_RULES: &[Rule] = &[
     Rule::RedirectedFromTestRule,
     Rule::RedirectedToTestRule,
     Rule::RedirectedFromPrefixTestRule,
+    Rule::PanicyTestRule,
 ];
 
 pub(crate) trait TestRule {
@@ -477,3 +478,36 @@ impl TestRule for RedirectedFromPrefixTestRule {
         );
     }
 }
+
+/// # What it does
+/// Fake rule for testing panics.
+///
+/// # Why is this bad?
+/// Panics are bad.
+///
+/// # Example
+/// ```python
+/// foo
+/// ```
+///
+/// Use instead:
+/// ```python
+/// bar
+/// ```
+#[derive(ViolationMetadata)]
+pub(crate) struct PanicyTestRule;
+
+impl Violation for PanicyTestRule {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::None;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        "If you see this, maybe panic!".to_string()
+    }
+}
+
+impl TestRule for PanicyTestRule {
+    fn diagnostic(_locator: &Locator, _comment_ranges: &CommentRanges, _context: &LintContext) {
+        panic!("Intentional panic!");
+    }
+}