Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 79 additions & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use std::path::Path;
use itertools::Itertools;
use log::debug;
use rustc_hash::{FxHashMap, FxHashSet};
use smallvec::SmallVec;

use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticTag, IntoDiagnosticMessage, Span};
use ruff_diagnostics::{Applicability, Fix, IsolationLevel};
Expand Down Expand Up @@ -421,6 +422,18 @@ impl<'a> Checker<'a> {
self.context.report_diagnostic_if_enabled(kind, range)
}

/// Return a [`DiagnosticGuard`] for reporting a diagnostic, with its fix title deferred.
///
/// Prefer [`Checker::report_diagnostic`] unless you need to attach sub-diagnostics before the
/// fix title. See its documentation for more details.
pub(crate) fn report_custom_diagnostic<'chk, T: Violation>(
&'chk self,
kind: T,
range: TextRange,
) -> DiagnosticGuard<'chk, 'a> {
self.context.report_custom_diagnostic(kind, range)
}

/// Adds a [`TextRange`] to the set of ranges of variable names
/// flagged in `flake8-bugbear` violations so far.
///
Expand Down Expand Up @@ -3367,6 +3380,7 @@ impl<'a> LintContext<'a> {
context: self,
diagnostic: Some(kind.into_diagnostic(range, &self.source_file)),
rule: T::rule(),
before_drop_fns: SmallVec::new(),
}
}

Expand All @@ -3386,12 +3400,48 @@ impl<'a> LintContext<'a> {
context: self,
diagnostic: Some(kind.into_diagnostic(range, &self.source_file)),
rule,
before_drop_fns: SmallVec::new(),
})
} else {
None
}
}

/// Return a [`DiagnosticGuard`] for reporting a diagnostic, with its fix title deferred.
///
/// Prefer [`LintContext::report_diagnostic`] unless you need to attach sub-diagnostics before
/// the fix title. See its documentation for more details.
#[expect(clippy::needless_pass_by_value)]
pub(crate) fn report_custom_diagnostic<'chk, T: Violation>(
&'chk self,
kind: T,
range: TextRange,
) -> DiagnosticGuard<'chk, 'a> {
let diagnostic = crate::message::create_lint_diagnostic(
kind.message(),
Option::<&'static str>::None,
range,
None,
None,
self.source_file.clone(),
None,
T::rule(),
);

let mut guard = DiagnosticGuard {
context: self,
diagnostic: Some(diagnostic),
rule: T::rule(),
before_drop_fns: SmallVec::new(),
};

if let Some(fix_title) = kind.fix_title() {
guard.before_drop(move |diag| diag.help(&fix_title));
}

guard
}

#[inline]
pub(crate) const fn is_rule_enabled(&self, rule: Rule) -> bool {
self.rules.enabled(rule)
Expand Down Expand Up @@ -3433,6 +3483,8 @@ impl<'a> LintContext<'a> {
}
}

type BeforeDropFn = Box<dyn Fn(&mut Diagnostic)>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be declared here if it's only used in one place, or would it be better to inline it below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a clippy lint for this, otherwise I would have left it in place. I could just use an expect attribute but I didn't really mind pulling out the type alias.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think Clippy is too aggressive about this lol. It's just a boxed closure!


/// An abstraction for mutating a diagnostic.
///
/// Callers can build this guard by starting with `Checker::report_diagnostic`.
Expand All @@ -3447,6 +3499,8 @@ pub(crate) struct DiagnosticGuard<'a, 'b> {
///
/// This is always `Some` until the `Drop` (or `defuse`) call.
diagnostic: Option<Diagnostic>,
/// Functions to call before dropping the guard and storing the diagnostic.
before_drop_fns: SmallVec<[BeforeDropFn; 1]>,
rule: Rule,
}

Expand Down Expand Up @@ -3508,6 +3562,27 @@ impl DiagnosticGuard<'_, '_> {
let ann = self.primary_annotation_mut().unwrap();
ann.push_tag(tag);
}

/// Register a function to call before dropping the guard.
///
/// This is intended for use with the [`Checker::report_custom_diagnostic`]
/// or [`LintContext::report_custom_diagnostic`] methods in cases where a
/// fix title should be added as a `help` diagnostic after all other
/// sub-diagnostics. `report_custom_diagnostic` uses this method to defer
/// adding the fix title, but you can defer additional diagnostics if you
/// want them to appear after the fix title. For example:
///
/// ```ignore
/// let mut diagnostic = checker.report_custom_diagnostic(MyViolation, range);
/// diagnostic.info("This will appear first");
/// diagnostic.before_drop(|diag| diag.info("This will appear last, after the fix title"));
/// ```
pub(crate) fn before_drop<F>(&mut self, f: F)
where
F: Fn(&mut Diagnostic) + 'static,
{
self.before_drop_fns.push(Box::new(f));
}
}

impl DiagnosticGuard<'_, '_> {
Expand Down Expand Up @@ -3592,7 +3667,10 @@ impl Drop for DiagnosticGuard<'_, '_> {
return;
}

if let Some(diagnostic) = self.diagnostic.take() {
if let Some(mut diagnostic) = self.diagnostic.take() {
for f in &self.before_drop_fns {
f(&mut diagnostic);
}
self.context.diagnostics.borrow_mut().push(diagnostic);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub(crate) fn implicit_string_concatenation_in_collection_literal(
continue;
}

let mut diagnostic = checker.report_diagnostic(
let mut diagnostic = checker.report_custom_diagnostic(
ImplicitStringConcatenationInCollectionLiteral,
string_like.range(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |______________________________________________________________^
6 | )
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
1 | facts = (
2 | "Lobsters have blue blood.",
3 | "The liver is the only human organ that can fully regenerate itself.",
Expand All @@ -35,8 +35,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |______________________________________________________________^
13 | ]
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
8 | facts = [
9 | "Lobsters have blue blood.",
10 | "The liver is the only human organ that can fully regenerate itself.",
Expand All @@ -59,8 +59,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |______________________________________________________________^
20 | }
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
15 | facts = {
16 | "Lobsters have blue blood.",
17 | "The liver is the only human organ that can fully regenerate itself.",
Expand All @@ -83,8 +83,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |_________________________^
33 | )
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
27 | }
28 |
29 | facts = (
Expand All @@ -108,8 +108,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |_________________________^
39 | ]
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
33 | )
34 |
35 | facts = [
Expand All @@ -133,8 +133,8 @@ ISC004 [*] Unparenthesized implicit string concatenation in collection
| |_________________________^
45 | }
|
help: Wrap implicitly concatenated strings in parentheses
help: Did you forget a comma?
help: Wrap implicitly concatenated strings in parentheses
39 | ]
40 |
41 | facts = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(crate) fn non_octal_permissions(checker: &Checker, call: &ExprCall) {
return;
}

let mut diagnostic = checker.report_diagnostic(NonOctalPermissions, mode_arg.range());
let mut diagnostic = checker.report_custom_diagnostic(NonOctalPermissions, mode_arg.range());

let Some(mode) = int.as_u16() else {
return;
Expand Down
Loading