Skip to content

fix: unnecessary_cast suggests extra brackets when in macro #14643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2025
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
55 changes: 42 additions & 13 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Expr, ExprKind, Lit, Node, Path, QPath, TyKind, UnOp};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::adjustment::Adjust;
use rustc_middle::ty::{self, FloatTy, InferTy, Ty};
use rustc_span::{Symbol, sym};
use std::ops::ControlFlow;

use super::UNNECESSARY_CAST;
Expand Down Expand Up @@ -142,6 +144,33 @@ pub(super) fn check<'tcx>(
}

if cast_from.kind() == cast_to.kind() && !expr.span.in_external_macro(cx.sess().source_map()) {
enum MaybeParenOrBlock {
Paren,
Block,
Nothing,
}

fn is_borrow_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::AddrOf(..))
|| cx
.typeck_results()
.expr_adjustments(expr)
.first()
.is_some_and(|adj| matches!(adj.kind, Adjust::Borrow(_)))
}

fn is_in_allowed_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
const ALLOWED_MACROS: &[Symbol] = &[
sym::format_args_macro,
sym::assert_eq_macro,
sym::debug_assert_eq_macro,
sym::assert_ne_macro,
sym::debug_assert_ne_macro,
];
matches!(expr.span.ctxt().outer_expn_data().macro_def_id, Some(def_id) if
cx.tcx.get_diagnostic_name(def_id).is_some_and(|sym| ALLOWED_MACROS.contains(&sym)))
}

if let Some(id) = path_to_local(cast_expr)
&& !cx.tcx.hir_span(id).eq_ctxt(cast_expr.span)
{
Expand All @@ -150,26 +179,26 @@ pub(super) fn check<'tcx>(
return false;
}

// If the whole cast expression is a unary expression (`(*x as T)`) or an addressof
// expression (`(&x as T)`), then not surrounding the suggestion into a block risks us
// changing the precedence of operators if the cast expression is followed by an operation
// with higher precedence than the unary operator (`(*x as T).foo()` would become
// `*x.foo()`, which changes what the `*` applies on).
// The same is true if the expression encompassing the cast expression is a unary
// expression or an addressof expression.
let needs_block = matches!(cast_expr.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..))
|| get_parent_expr(cx, expr).is_some_and(|e| matches!(e.kind, ExprKind::Unary(..) | ExprKind::AddrOf(..)));
// Changing `&(x as i32)` to `&x` would change the meaning of the code because the previous creates
// a reference to the temporary while the latter creates a reference to the original value.
let surrounding = match cx.tcx.parent_hir_node(expr.hir_id) {
Node::Expr(parent) if is_borrow_expr(cx, parent) && !is_in_allowed_macro(cx, parent) => {
MaybeParenOrBlock::Block
},
Node::Expr(parent) if cast_expr.precedence() < parent.precedence() => MaybeParenOrBlock::Paren,
_ => MaybeParenOrBlock::Nothing,
};

span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"),
"try",
if needs_block {
format!("{{ {cast_str} }}")
} else {
cast_str
match surrounding {
MaybeParenOrBlock::Paren => format!("({cast_str})"),
MaybeParenOrBlock::Block => format!("{{ {cast_str} }}"),
MaybeParenOrBlock::Nothing => cast_str,
},
Applicability::MachineApplicable,
);
Expand Down
16 changes: 15 additions & 1 deletion tests/ui/unnecessary_cast.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,21 @@ mod fixable {
// Issue #11968: The suggestion for this lint removes the parentheses and leave the code as
// `*x.pow(2)` which tries to dereference the return value rather than `x`.
fn issue_11968(x: &usize) -> usize {
{ *x }.pow(2)
(*x).pow(2)
//~^ unnecessary_cast
}

#[allow(clippy::cast_lossless)]
fn issue_14640() {
let x = 5usize;
let vec: Vec<u64> = vec![1, 2, 3, 4, 5];
assert_eq!(vec.len(), x);
//~^ unnecessary_cast

let _ = (5i32 as i64).abs();
//~^ unnecessary_cast

let _ = 5i32 as i64;
//~^ unnecessary_cast
}
}
14 changes: 14 additions & 0 deletions tests/ui/unnecessary_cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,4 +269,18 @@ mod fixable {
(*x as usize).pow(2)
//~^ unnecessary_cast
}

#[allow(clippy::cast_lossless)]
fn issue_14640() {
let x = 5usize;
let vec: Vec<u64> = vec![1, 2, 3, 4, 5];
assert_eq!(vec.len(), x as usize);
//~^ unnecessary_cast

let _ = (5i32 as i64 as i64).abs();
//~^ unnecessary_cast

let _ = 5i32 as i64 as i64;
//~^ unnecessary_cast
}
}
22 changes: 20 additions & 2 deletions tests/ui/unnecessary_cast.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,25 @@ error: casting to the same type is unnecessary (`usize` -> `usize`)
--> tests/ui/unnecessary_cast.rs:269:9
|
LL | (*x as usize).pow(2)
| ^^^^^^^^^^^^^ help: try: `{ *x }`
| ^^^^^^^^^^^^^ help: try: `(*x)`

error: aborting due to 41 previous errors
error: casting to the same type is unnecessary (`usize` -> `usize`)
--> tests/ui/unnecessary_cast.rs:277:31
|
LL | assert_eq!(vec.len(), x as usize);
| ^^^^^^^^^^ help: try: `x`

error: casting to the same type is unnecessary (`i64` -> `i64`)
--> tests/ui/unnecessary_cast.rs:280:17
|
LL | let _ = (5i32 as i64 as i64).abs();
| ^^^^^^^^^^^^^^^^^^^^ help: try: `(5i32 as i64)`

error: casting to the same type is unnecessary (`i64` -> `i64`)
--> tests/ui/unnecessary_cast.rs:283:17
|
LL | let _ = 5i32 as i64 as i64;
| ^^^^^^^^^^^^^^^^^^ help: try: `5i32 as i64`

error: aborting due to 44 previous errors