Skip to content

Commit da0578a

Browse files
committed
borrow_deref_ref
1 parent a3bf9d1 commit da0578a

31 files changed

+279
-64
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2887,6 +2887,7 @@ Released 2018-09-13
28872887
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
28882888
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
28892889
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
2890+
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
28902891
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
28912892
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
28922893
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection

clippy_lints/src/borrow_deref_ref.rs

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

clippy_lints/src/checked_conversions.rs

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

clippy_lints/src/enum_variants.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ fn check_variant(
154154
}
155155
}
156156
let first = &def.variants[0].ident.name.as_str();
157-
let mut pre = &first[..str_utils::camel_case_until(&*first).byte_index];
158-
let mut post = &first[str_utils::camel_case_start(&*first).byte_index..];
157+
let mut pre = &first[..str_utils::camel_case_until(first).byte_index];
158+
let mut post = &first[str_utils::camel_case_start(first).byte_index..];
159159
for var in def.variants {
160160
let name = var.ident.name.as_str();
161161

clippy_lints/src/eval_order_dependence.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
121121
self.visit_expr(if_expr);
122122
}
123123
// make sure top level arm expressions aren't linted
124-
self.maybe_walk_expr(&*arm.body);
124+
self.maybe_walk_expr(arm.body);
125125
}
126126
},
127127
_ => walk_expr(self, e),

