Skip to content

Commit

Permalink
Auto merge of #13145 - xFrednet:07797-restriction-and-then-why, r=Jarcho
Browse files Browse the repository at this point in the history
Make restriction lint's use `span_lint_and_then` (q -> w)

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

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

---

cc: #7797

sister PR of: #13136

changelog: none
  • Loading branch information
bors committed Aug 6, 2024
2 parents 5f6d07b + 4bf4c47 commit cfb3881
Show file tree
Hide file tree
Showing 17 changed files with 244 additions and 168 deletions.
34 changes: 19 additions & 15 deletions clippy_lints/src/matches/match_wild_enum.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_sugg, span_lint_and_then};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_refutable, peel_hir_pat_refs, recurse_or_patterns};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -148,23 +148,27 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
Applicability::MaybeIncorrect,
),
variants => {
let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect();
let message = if adt_def.is_variant_list_non_exhaustive() || has_external_hidden {
suggestions.push("_".into());
"wildcard matches known variants and will also match future added variants"
let (message, add_wildcard) = if adt_def.is_variant_list_non_exhaustive() || has_external_hidden {
(
"wildcard matches known variants and will also match future added variants",
true,
)
} else {
"wildcard match will also match any future added variants"
("wildcard match will also match any future added variants", false)
};

span_lint_and_sugg(
cx,
WILDCARD_ENUM_MATCH_ARM,
wildcard_span,
message,
"try",
suggestions.join(" | "),
Applicability::MaybeIncorrect,
);
span_lint_and_then(cx, WILDCARD_ENUM_MATCH_ARM, wildcard_span, message, |diag| {
let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect();
if add_wildcard {
suggestions.push("_".into());
}
diag.span_suggestion(
wildcard_span,
"try",
suggestions.join(" | "),
Applicability::MaybeIncorrect,
);
});
},
};
}
Expand Down
10 changes: 6 additions & 4 deletions clippy_lints/src/matches/rest_pat_in_fully_bound_struct.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 rustc_hir::{Pat, PatKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty;
Expand All @@ -15,13 +15,15 @@ pub(crate) fn check(cx: &LateContext<'_>, pat: &Pat<'_>) {
&& fields.len() == def.non_enum_variant().fields.len()
&& !def.non_enum_variant().is_field_list_non_exhaustive()
{
span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
REST_PAT_IN_FULLY_BOUND_STRUCTS,
pat.span,
"unnecessary use of `..` pattern in struct binding. All fields were already bound",
None,
"consider removing `..` from this binding",
|diag| {
diag.help("consider removing `..` from this binding");
},
);
}
}
39 changes: 19 additions & 20 deletions clippy_lints/src/matches/try_err.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::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res};
Expand Down Expand Up @@ -48,29 +48,28 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine
return;
};

let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
let span = hygiene::walk_chain(err_arg.span, try_arg.span.ctxt());
let mut applicability = Applicability::MachineApplicable;
let origin_snippet = snippet_with_applicability(cx, span, "_", &mut applicability);
let ret_prefix = if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::Ret(_))) {
"" // already returns
} else {
"return "
};
let suggestion = if err_ty == expr_err_ty {
format!("{ret_prefix}{prefix}{origin_snippet}{suffix}")
} else {
format!("{ret_prefix}{prefix}{origin_snippet}.into(){suffix}")
};

span_lint_and_sugg(
span_lint_and_then(
cx,
TRY_ERR,
expr.span,
"returning an `Err(_)` with the `?` operator",
"try",
suggestion,
applicability,
|diag| {
let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
let span = hygiene::walk_chain(err_arg.span, try_arg.span.ctxt());
let mut applicability = Applicability::MachineApplicable;
let origin_snippet = snippet_with_applicability(cx, span, "_", &mut applicability);
let ret_prefix = if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::Ret(_))) {
"" // already returns
} else {
"return "
};
let suggestion = if err_ty == expr_err_ty {
format!("{ret_prefix}{prefix}{origin_snippet}{suffix}")
} else {
format!("{ret_prefix}{prefix}{origin_snippet}.into(){suffix}")
};
diag.span_suggestion(expr.span, "try", suggestion, applicability);
},
);
}
}
Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/methods/verbose_file_reads.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_trait_method;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir::{Expr, ExprKind, QPath};
Expand All @@ -23,6 +23,9 @@ pub(super) fn check<'tcx>(
&& matches!(recv.kind, ExprKind::Path(QPath::Resolved(None, _)))
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty_adjusted(recv).peel_refs(), sym::File)
{
span_lint_and_help(cx, VERBOSE_FILE_READS, expr.span, msg, None, help);
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(cx, VERBOSE_FILE_READS, expr.span, msg, |diag| {
diag.help(help);
});
}
}
30 changes: 21 additions & 9 deletions clippy_lints/src/misc_early/literal_suffix.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 rustc_errors::Applicability;
use rustc_lint::EarlyContext;
use rustc_span::Span;
Expand All @@ -12,24 +12,36 @@ pub(super) fn check(cx: &EarlyContext<'_>, lit_span: Span, lit_snip: &str, suffi
// Do not lint when literal is unsuffixed.
if !suffix.is_empty() {
if lit_snip.as_bytes()[maybe_last_sep_idx] == b'_' {
span_lint_and_sugg(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
SEPARATED_LITERAL_SUFFIX,
lit_span,
format!("{sugg_type} type suffix should not be separated by an underscore"),
"remove the underscore",
format!("{}{suffix}", &lit_snip[..maybe_last_sep_idx]),
Applicability::MachineApplicable,
|diag| {
diag.span_suggestion(
lit_span,
"remove the underscore",
format!("{}{suffix}", &lit_snip[..maybe_last_sep_idx]),
Applicability::MachineApplicable,
);
},
);
} else {
span_lint_and_sugg(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
UNSEPARATED_LITERAL_SUFFIX,
lit_span,
format!("{sugg_type} type suffix should be separated by an underscore"),
"add an underscore",
format!("{}_{suffix}", &lit_snip[..=maybe_last_sep_idx]),
Applicability::MachineApplicable,
|diag| {
diag.span_suggestion(
lit_span,
"add an underscore",
format!("{}_{suffix}", &lit_snip[..=maybe_last_sep_idx]),
Applicability::MachineApplicable,
);
},
);
}
}
Expand Down
21 changes: 12 additions & 9 deletions clippy_lints/src/misc_early/unneeded_field_pattern.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::source::snippet_opt;
use rustc_ast::ast::{Pat, PatKind};
use rustc_lint::EarlyContext;
Expand All @@ -21,13 +21,15 @@ pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) {
}
}
if !pfields.is_empty() && wilds == pfields.len() {
span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
UNNEEDED_FIELD_PATTERN,
pat.span,
"all the struct fields are matched to a wildcard pattern, consider using `..`",
None,
format!("try with `{type_name} {{ .. }}` instead"),
|diag| {
diag.help(format!("try with `{type_name} {{ .. }}` instead"));
},
);
return;
}
Expand Down Expand Up @@ -56,14 +58,15 @@ pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) {
}
}

