Repository: astral-sh/ruff
File: crates/ty_python_semantic/src/diagnostic.rs (Rust)

Code before:

    let mut buffer = String::new();
    for element in elements {
        write!(&mut buffer, "`{element}`, ").ok();
    }
    write!(&mut buffer, "`{penultimate_element}` and `{final_element}`").ok();

    buffer
}

/// An abstraction for mutating a diagnostic.
///
/// Callers likely should use `LintDiagnosticGuard` via
/// `InferContext::report_lint` instead. This guard is only intended for use
/// with non-lint diagnostics or non-type checking diagnostics. It is fundamentally lower level and easier to
/// get things wrong by using it.
///
/// Unlike `LintDiagnosticGuard`, this API does not guarantee that the
/// constructed `Diagnostic` not only has a primary annotation, but its
/// associated file is equivalent to the file being type checked. As a result,
/// if either is violated, then the `Drop` impl on `DiagnosticGuard` will
/// panic.
pub(super) struct DiagnosticGuard<'sink> {
    file: File,

    sink: &'sink std::cell::RefCell<TypeCheckDiagnostics>,

    /// The diagnostic that we want to report.
    ///
    /// This is always `Some` until the `Drop` impl.
    diag: Option<Diagnostic>,
}

impl<'sink> DiagnosticGuard<'sink> {
    pub(crate) fn new(
        file: File,
        sink: &'sink std::cell::RefCell<TypeCheckDiagnostics>,
        diag: Diagnostic,
    ) -> Self {
        Self {
            file,
            sink,
            diag: Some(diag),
        }
    }
}

impl std::ops::Deref for DiagnosticGuard<'_> {
    type Target = Diagnostic;

    fn deref(&self) -> &Diagnostic {

Review comment (bug):
What goes wrong when you use `&mut TypeCheckDiagnostics` here?

It might be worth a brief comment saying why interior mutability is being used here.

Code after:
    let penultimate_element = elements.next_back().unwrap();

    let mut buffer = String::new();
    for element in elements {
        write!(&mut buffer, "`{element}`, ").ok();
    }
    write!(&mut buffer, "`{penultimate_element}` and `{final_element}`").ok();

    buffer
}

/// An abstraction for mutating a diagnostic.
///
/// Callers likely should use `LintDiagnosticGuard` via
/// `InferContext::report_lint` instead. This guard is only intended for use
/// with non-lint diagnostics or non-type checking diagnostics. It is fundamentally lower level and easier to
/// get things wrong by using it.
///
/// Unlike `LintDiagnosticGuard`, this API does not guarantee that the
/// constructed `Diagnostic` not only has a primary annotation, but its
/// associated file is equivalent to the file being type checked. As a result,
/// if either is violated, then the `Drop` impl on `DiagnosticGuard` will
/// panic.
pub(super) struct DiagnosticGuard<'sink> {
    /// The file of the primary span (to which file does this diagnostic belong).
    file: File,

    /// The target where to emit the diagnostic to.
    ///
    /// We use a [`RefCell`] here over a `&mut TypeCheckDiagnostics` to ensure the fact that
    /// `InferContext` (and other contexts with diagnostics) use a [`RefCell`] internally
    /// remains abstracted away. Specifically, we want to ensure that calling `report_lint` on
    /// `InferContext` twice doesn't result in a panic:
    ///
    /// ```ignore
    /// let diag1 = context.report_lint(...);
    ///
    /// // would panic if using a `&mut TypeCheckDiagnostics`
    /// // because of a second mutable borrow.
    /// let diag2 = context.report_lint(...);
    /// ```
    sink: &'sink RefCell<TypeCheckDiagnostics>,

    /// The diagnostic that we want to report.
    ///
    /// This is always `Some` until the `Drop` impl.
    diag: Option<Diagnostic>,
}

impl<'sink> DiagnosticGuard<'sink> {
    pub(crate) fn new(