Skip to content

Commit

Permalink
Auto merge of #13136 - xFrednet:07797-restriction-and-then, r=blyxyas
Browse files Browse the repository at this point in the history
Make restriction lint's use `span_lint_and_then` (a -> e)

This migrates a few restriction lints to use `span_lint_and_then`. This change is motivated by #7797.

I'm also interested if it will have an impact on performance. With some of these lints, like [`clippy::implicit_return`](https://rust-lang.github.io/rust-clippy/master/index.html#/implicit_return) I expect an impact, as it was previously creating a suggestion **for every implicit return** which is just wild.

I've also cleaned up some lint message. Mostly minor stuff. For example: suggestions with a longer message than `"try"` now use `SuggestionStyle::ShowAlways`

---

`@blyxyas` Could you benchmark this PR? I want to get all the numbers :3

---

This also crashed our new lintcheck CI with the following message:

> Error: $GITHUB_STEP_SUMMARY upload aborted, supports content up to a size of 1024k, got 46731k. For more information see: https://docs.github.com/actions/using-workflows/workflow-commands-for-github-actions#adding-a-markdown-summary

Which is just wild. Like, I've [tested the first 20 lints](https://github.com/xFrednet/rust-clippy/actions/runs/10027528172) and got like four changes and then this. 50 MB of changed lint messages o.O. Looks like I'll create a separate PR to fix that step ^^

---

cc: #7797

changelog: none

r? `@blyxyas`
  • Loading branch information
bors committed Aug 5, 2024
2 parents e17d254 + 0532104 commit c082bc2
Show file tree
Hide file tree
Showing 34 changed files with 927 additions and 470 deletions.
10 changes: 6 additions & 4 deletions clippy_lints/src/as_conversions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_from_proc_macro;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
Expand Down Expand Up @@ -52,13 +52,15 @@ impl<'tcx> LateLintPass<'tcx> for AsConversions {
&& !in_external_macro(cx.sess(), expr.span)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
AS_CONVERSIONS,
expr.span,
"using a potentially dangerous silent `as` conversion",
None,
"consider using a safe wrapper for this conversion",
|diag| {
diag.help("consider using a safe wrapper for this conversion");
},
);
}
}
Expand Down
47 changes: 18 additions & 29 deletions clippy_lints/src/assertions_on_result_states.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn};
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::{has_debug_impl, is_copy, is_type_diagnostic_item};
Expand Down Expand Up @@ -68,39 +68,28 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates {
return;
}
}
let semicolon = if is_expr_final_block_expr(cx.tcx, e) { ";" } else { "" };
let mut app = Applicability::MachineApplicable;
match method_segment.ident.as_str() {
let (message, replacement) = match method_segment.ident.as_str() {
"is_ok" if type_suitable_to_unwrap(cx, args.type_at(1)) => {
span_lint_and_sugg(
cx,
ASSERTIONS_ON_RESULT_STATES,
macro_call.span,
"called `assert!` with `Result::is_ok`",
"replace with",
format!(
"{}.unwrap(){semicolon}",
snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
),
app,
);
("called `assert!` with `Result::is_ok`", "unwrap")
},
"is_err" if type_suitable_to_unwrap(cx, args.type_at(0)) => {
span_lint_and_sugg(
cx,
ASSERTIONS_ON_RESULT_STATES,
macro_call.span,
"called `assert!` with `Result::is_err`",
"replace with",
format!(
"{}.unwrap_err(){semicolon}",
snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
),
app,
);
("called `assert!` with `Result::is_err`", "unwrap_err")
},
_ => (),
_ => return,
};
span_lint_and_then(cx, ASSERTIONS_ON_RESULT_STATES, macro_call.span, message, |diag| {
let semicolon = if is_expr_final_block_expr(cx.tcx, e) { ";" } else { "" };
let mut app = Applicability::MachineApplicable;
diag.span_suggestion(
macro_call.span,
"replace with",
format!(
"{}.{replacement}(){semicolon}",
snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0
),
app,
);
});
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions clippy_lints/src/attrs/allow_attributes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::ALLOW_ATTRIBUTES;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_from_proc_macro;
use rustc_ast::{AttrStyle, Attribute};
use rustc_errors::Applicability;
Expand All @@ -13,14 +13,14 @@ pub fn check<'cx>(cx: &LateContext<'cx>, attr: &'cx Attribute) {
&& let Some(ident) = attr.ident()
&& !is_from_proc_macro(cx, attr)
{
span_lint_and_sugg(
cx,
ALLOW_ATTRIBUTES,
ident.span,
"#[allow] attribute found",
"replace it with",
"expect".into(),
Applicability::MachineApplicable,
);
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(cx, ALLOW_ATTRIBUTES, ident.span, "#[allow] attribute found", |diag| {
diag.span_suggestion(
ident.span,
"replace it with",
"expect",
Applicability::MachineApplicable,
);
});
}
}
10 changes: 6 additions & 4 deletions clippy_lints/src/attrs/allow_attributes_without_reason.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{Attribute, ALLOW_ATTRIBUTES_WITHOUT_REASON};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_from_proc_macro;
use rustc_ast::{MetaItemKind, NestedMetaItem};
use rustc_lint::{LateContext, LintContext};
Expand All @@ -21,12 +21,14 @@ pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMet
return;
}