span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
UNNEEDED_FIELD_PATTERN,
field.span,
"you matched a field with a wildcard pattern, consider using `..` \
instead",
None,
format!("try with `{type_name} {{ {}, .. }}`", normal[..].join(", ")),
"you matched a field with a wildcard pattern, consider using `..` instead",
|diag| {
diag.help(format!("try with `{type_name} {{ {}, .. }}`", normal[..].join(", ")));
},
);
}
}
Expand Down
10 changes: 6 additions & 4 deletions clippy_lints/src/question_mark_used.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::macros::span_is_local;
use rustc_hir::{Expr, ExprKind, MatchSource};
Expand Down Expand Up @@ -39,13 +39,15 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMarkUsed {
return;
}

span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
QUESTION_MARK_USED,
expr.span,
"question mark operator was used",
None,
"consider using a custom macro or match expression",
|diag| {
diag.help("consider using a custom macro or match expression");
},
);
}
}
Expand Down
14 changes: 5 additions & 9 deletions clippy_lints/src/ref_patterns.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 rustc_ast::ast::{BindingMode, Pat, PatKind};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::declare_lint_pass;
Expand Down Expand Up @@ -33,14 +33,10 @@ impl EarlyLintPass for RefPatterns {
if let PatKind::Ident(BindingMode::REF, _, _) = pat.kind
&& !pat.span.from_expansion()
{
span_lint_and_help(
cx,
REF_PATTERNS,
pat.span,
"usage of ref pattern",
None,
"consider using `&` for clarity instead",
);
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(cx, REF_PATTERNS, pat.span, "usage of ref pattern", |diag| {
diag.help("consider using `&` for clarity instead");
});
}
}
}
13 changes: 4 additions & 9 deletions clippy_lints/src/shadow.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use clippy_utils::visitors::is_local_used;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -194,14 +194,9 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span)
(SHADOW_UNRELATED, msg)
},
};
span_lint_and_note(
cx,
lint,
span,
msg,
Some(cx.tcx.hir().span(shadowed)),
"previous binding is here",
);
span_lint_and_then(cx, lint, span, msg, |diag| {
diag.span_note(cx.tcx.hir().span(shadowed), "previous binding is here");
});
}

/// Returns true if the expression is a simple transformation of a local binding such as `&x`
Expand Down
10 changes: 6 additions & 4 deletions clippy_lints/src/single_char_lifetime_names.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 rustc_ast::ast::{GenericParam, GenericParamKind};
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -48,13 +48,15 @@ impl EarlyLintPass for SingleCharLifetimeNames {

if let GenericParamKind::Lifetime = param.kind {
if !param.is_placeholder && param.ident.as_str().len() <= 2 {
span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
ctx,
SINGLE_CHAR_LIFETIME_NAMES,
param.ident.span,
"single-character lifetime names are likely uninformative",
None,
"use a more informative name",
|diag| {
diag.help("use a more informative name");
},
);
}
}
Expand Down
23 changes: 12 additions & 11 deletions clippy_lints/src/strings.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg};
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::ty::is_type_lang_item;
use clippy_utils::{
Expand Down Expand Up @@ -399,17 +399,16 @@ impl<'tcx> LateLintPass<'tcx> for StrToString {
&& let ty::Ref(_, ty, ..) = ty.kind()
&& ty.is_str()
{
let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_with_applicability(cx, self_arg.span, "..", &mut applicability);

span_lint_and_sugg(
span_lint_and_then(
cx,
STR_TO_STRING,
expr.span,
"`to_string()` called on a `&str`",
"try",
format!("{snippet}.to_owned()"),
applicability,
|diag| {
let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_with_applicability(cx, self_arg.span, "..", &mut applicability);
diag.span_suggestion(expr.span, "try", format!("{snippet}.to_owned()"), applicability);
},
);
}
}
Expand Down Expand Up @@ -455,13 +454,15 @@ impl<'tcx> LateLintPass<'tcx> for StringToString {
&& let ty = cx.typeck_results().expr_ty(self_arg)
&& is_type_lang_item(cx, ty, LangItem::String)
{
span_lint_and_help(
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
span_lint_and_then(
cx,
STRING_TO_STRING,
expr.span,
"`to_string()` called on a `String`",
None,
"consider using `.clone()`",
|diag| {
diag.help("consider using `.clone()`");
},
);
}
}
Expand Down
Loading

0 comments on commit cfb3881

Please sign in to comment.