Skip to content

Commit

Permalink
Auto merge of #12453 - y21:span_lint_into_diag, r=blyxyas
Browse files Browse the repository at this point in the history
accept `String` in `span_lint*` functions directly to avoid unnecessary clones

context: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Accepting.20.60Into.3C.7BSub.7DdiagMessage.3E.60.20in.20.60span_lint*.60.20functions/near/425703273

tldr: the `span_lint*` functions now accept both `String`s, which are then reused and not recloned like before, and also `&'static str`, in which case it [doesn't need to allocate](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_error_messages/lib.rs.html#359).
Previously, it accepted `&str` and would always call `.to_string()`, which is worse in any case: it allocates for `&'static str` and forces a clone even if the caller already has a `String`.

---------

This PR has a massive diff, but the only interesting change is in the first commit, which changes the message/help/note parameter in the `span_lint*` functions to not take a `&str`, but an `impl Into<DiagMessage>`.

The second commit changes all of the errors that now occur:
- `&format!(...)` cannot be passed to `span_lint` anymore. Instead, we now simply pass `format!()` directly.
- `Into<DiagMessage>` can be `&'static str`, but not any `&str`. So this requires changing a bunch of other `&str` to `&'static str` at call sites as well.
- Added [`Sugg::into_string`](https://github.com/y21/rust-clippy/blob/9fc88bc2851fbb287d89f65b78fb67af504f8362/clippy_utils/src/sugg.rs#L362), which, as opposed to `Sugg::to_string`, can take advantage of the fact that it takes ownership and is able to reuse the `String`

changelog: none
  • Loading branch information
bors committed Apr 1, 2024
2 parents cb87a57 + a6881b0 commit f678d26
Show file tree
Hide file tree
Showing 235 changed files with 406 additions and 398 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl ApproxConstant {
cx,
APPROX_CONSTANT,
e.span,
&format!("approximate value of `{module}::consts::{}` found", &name),
format!("approximate value of `{module}::consts::{}` found", &name),
None,
"consider using the constant directly",
);
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/asm_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ fn check_asm_syntax(
cx,
lint,
span,
&format!("{style} x86 assembly syntax used"),
format!("{style} x86 assembly syntax used"),
None,
&format!("use {} x86 assembly syntax", !style),
format!("use {} x86 assembly syntax", !style),
);
}
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants {
cx,
ASSERTIONS_ON_CONSTANTS,
macro_call.span,
&format!(
format!(
"`{}!(true)` will be optimized out by the compiler",
cx.tcx.item_name(macro_call.def_id)
),
Expand All @@ -74,9 +74,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants {
cx,
ASSERTIONS_ON_CONSTANTS,
macro_call.span,
&format!("`assert!(false{assert_arg})` should probably be replaced"),
format!("`assert!(false{assert_arg})` should probably be replaced"),
None,
&format!("use `panic!({panic_arg})` or `unreachable!({panic_arg})`"),
format!("use `panic!({panic_arg})` or `unreachable!({panic_arg})`"),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs/allow_attributes_without_reason.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMet
cx,
ALLOW_ATTRIBUTES_WITHOUT_REASON,
attr.span,
&format!("`{}` attribute without specifying a reason", name.as_str()),
format!("`{}` attribute without specifying a reason", name.as_str()),
None,
"try adding a reason at the end with `, reason = \"..\"`",
);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs/inline_always.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub(super) fn check(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Att
cx,
INLINE_ALWAYS,
attr.span,
&format!("you have declared `#[inline(always)]` on `{name}`. This is usually a bad idea"),
format!("you have declared `#[inline(always)]` on `{name}`. This is usually a bad idea"),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs/maybe_misused_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn check_nested_misused_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
cx,
MAYBE_MISUSED_CFG,
meta.span,
&format!("'test' may be misspelled as '{}'", ident.name.as_str()),
format!("'test' may be misspelled as '{}'", ident.name.as_str()),
"did you mean",
"test".to_string(),
Applicability::MaybeIncorrect,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs/unnecessary_clippy_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub(super) fn check(
clippy_lints,
"no need to put clippy lints behind a `clippy` cfg",
None,
&format!(
format!(
"write instead: `#{}[{}({})]`",
if attr.style == AttrStyle::Inner { "!" } else { "" },
ident.name,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/await_holding_invalid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedPa
cx,
AWAIT_HOLDING_INVALID_TYPE,
span,
&format!(
format!(
"`{}` may not be held across an `await` point per `clippy.toml`",
disallowed.path()
),
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/blocks_in_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
else {
return;
};
let complex_block_message = &format!(
let complex_block_message = format!(
"in {desc}, avoid complex blocks or closures with blocks; \
instead, move the block or closure higher and bind it with a `let`",
);
Expand Down Expand Up @@ -141,7 +141,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
let ex = &body.value;
if let ExprKind::Block(block, _) = ex.kind {
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
span_lint(cx, BLOCKS_IN_CONDITIONS, ex.span, complex_block_message);
span_lint(cx, BLOCKS_IN_CONDITIONS, ex.span, complex_block_message.clone());
return ControlFlow::Continue(Descend::No);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/bool_assert_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
cx,
BOOL_ASSERT_COMPARISON,
macro_call.span,
&format!("used `{macro_name}!` with a literal bool"),
format!("used `{macro_name}!` with a literal bool"),
|diag| {
// assert_eq!(...)
// ^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cargo/common_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata, ignore_publish: b

fn missing_warning(cx: &LateContext<'_>, package: &cargo_metadata::Package, field: &str) {
let message = format!("package `{}` is missing `{field}` metadata", package.name);
span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, &message);
span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message);
}

fn is_empty_str<T: AsRef<std::ffi::OsStr>>(value: &Option<T>) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/cargo/feature_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ fn lint(cx: &LateContext<'_>, feature: &str, substring: &str, is_prefix: bool) {
REDUNDANT_FEATURE_NAMES
},
DUMMY_SP,
&format!(
format!(
"the \"{substring}\" {} in the feature name \"{feature}\" is {}",
if is_prefix { "prefix" } else { "suffix" },
if is_negative { "negative" } else { "redundant" }
),
None,
&format!(
format!(
"consider renaming the feature to \"{}\"{}",
if is_prefix {
feature.strip_prefix(substring)
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cargo/lint_groups_priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ fn check_table(cx: &LateContext<'_>, table: LintTable, groups: &FxHashSet<&str>,
cx,
LINT_GROUPS_PRIORITY,
toml_span(group.span(), file),
&format!(
format!(
"lint group `{}` has the same priority ({priority}) as a lint",
group.as_ref()
),
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/cargo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl LateLintPass<'_> for Cargo {
},
Err(e) => {
for lint in NO_DEPS_LINTS {
span_lint(cx, lint, DUMMY_SP, &format!("could not read cargo metadata: {e}"));
span_lint(cx, lint, DUMMY_SP, format!("could not read cargo metadata: {e}"));
}
},
}
Expand All @@ -257,7 +257,7 @@ impl LateLintPass<'_> for Cargo {
},
Err(e) => {
for lint in WITH_DEPS_LINTS {
span_lint(cx, lint, DUMMY_SP, &format!("could not read cargo metadata: {e}"));
span_lint(cx, lint, DUMMY_SP, format!("could not read cargo metadata: {e}"));
}
},
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cargo/multiple_crate_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata, allowed_duplicate
cx,
MULTIPLE_CRATE_VERSIONS,
DUMMY_SP,
&format!("multiple versions for dependency `{name}`: {versions}"),
format!("multiple versions for dependency `{name}`: {versions}"),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cargo/wildcard_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata) {
cx,
WILDCARD_DEPENDENCIES,
DUMMY_SP,
&format!("wildcard dependency for `{}`", dep.name),
format!("wildcard dependency for `{}`", dep.name),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/as_ptr_cast_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
AS_PTR_CAST_MUT,
expr.span,
&format!("casting the result of `as_ptr` to *mut {ptrty}"),
format!("casting the result of `as_ptr` to *mut {ptrty}"),
"replace with",
format!("{recv}.as_mut_ptr()"),
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_abs_to_unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub(super) fn check(
cx,
CAST_ABS_TO_UNSIGNED,
span,
&format!("casting the result of `{cast_from}::abs()` to {cast_to}"),
format!("casting the result of `{cast_from}::abs()` to {cast_to}"),
"replace with",
format!("{}.unsigned_abs()", Sugg::hir(cx, receiver, "..").maybe_par()),
Applicability::MachineApplicable,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_lossless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(super) fn check(
cx,
CAST_LOSSLESS,
expr.span,
&message,
message,
"try",
format!("{cast_to_fmt}::from({sugg})"),
app,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_nan_to_int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
CAST_NAN_TO_INT,
expr.span,
&format!("casting a known NaN to {to_ty}"),
format!("casting a known NaN to {to_ty}"),
None,
"this always evaluates to 0",
);
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub(super) fn check(
cx,
CAST_ENUM_TRUNCATION,
expr.span,
&format!(
format!(
"casting `{cast_from}::{}` to `{cast_to}` will truncate the value{suffix}",
variant.name,
),
Expand All @@ -163,7 +163,7 @@ pub(super) fn check(
_ => return,
};

span_lint_and_then(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg, |diag| {
span_lint_and_then(cx, CAST_POSSIBLE_TRUNCATION, expr.span, msg, |diag| {
diag.help("if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...");
if !cast_from.is_floating_point() {
offer_suggestion(cx, expr, cast_expr, cast_to_span, diag);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_possible_wrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
),
};

span_lint_and_then(cx, CAST_POSSIBLE_WRAP, expr.span, &message, |diag| {
span_lint_and_then(cx, CAST_POSSIBLE_WRAP, expr.span, message, |diag| {
if let EmitState::LintOnPtrSize(16) = should_lint {
diag
.note("`usize` and `isize` may be as small as 16 bits on some platforms")
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_precision_loss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
cx,
CAST_PRECISION_LOSS,
expr.span,
&format!(
format!(
"casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \
but `{1}`'s mantissa is only {4} bits wide)",
cast_from,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_ptr_alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f
cx,
CAST_PTR_ALIGNMENT,
expr.span,
&format!(
format!(
"casting from `{cast_from}` to a more-strictly-aligned pointer (`{cast_to}`) ({} < {} bytes)",
from_layout.align.abi.bytes(),
to_layout.align.abi.bytes(),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_sign_loss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(super) fn check<'cx>(
cx,
CAST_SIGN_LOSS,
expr.span,
&format!("casting `{cast_from}` to `{cast_to}` may lose the sign of the value"),
format!("casting `{cast_from}` to `{cast_to}` may lose the sign of the value"),
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_slice_different_sizes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, msrv: &Msrv
cx,
CAST_SLICE_DIFFERENT_SIZES,
expr.span,
&format!(
format!(
"casting between raw pointers to `[{}]` (element size {from_size}) and `[{}]` (element size {to_size}) does not adjust the count",
start_ty.ty, end_ty.ty,
),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_slice_from_raw_parts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
CAST_SLICE_FROM_RAW_PARTS,
span,
&format!("casting the result of `{func}` to {cast_to}"),
format!("casting the result of `{func}` to {cast_to}"),
"replace with",
format!("core::ptr::slice_{func}({ptr}, {len})"),
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/fn_to_numeric_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
FN_TO_NUMERIC_CAST,
expr.span,
&format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
"try",
format!("{from_snippet} as usize"),
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/fn_to_numeric_cast_any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
FN_TO_NUMERIC_CAST_ANY,
expr.span,
&format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
"did you mean to invoke the function?",
format!("{from_snippet}() as {cast_to}"),
applicability,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
cx,
FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
expr.span,
&format!("casting function pointer `{from_snippet}` to `{cast_to}`, which truncates the value"),
format!("casting function pointer `{from_snippet}` to `{cast_to}`, which truncates the value"),
"try",
format!("{from_snippet} as usize"),
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/ptr_cast_constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(super) fn check<'tcx>(
PTR_CAST_CONSTNESS,
expr.span,
"`as` casting between raw pointers while changing only its constness",
&format!("try `pointer::cast_{constness}`, a safer alternative"),
format!("try `pointer::cast_{constness}`, a safer alternative"),
format!("{}.cast_{constness}()", sugg.maybe_par()),
Applicability::MachineApplicable,
);
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(super) fn check<'tcx>(
cx,
UNNECESSARY_CAST,
expr.span,
&format!(
format!(
"casting raw pointers to the same type and constness is unnecessary (`{cast_from}` -> `{cast_to}`)"
),
"try",
Expand Down Expand Up @@ -166,7 +166,7 @@ pub(super) fn check<'tcx>(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
"try",
if needs_block {
format!("{{ {cast_str} }}")
Expand Down Expand Up @@ -209,7 +209,7 @@ fn lint_unnecessary_cast(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting {literal_kind_name} literal to `{cast_to}` is unnecessary"),
format!("casting {literal_kind_name} literal to `{cast_to}` is unnecessary"),
"try",
sugg,
Applicability::MachineApplicable,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl CognitiveComplexity {
cx,
COGNITIVE_COMPLEXITY,
fn_span,
&format!(
format!(
"the function has a cognitive complexity of ({cc}/{})",
self.limit.limit()
),
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
cx,
DEFAULT_TRAIT_ACCESS,
expr.span,
&format!("calling `{replacement}` is more clear than this expression"),
format!("calling `{replacement}` is more clear than this expression"),
"try",
replacement,
Applicability::Unspecified, // First resolve the TODO above
Expand Down Expand Up @@ -243,7 +243,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
first_assign.unwrap().span,
"field assignment outside of initializer for an instance created with Default::default()",
Some(local.span),
&format!("consider initializing the variable with `{sugg}` and removing relevant reassignments"),
format!("consider initializing the variable with `{sugg}` and removing relevant reassignments"),
);
self.reassigned_linted.insert(span);
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/default_instead_of_iter_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for DefaultIterEmpty {
cx,
DEFAULT_INSTEAD_OF_ITER_EMPTY,
expr.span,
&format!("`{path}()` is the more idiomatic way"),
format!("`{path}()` is the more idiomatic way"),
"try",
sugg,
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/default_union_representation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for DefaultUnionRepresentation {
item.span,
"this union has the default representation",
None,
&format!(
format!(
"consider annotating `{}` with `#[repr(C)]` to explicitly specify memory layout",
cx.tcx.def_path_str(item.owner_id)
),
Expand Down
Loading

0 comments on commit f678d26

Please sign in to comment.