span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
ALLOW_ATTRIBUTES_WITHOUT_REASON,
attr.span,
format!("`{}` attribute without specifying a reason", name.as_str()),
None,
"try adding a reason at the end with `, reason = \"..\"`",
|diag| {
diag.help("try adding a reason at the end with `, reason = \"..\"`");
},
);
}
37 changes: 20 additions & 17 deletions clippy_lints/src/casts/fn_to_numeric_cast_any.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
Expand All @@ -14,21 +14,24 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
_ => { /* continue to checks */ },
}

match cast_from.kind() {
ty::FnDef(..) | ty::FnPtr(_) => {
let mut applicability = Applicability::MaybeIncorrect;
let from_snippet = snippet_with_applicability(cx, cast_expr.span, "..", &mut applicability);
if let ty::FnDef(..) | ty::FnPtr(_) = cast_from.kind() {
let mut applicability = Applicability::MaybeIncorrect;
let from_snippet = snippet_with_applicability(cx, cast_expr.span, "..", &mut applicability);

span_lint_and_sugg(
cx,
FN_TO_NUMERIC_CAST_ANY,
expr.span,
format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
"did you mean to invoke the function?",
format!("{from_snippet}() as {cast_to}"),
applicability,
);
},
_ => {},
span_lint_and_then(
cx,
FN_TO_NUMERIC_CAST_ANY,
expr.span,
format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
|diag| {
diag.span_suggestion_with_style(
expr.span,
"did you mean to invoke the function?",
format!("{from_snippet}() as {cast_to}"),
applicability,
SuggestionStyle::ShowAlways,
);
},
);
}
}
24 changes: 17 additions & 7 deletions clippy_lints/src/create_dir.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use rustc_errors::Applicability;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
Expand Down Expand Up @@ -39,14 +39,24 @@ impl LateLintPass<'_> for CreateDir {
&& let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::fs_create_dir, def_id)
{
span_lint_and_sugg(
span_lint_and_then(
cx,
CREATE_DIR,
expr.span,
"calling `std::fs::create_dir` where there may be a better way",
"consider calling `std::fs::create_dir_all` instead",
format!("create_dir_all({})", snippet(cx, arg.span, "..")),
Applicability::MaybeIncorrect,
|diag| {
let mut app = Applicability::MaybeIncorrect;
diag.span_suggestion_with_style(
expr.span,
"consider calling `std::fs::create_dir_all` instead",
format!(
"create_dir_all({})",
snippet_with_applicability(cx, arg.span, "..", &mut app)
),
app,
SuggestionStyle::ShowAlways,
);
},
);
}
}
Expand Down
108 changes: 57 additions & 51 deletions clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_in_test;
use clippy_utils::macros::{macro_backtrace, MacroCall};
use clippy_utils::source::snippet_with_applicability;
Expand Down Expand Up @@ -65,61 +65,67 @@ impl LateLintPass<'_> for DbgMacro {
// allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
!(self.allow_dbg_in_tests && is_in_test(cx.tcx, expr.hir_id))
{
let mut applicability = Applicability::MachineApplicable;

let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
// dbg!()
ExprKind::Block(..) => {
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
// remove the whole statement.
if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
{
(macro_call.span.to(semi_span), String::new())
} else {
(macro_call.span, String::from("()"))
}
},
// dbg!(1)
ExprKind::Match(val, ..) => (
macro_call.span,
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
),
// dbg!(2, 3)
ExprKind::Tup(
[
Expr {
kind: ExprKind::Match(first, ..),
..
},
..,
Expr {
kind: ExprKind::Match(last, ..),
..
},
],
) => {
let snippet = snippet_with_applicability(
cx,
first.span.source_callsite().to(last.span.source_callsite()),
"..",
&mut applicability,
);
(macro_call.span, format!("({snippet})"))
},
_ => return,
};