clippy_lints/src/format.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
5454
span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability);
5555
} else {
5656
if_chain! {
57-
if let [e] = &*format_args.format_string_parts;
57+
if let [e] = format_args.format_string_parts;
5858
if let ExprKind::Lit(lit) = &e.kind;
5959
if let Some(s_src) = snippet_opt(cx, lit.span);
6060
then {

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
@@ -21,6 +21,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
2121
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
2222
LintId::of(booleans::LOGIC_BUG),
2323
LintId::of(booleans::NONMINIMAL_BOOL),
24+
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
2425
LintId::of(casts::CAST_REF_TO_MUT),
2526
LintId::of(casts::CHAR_LIT_AS_U8),
2627
LintId::of(casts::FN_TO_NUMERIC_CAST),

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(casts::CHAR_LIT_AS_U8),
910
LintId::of(casts::UNNECESSARY_CAST),
1011
LintId::of(derivable_impls::DERIVABLE_IMPLS),

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ store.register_lints(&[
5959
bool_assert_comparison::BOOL_ASSERT_COMPARISON,
6060
booleans::LOGIC_BUG,
6161
booleans::NONMINIMAL_BOOL,
62+
borrow_deref_ref::BORROW_DEREF_REF,
6263
bytecount::NAIVE_BYTECOUNT,
6364
cargo_common_metadata::CARGO_COMMON_METADATA,
6465
case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ mod blacklisted_name;
175175
mod blocks_in_if_conditions;
176176
mod bool_assert_comparison;
177177
mod booleans;
178+
mod borrow_deref_ref;
178179
mod bytecount;
179180
mod cargo_common_metadata;
180181
mod case_sensitive_file_extension_comparisons;
@@ -603,6 +604,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
603604
store.register_late_pass(|| Box::new(mutex_atomic::Mutex));
604605
store.register_late_pass(|| Box::new(needless_update::NeedlessUpdate));
605606
store.register_late_pass(|| Box::new(needless_borrowed_ref::NeedlessBorrowedRef));
607+
store.register_late_pass(|| Box::new(borrow_deref_ref::BorrowDerefRef));
606608
store.register_late_pass(|| Box::new(no_effect::NoEffect));
607609
store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));
608610
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.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -884,8 +884,8 @@ fn check_match_bool(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
884884
let exprs = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
885885
if let ExprKind::Lit(ref lit) = arm_bool.kind {
886886
match lit.node {
887-
LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
888-
LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
887+
LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
888+
LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
889889
_ => None,
890890
}
891891
} else {

clippy_lints/src/mut_key.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for MutableKeyType {
105105
if let hir::PatKind::Wild = local.pat.kind {
106106
return;
107107
}
108-
check_ty(cx, local.span, cx.typeck_results().pat_ty(&*local.pat));
108+
check_ty(cx, local.span, cx.typeck_results().pat_ty(local.pat));
109109
}
110110
}
111111

clippy_lints/src/pass_by_ref_or_value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue {
233233
}
234234

235235
if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind {
236-
self.check_poly_fn(cx, item.def_id, &*method_sig.decl, None);
236+
self.check_poly_fn(cx, item.def_id, method_sig.decl, None);
237237
}
238238
}
239239

clippy_lints/src/redundant_clone.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,8 @@ fn is_call_with_ref_arg<'tcx>(
289289
if let mir::TerminatorKind::Call { func, args, destination, .. } = kind;
290290
if args.len() == 1;
291291
if let mir::Operand::Move(mir::Place { local, .. }) = &args[0];
292-
if let ty::FnDef(def_id, _) = *func.ty(&*mir, cx.tcx).kind();
293-
if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
292+
if let ty::FnDef(def_id, _) = *func.ty(mir, cx.tcx).kind();
293+
if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(mir, cx.tcx));
294294
if !is_copy(cx, inner_ty);
295295
then {
296296
Some((def_id, *local, inner_ty, destination.as_ref().map(|(dest, _)| dest)?.as_local()?))
@@ -319,7 +319,7 @@ fn find_stmt_assigns_to<'tcx>(
319319
None
320320
})?;
321321

322-
match (by_ref, &*rvalue) {
322+
match (by_ref, rvalue) {
323323
(true, mir::Rvalue::Ref(_, _, place)) | (false, mir::Rvalue::Use(mir::Operand::Copy(place))) => {
324324
Some(base_local_and_movability(cx, mir, *place))
325325
},

clippy_lints/src/redundant_static_lifetimes.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ impl RedundantStaticLifetimes {
5151
fn visit_type(&mut self, ty: &Ty, cx: &EarlyContext<'_>, reason: &str) {
5252
match ty.kind {
5353
// Be careful of nested structures (arrays and tuples)
54-
TyKind::Array(ref ty, _) => {
55-
self.visit_type(&*ty, cx, reason);
54+
TyKind::Array(ref ty, _) | TyKind::Slice(ref ty) => {
55+
self.visit_type(ty, cx, reason);
5656
},
5757
TyKind::Tup(ref tup) => {
5858
for tup_ty in tup {
59-
self.visit_type(&*tup_ty, cx, reason);
59+
self.visit_type(tup_ty, cx, reason);
6060
}
6161
},
6262
// This is what we are looking for !
@@ -89,9 +89,6 @@ impl RedundantStaticLifetimes {
8989
}
9090
self.visit_type(&*borrow_type.ty, cx, reason);
9191
},
92-
TyKind::Slice(ref ty) => {
93-
self.visit_type(ty, cx, reason);
94-
},
9592
_ => {},
9693
}
9794
}

clippy_utils/src/ast_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ pub fn extract_assert_macro_args(mut expr: &Expr) -> Option<[&Expr; 2]> {
705705
if let [a, b, ..] = tup.as_slice();
706706
if let (&ExprKind::AddrOf(_, _, ref a), &ExprKind::AddrOf(_, _, ref b)) = (&a.kind, &b.kind);
707707
then {
708-
return Some([&*a, &*b]);
708+
return Some([a, b]);
709709
}
710710
}
711711
None

clippy_utils/src/consts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
458458
fn ifthenelse(&mut self, cond: &Expr<'_>, then: &Expr<'_>, otherwise: Option<&Expr<'_>>) -> Option<Constant> {
459459
if let Some(Constant::Bool(b)) = self.expr(cond) {
460460
if b {
461-
self.expr(&*then)
461+
self.expr(then)
462462
} else {
463463
otherwise.as_ref().and_then(|expr| self.expr(expr))
464464
}

clippy_utils/src/higher.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'tcx> ForLoop<'tcx> {
3636
if let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind;
3737
if let hir::ExprKind::Call(_, [arg]) = iterexpr.kind;
3838
if let hir::ExprKind::Loop(block, ..) = arm.body.kind;
39-
if let [stmt] = &*block.stmts;
39+
if let [stmt] = block.stmts;
4040
if let hir::StmtKind::Expr(e) = stmt.kind;
4141
if let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind;
4242
if let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind;

clippy_utils/src/hir_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,10 @@ impl HirEqInterExpr<'_, '_, '_> {
393393
self.eq_ty(lt, rt) && self.eq_body(ll_id.body, rl_id.body)
394394
},
395395
(&TyKind::Ptr(ref l_mut), &TyKind::Ptr(ref r_mut)) => {
396-
l_mut.mutbl == r_mut.mutbl && self.eq_ty(&*l_mut.ty, &*r_mut.ty)
396+
l_mut.mutbl == r_mut.mutbl && self.eq_ty(l_mut.ty, r_mut.ty)
397397
},
398398
(&TyKind::Rptr(_, ref l_rmut), &TyKind::Rptr(_, ref r_rmut)) => {
399-
l_rmut.mutbl == r_rmut.mutbl && self.eq_ty(&*l_rmut.ty, &*r_rmut.ty)
399+
l_rmut.mutbl == r_rmut.mutbl && self.eq_ty(l_rmut.ty, r_rmut.ty)
400400
},
401401
(&TyKind::Path(ref l), &TyKind::Path(ref r)) => self.eq_qpath(l, r),
402402
(&TyKind::Tup(l), &TyKind::Tup(r)) => over(l, r, |l, r| self.eq_ty(l, r)),
@@ -590,7 +590,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
590590
self.hash_name(i.ident.name);
591591
}
592592
if let Some(j) = *j {
593-
self.hash_expr(&*j);
593+
self.hash_expr(j);
594594
}
595595
},
596596
ExprKind::Box(e) | ExprKind::DropTemps(e) | ExprKind::Yield(e, _) => {

clippy_utils/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
14401440
},
14411441
PatKind::Tuple(pats, _) => are_refutable(cx, pats),
14421442
PatKind::Struct(ref qpath, fields, _) => {
1443-
is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| &*field.pat))
1443+
is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| field.pat))
14441444
},
14451445
PatKind::TupleStruct(ref qpath, pats, _) => is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats),
14461446
PatKind::Slice(head, middle, tail) => {
@@ -1696,7 +1696,7 @@ pub fn if_sequence<'tcx>(mut expr: &'tcx Expr<'tcx>) -> (Vec<&'tcx Expr<'tcx>>,
16961696
let mut blocks: Vec<&Block<'_>> = Vec::new();
16971697

16981698
while let Some(higher::IfOrIfLet { cond, then, r#else }) = higher::IfOrIfLet::hir(expr) {
1699-
conds.push(&*cond);
1699+
conds.push(cond);
17001700
if let ExprKind::Block(block, _) = then.kind {
17011701
blocks.push(block);
17021702
} else {

0 commit comments

Comments
 (0)