Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit c4c413b

Browse files
borsbors
committed
Auto merge of rust-lang#7930 - lengyijun:needless_deref_new, r=Jarcho
new lint: `borrow_deref_ref` changelog: ``[`borrow_deref_ref`]`` Related pr: rust-lang#6837 rust-lang#7577 `@Jarcho` Could you please give a review? `cargo lintcheck` gives no false negative (but tested crates are out-of-date). TODO: 1. Not sure the name. `deref_on_immutable_ref` or some others?
2 parents f0bf200 + 202fdb9 commit c4c413b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+399
-94
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3297,6 +3297,7 @@ Released 2018-09-13
32973297
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
32983298
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
32993299
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
3300+
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
33003301
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
33013302
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
33023303
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection

‎clippy_lints/src/borrow_deref_ref.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
use crate::reference::DEREF_ADDROF;
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::source::snippet_opt;
4+
use clippy_utils::ty::implements_trait;
5+
use clippy_utils::{get_parent_expr, is_lint_allowed};
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{ExprKind, UnOp};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_middle::mir::Mutability;
10+
use rustc_middle::ty;
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks for `&*(&T)`.
16+
///
17+
/// ### Why is this bad?
18+
/// Dereferencing and then borrowing a reference value has no effect in most cases.
19+
///
20+
/// ### Known problems
21+
/// false negative on such code:
22+
/// ```
23+
/// let x = &12;
24+
/// let addr_x = &x as *const _ as usize;
25+
/// let addr_y = &&*x as *const _ as usize; // assert ok now, and lint triggerd.
26+
/// // But if we fix it, assert will fail.
27+
/// assert_ne!(addr_x, addr_y);
28+
/// ```
29+
///
30+
/// ### Example
31+
/// ```rust
32+
/// let s = &String::new();
33+
///
34+
/// // Bad
35+
/// let a: &String = &* s;
36+
/// foo(&*s);
37+
///
38+
/// // Good
39+
/// let a: &String = s;
40+
/// foo(&**s);
41+
///
42+
/// fn foo(_: &str){ }
43+
/// ```
44+
#[clippy::version = "1.59.0"]
45+
pub BORROW_DEREF_REF,
46+
complexity,
47+
"deref on an immutable reference returns the same type as itself"
48+
}
49+
50+
declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]);
51+
52+
impl LateLintPass<'_> for BorrowDerefRef {
53+
fn check_expr(&mut self, cx: &LateContext<'_>, e: &rustc_hir::Expr<'_>) {
54+
if_chain! {
55+
if !e.span.from_expansion();
56+
if let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind;
57+
if !addrof_target.span.from_expansion();
58+
if let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind;
59+
if !deref_target.span.from_expansion();
60+
if !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..) );
61+
let ref_ty = cx.typeck_results().expr_ty(deref_target);
62+
if let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind();
63+
then{
64+
65+
if let Some(parent_expr) = get_parent_expr(cx, e){
66+
if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..)) &&
67+
!is_lint_allowed(cx, DEREF_ADDROF, parent_expr.hir_id) {
68+
return;
69+
}
70+
71+
// modification to `&mut &*x` is different from `&mut x`
72+
if matches!(deref_target.kind, ExprKind::Path(..)
73+
| ExprKind::Field(..)
74+
| ExprKind::Index(..)
75+
| ExprKind::Unary(UnOp::Deref, ..))
76+
&& matches!(parent_expr.kind, ExprKind::AddrOf(_, Mutability::Mut, _)) {
77+
return;
78+
}
79+
}
80+
81+
span_lint_and_then(
82+
cx,
83+
BORROW_DEREF_REF,
84+
e.span,
85+
"deref on an immutable reference",
86+
|diag| {
87+
diag.span_suggestion(
88+
e.span,
89+
"if you would like to reborrow, try removing `&*`",
90+
snippet_opt(cx, deref_target.span).unwrap(),
91+
Applicability::MachineApplicable
92+
);
93+
94+
// has deref trait -> give 2 help
95+
// doesn't have deref trait -> give 1 help
96+
if let Some(deref_trait_id) = cx.tcx.lang_items().deref_trait(){
97+
if !implements_trait(cx, *inner_ty, deref_trait_id, &[]) {
98+
return;
99+
}
100+
}
101+
102+
diag.span_suggestion(
103+
e.span,
104+
"if you would like to deref, try using `&**`",
105+
format!(
106+
"&**{}",
107+
&snippet_opt(cx, deref_target.span).unwrap(),
108+
),
109+
Applicability::MaybeIncorrect
110+
);
111+
112+
}
113+
);
114+
115+
}
116+
}
117+
}
118+
}