self.prev_ctxt = cur_syntax_ctxt;

span_lint_and_sugg(
span_lint_and_then(
cx,
DBG_MACRO,
sugg_span,
macro_call.span,
"the `dbg!` macro is intended as a debugging tool",
"remove the invocation before committing it to a version control system",
suggestion,
applicability,
|diag| {
let mut applicability = Applicability::MachineApplicable;

let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
// dbg!()
ExprKind::Block(..) => {
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
// remove the whole statement.
if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
{
(macro_call.span.to(semi_span), String::new())
} else {
(macro_call.span, String::from("()"))
}
},
// dbg!(1)
ExprKind::Match(val, ..) => (
macro_call.span,
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
.to_string(),
),
// dbg!(2, 3)
ExprKind::Tup(
[
Expr {
kind: ExprKind::Match(first, ..),
..
},
..,
Expr {
kind: ExprKind::Match(last, ..),
..
},
],
) => {
let snippet = snippet_with_applicability(
cx,
first.span.source_callsite().to(last.span.source_callsite()),
"..",
&mut applicability,
);
(macro_call.span, format!("({snippet})"))
},
_ => unreachable!(),
};

diag.span_suggestion(
sugg_span,
"remove the invocation before committing it to a version control system",
suggestion,
applicability,
);
},
);
}
}
Expand Down
23 changes: 11 additions & 12 deletions clippy_lints/src/default_numeric_fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,27 +92,26 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> {
let (suffix, is_float) = match lit_ty.kind() {
ty::Int(IntTy::I32) => ("i32", false),
ty::Float(FloatTy::F64) => ("f64", true),
// Default numeric fallback never results in other types.
_ => return,
};

let src = if let Some(src) = snippet_opt(self.cx, lit.span) {
src
} else {
match lit.node {
LitKind::Int(src, _) => format!("{src}"),
LitKind::Float(src, _) => format!("{src}"),
_ => return,
}
};
let sugg = numeric_literal::format(&src, Some(suffix), is_float);
span_lint_hir_and_then(
self.cx,
DEFAULT_NUMERIC_FALLBACK,
emit_hir_id,
lit.span,
"default numeric fallback might occur",
|diag| {
let src = if let Some(src) = snippet_opt(self.cx, lit.span) {
src
} else {
match lit.node {
LitKind::Int(src, _) => format!("{src}"),
LitKind::Float(src, _) => format!("{src}"),
_ => unreachable!("Default numeric fallback never results in other types"),
}
};

let sugg = numeric_literal::format(&src, Some(suffix), is_float);
diag.span_suggestion(lit.span, "consider adding suffix", sugg, Applicability::MaybeIncorrect);
},
);
Expand Down
Loading

0 comments on commit c082bc2

Please sign in to comment.