‎clippy_lints/src/checked_conversions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
318318
if let QPath::TypeRelative(ty, path) = &path;
319319
if path.ident.name.as_str() == function;
320320
if let TyKind::Path(QPath::Resolved(None, tp)) = &ty.kind;
321-
if let [int] = &*tp.segments;
321+
if let [int] = tp.segments;
322322
then {
323323
let name = int.ident.name.as_str();
324324
candidates.iter().find(|c| &name == *c).copied()
@@ -332,7 +332,7 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
332332
fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> {
333333
if_chain! {
334334
if let QPath::Resolved(_, path) = *path;
335-
if let [ty] = &*path.segments;
335+
if let [ty] = path.segments;
336336
then {
337337
let name = ty.ident.name.as_str();
338338
INTS.iter().find(|c| &name == *c).copied()

‎clippy_lints/src/let_if_seq.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
6868
if let hir::ExprKind::If(hir::Expr { kind: hir::ExprKind::DropTemps(cond), ..}, then, else_) = if_.kind;
6969
if !is_local_used(cx, *cond, canonical_id);
7070
if let hir::ExprKind::Block(then, _) = then.kind;
71-
if let Some(value) = check_assign(cx, canonical_id, &*then);
71+
if let Some(value) = check_assign(cx, canonical_id, then);
7272
if !is_local_used(cx, value, canonical_id);
7373
then {
7474
let span = stmt.span.to(if_.span);

‎clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
2525
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
2626
LintId::of(booleans::LOGIC_BUG),
2727
LintId::of(booleans::NONMINIMAL_BOOL),
28+
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
2829
LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN),
2930
LintId::of(casts::CAST_ABS_TO_UNSIGNED),
3031
LintId::of(casts::CAST_ENUM_CONSTRUCTOR),

‎clippy_lints/src/lib.register_complexity.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec![
66
LintId::of(attrs::DEPRECATED_CFG_ATTR),
77
LintId::of(booleans::NONMINIMAL_BOOL),
8+
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
89
LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN),
910
LintId::of(casts::CHAR_LIT_AS_U8),
1011
LintId::of(casts::UNNECESSARY_CAST),

‎clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ store.register_lints(&[
6565
booleans::LOGIC_BUG,
6666
booleans::NONMINIMAL_BOOL,
6767
borrow_as_ptr::BORROW_AS_PTR,
68+
borrow_deref_ref::BORROW_DEREF_REF,
6869
bytecount::NAIVE_BYTECOUNT,
6970
bytes_count_to_len::BYTES_COUNT_TO_LEN,
7071
cargo::CARGO_COMMON_METADATA,

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ mod blocks_in_if_conditions;
184184
mod bool_assert_comparison;
185185
mod booleans;
186186
mod borrow_as_ptr;
187+
mod borrow_deref_ref;
187188
mod bytecount;
188189
mod bytes_count_to_len;
189190
mod cargo;
@@ -640,6 +641,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
640641
store.register_late_pass(|| Box::new(mutex_atomic::Mutex));
641642
store.register_late_pass(|| Box::new(needless_update::NeedlessUpdate));
642643
store.register_late_pass(|| Box::new(needless_borrowed_ref::NeedlessBorrowedRef));
644+
store.register_late_pass(|| Box::new(borrow_deref_ref::BorrowDerefRef));
643645
store.register_late_pass(|| Box::new(no_effect::NoEffect));
644646
store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));
645647
store.register_late_pass(|| Box::new(transmute::Transmute));

‎clippy_lints/src/loops/never_loop.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
146146
if arms.is_empty() {
147147
e
148148
} else {
149-
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| &*a.body), main_loop_id);
149+
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| a.body), main_loop_id);
150150
combine_seq(e, arms)
151151
}
152152
},

‎clippy_lints/src/matches/match_bool.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
2424
let exprs = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
2525
if let ExprKind::Lit(ref lit) = arm_bool.kind {
2626
match lit.node {
27-
LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
28-
LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
27+
LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
28+
LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
2929
_ => None,
3030
}
3131
} else {

0 commit comments

Comments
 (0)