Skip to content

new lint: borrow_deref_ref #7930

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 3 commits into from
Jun 1, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3296,6 +3296,7 @@ Released 2018-09-13
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
[`borrow_deref_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_deref_ref
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
Expand Down
118 changes: 118 additions & 0 deletions clippy_lints/src/borrow_deref_ref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use crate::reference::DEREF_ADDROF;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::implements_trait;
use clippy_utils::{get_parent_expr, is_lint_allowed};
use rustc_errors::Applicability;
use rustc_hir::{ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::Mutability;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for `&*(&T)`.
///
/// ### Why is this bad?
/// Dereferencing and then borrowing a reference value has no effect in most cases.
///
/// ### Known problems
/// false negative on such code:
/// ```
/// let x = &12;
/// let addr_x = &x as *const _ as usize;
/// let addr_y = &&*x as *const _ as usize; // assert ok now, and lint triggerd.
/// // But if we fix it, assert will fail.
/// assert_ne!(addr_x, addr_y);
/// ```
///
/// ### Example
/// ```rust
/// let s = &String::new();
///
/// // Bad
/// let a: &String = &* s;
/// foo(&*s);
///
/// // Good
/// let a: &String = s;
/// foo(&**s);
///
/// fn foo(_: &str){ }
/// ```
#[clippy::version = "1.59.0"]
pub BORROW_DEREF_REF,
complexity,
"deref on an immutable reference returns the same type as itself"
}

declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]);

impl LateLintPass<'_> for BorrowDerefRef {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &rustc_hir::Expr<'_>) {
if_chain! {
if !e.span.from_expansion();
if let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind;
if !addrof_target.span.from_expansion();
if let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind;
if !deref_target.span.from_expansion();
if !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..) );
let ref_ty = cx.typeck_results().expr_ty(deref_target);
if let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind();
then{

if let Some(parent_expr) = get_parent_expr(cx, e){
if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..)) &&
!is_lint_allowed(cx, DEREF_ADDROF, parent_expr.hir_id) {
return;
}

// modification to `&mut &*x` is different from `&mut x`
if matches!(deref_target.kind, ExprKind::Path(..)
| ExprKind::Field(..)
| ExprKind::Index(..)
| ExprKind::Unary(UnOp::Deref, ..))
&& matches!(parent_expr.kind, ExprKind::AddrOf(_, Mutability::Mut, _)) {
return;
}
}

span_lint_and_then(
cx,
BORROW_DEREF_REF,
e.span,
"deref on an immutable reference",
|diag| {
diag.span_suggestion(
e.span,
"if you would like to reborrow, try removing `&*`",
snippet_opt(cx, deref_target.span).unwrap(),
Applicability::MachineApplicable
);

// has deref trait -> give 2 help
// doesn't have deref trait -> give 1 help
if let Some(deref_trait_id) = cx.tcx.lang_items().deref_trait(){
if !implements_trait(cx, *inner_ty, deref_trait_id, &[]) {
return;
}
}

diag.span_suggestion(
e.span,
"if you would like to deref, try using `&**`",
format!(
"&**{}",
&snippet_opt(cx, deref_target.span).unwrap(),
),
Applicability::MaybeIncorrect
);

}
);

}
}
}
}
4 changes: 2 additions & 2 deletions clippy_lints/src/checked_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
if let QPath::TypeRelative(ty, path) = &path;
if path.ident.name.as_str() == function;
if let TyKind::Path(QPath::Resolved(None, tp)) = &ty.kind;
if let [int] = &*tp.segments;
if let [int] = tp.segments;
then {
let name = int.ident.name.as_str();
candidates.iter().find(|c| &name == *c).copied()
Expand All @@ -332,7 +332,7 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> {
if_chain! {
if let QPath::Resolved(_, path) = *path;
if let [ty] = &*path.segments;
if let [ty] = path.segments;
then {
let name = ty.ident.name.as_str();
INTS.iter().find(|c| &name == *c).copied()
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/let_if_seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
if let hir::ExprKind::If(hir::Expr { kind: hir::ExprKind::DropTemps(cond), ..}, then, else_) = if_.kind;
if !is_local_used(cx, *cond, canonical_id);
if let hir::ExprKind::Block(then, _) = then.kind;
if let Some(value) = check_assign(cx, canonical_id, &*then);
if let Some(value) = check_assign(cx, canonical_id, then);
if !is_local_used(cx, value, canonical_id);
then {
let span = stmt.span.to(if_.span);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
LintId::of(booleans::LOGIC_BUG),
LintId::of(booleans::NONMINIMAL_BOOL),
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN),
LintId::of(casts::CAST_ABS_TO_UNSIGNED),
LintId::of(casts::CAST_ENUM_CONSTRUCTOR),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec![
LintId::of(attrs::DEPRECATED_CFG_ATTR),
LintId::of(booleans::NONMINIMAL_BOOL),
LintId::of(borrow_deref_ref::BORROW_DEREF_REF),
LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN),
LintId::of(casts::CHAR_LIT_AS_U8),
LintId::of(casts::UNNECESSARY_CAST),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ store.register_lints(&[
booleans::LOGIC_BUG,
booleans::NONMINIMAL_BOOL,
borrow_as_ptr::BORROW_AS_PTR,
borrow_deref_ref::BORROW_DEREF_REF,
bytecount::NAIVE_BYTECOUNT,
bytes_count_to_len::BYTES_COUNT_TO_LEN,
cargo::CARGO_COMMON_METADATA,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ mod blocks_in_if_conditions;
mod bool_assert_comparison;
mod booleans;
mod borrow_as_ptr;
mod borrow_deref_ref;
mod bytecount;
mod bytes_count_to_len;
mod cargo;
Expand Down Expand Up @@ -638,6 +639,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(mutex_atomic::Mutex));
store.register_late_pass(|| Box::new(needless_update::NeedlessUpdate));
store.register_late_pass(|| Box::new(needless_borrowed_ref::NeedlessBorrowedRef));
store.register_late_pass(|| Box::new(borrow_deref_ref::BorrowDerefRef));
store.register_late_pass(|| Box::new(no_effect::NoEffect));
store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));
store.register_late_pass(|| Box::new(transmute::Transmute));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops/never_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
if arms.is_empty() {
e
} else {
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| &*a.body), main_loop_id);
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| a.body), main_loop_id);
combine_seq(e, arms)
}
},
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/matches/match_bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
let exprs = if let PatKind::Lit(arm_bool) = arms[0].pat.kind {
if let ExprKind::Lit(ref lit) = arm_bool.kind {
match lit.node {
LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
LitKind::Bool(true) => Some((arms[0].body, arms[1].body)),
LitKind::Bool(false) => Some((arms[1].body, arms[0].body)),
_ => None,
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/unnecessary_to_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ fn is_cloned_or_copied(cx: &LateContext<'_>, method_name: Symbol, method_def_id:
/// Returns true if the named method can be used to convert the receiver to its "owned"
/// representation.
fn is_to_owned_like(cx: &LateContext<'_>, method_name: Symbol, method_def_id: DefId) -> bool {
is_clone_like(cx, &*method_name.as_str(), method_def_id)
is_clone_like(cx, method_name.as_str(), method_def_id)
|| is_cow_into_owned(cx, method_name, method_def_id)
|| is_to_string(cx, method_name, method_def_id)
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/mixed_read_write_in_expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
self.visit_expr(if_expr);
}
// make sure top level arm expressions aren't linted
self.maybe_walk_expr(&*arm.body);
self.maybe_walk_expr(arm.body);
}
},
_ => walk_expr(self, e),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/mut_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for MutableKeyType {
if let hir::PatKind::Wild = local.pat.kind {
return;
}
check_ty(cx, local.span, cx.typeck_results().pat_ty(&*local.pat));
check_ty(cx, local.span, cx.typeck_results().pat_ty(local.pat));
}
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir
expr_visitor_no_bodies(|expr| {
let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return true };
if matches!(
&*cx.tcx.item_name(macro_call.def_id).as_str(),
cx.tcx.item_name(macro_call.def_id).as_str(),
"unimplemented" | "unreachable" | "panic" | "todo" | "assert" | "assert_eq" | "assert_ne"
) {
panics.push(macro_call.span);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/pass_by_ref_or_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue {
}

if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind {
self.check_poly_fn(cx, item.def_id, &*method_sig.decl, None);
self.check_poly_fn(cx, item.def_id, method_sig.decl, None);
}
}

Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/redundant_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ fn is_call_with_ref_arg<'tcx>(
if let mir::TerminatorKind::Call { func, args, destination, .. } = kind;
if args.len() == 1;
if let mir::Operand::Move(mir::Place { local, .. }) = &args[0];
if let ty::FnDef(def_id, _) = *func.ty(&*mir, cx.tcx).kind();
if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx));
if let ty::FnDef(def_id, _) = *func.ty(mir, cx.tcx).kind();
if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(mir, cx.tcx));
if !is_copy(cx, inner_ty);
then {
Some((def_id, *local, inner_ty, destination.as_ref().map(|(dest, _)| dest)?.as_local()?))
Expand Down Expand Up @@ -318,7 +318,7 @@ fn find_stmt_assigns_to<'tcx>(
None
})?;

match (by_ref, &*rvalue) {
match (by_ref, rvalue) {
(true, mir::Rvalue::Ref(_, _, place)) | (false, mir::Rvalue::Use(mir::Operand::Copy(place))) => {
Some(base_local_and_movability(cx, mir, *place))
},
Expand Down
9 changes: 3 additions & 6 deletions clippy_lints/src/redundant_static_lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ impl RedundantStaticLifetimes {
fn visit_type(&mut self, ty: &Ty, cx: &EarlyContext<'_>, reason: &str) {
match ty.kind {
// Be careful of nested structures (arrays and tuples)
TyKind::Array(ref ty, _) => {
self.visit_type(&*ty, cx, reason);
TyKind::Array(ref ty, _) | TyKind::Slice(ref ty) => {
self.visit_type(ty, cx, reason);
},
TyKind::Tup(ref tup) => {
for tup_ty in tup {
self.visit_type(&*tup_ty, cx, reason);
self.visit_type(tup_ty, cx, reason);
}
},
// This is what we are looking for !
Expand Down Expand Up @@ -89,9 +89,6 @@ impl RedundantStaticLifetimes {
}
self.visit_type(&*borrow_type.ty, cx, reason);
},
TyKind::Slice(ref ty) => {
self.visit_type(ty, cx, reason);
},
_ => {},
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/size_of_in_element_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn get_pointee_ty_and_count_expr<'tcx>(
// Find calls to copy_{from,to}{,_nonoverlapping} and write_bytes methods
if let ExprKind::MethodCall(method_path, [ptr_self, .., count], _) = expr.kind;
let method_ident = method_path.ident.as_str();
if METHODS.iter().any(|m| *m == &*method_ident);
if METHODS.iter().any(|m| *m == method_ident);

// Get the pointee type
if let ty::RawPtr(TypeAndMut { ty: pointee_ty, .. }) =
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass {
}
} else if let Some(macro_call) = root_macro_call_first_node(cx, item) {
if !matches!(
&*cx.tcx.item_name(macro_call.def_id).as_str(),
cx.tcx.item_name(macro_call.def_id).as_str(),
"impl_lint_pass" | "declare_lint_pass"
) {
return;
Expand Down Expand Up @@ -504,7 +504,7 @@ fn check_invalid_clippy_version_attribute(cx: &LateContext<'_>, item: &'_ Item<'
return;
}

if RustcVersion::parse(&*value.as_str()).is_err() {
if RustcVersion::parse(value.as_str()).is_err() {
span_lint_and_help(
cx,
INVALID_CLIPPY_VERSION_ATTRIBUTE,
Expand Down Expand Up @@ -595,7 +595,7 @@ impl<'tcx> LateLintPass<'tcx> for CompilerLintFunctions {
if_chain! {
if let ExprKind::MethodCall(path, [self_arg, ..], _) = &expr.kind;
let fn_name = path.ident;
if let Some(sugg) = self.map.get(&*fn_name.as_str());
if let Some(sugg) = self.map.get(fn_name.as_str());
let ty = cx.typeck_results().expr_ty(self_arg).peel_refs();
if match_type(cx, ty, &paths::EARLY_CONTEXT)
|| match_type(cx, ty, &paths::LATE_CONTEXT);
Expand Down Expand Up @@ -679,7 +679,7 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls {
then {
let and_then_snippets = get_and_then_snippets(cx, and_then_args);
let mut sle = SpanlessEq::new(cx).deny_side_effects();
match &*ps.ident.as_str() {
match ps.ident.as_str() {
"span_suggestion" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => {
suggest_suggestion(cx, expr, &and_then_snippets, &span_suggestion_snippets(cx, span_call_args));
},
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/utils/internal_lints/metadata_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,12 +527,11 @@ fn extract_attr_docs_or_lint(cx: &LateContext<'_>, item: &Item<'_>) -> Option<St
fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str);
let mut docs = String::from(&*lines.next()?.as_str());
let mut docs = String::from(lines.next()?.as_str());
let mut in_code_block = false;
let mut is_code_block_rust = false;
for line in lines {
let line = line.as_str();
let line = &*line;

// Rustdoc hides code lines starting with `# ` and this removes them from Clippy's lint list :)
if is_code_block_rust && line.trim_start().starts_with("# ") {
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
fn ifthenelse(&mut self, cond: &Expr<'_>, then: &Expr<'_>, otherwise: Option<&Expr<'_>>) -> Option<Constant> {
if let Some(Constant::Bool(b)) = self.expr(cond) {
if b {
self.expr(&*then)
self.expr(then)
} else {
otherwise.as_ref().and_then(|expr| self.expr(expr))
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl<'tcx> ForLoop<'tcx> {
if let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind;
if let hir::ExprKind::Call(_, [arg]) = iterexpr.kind;
if let hir::ExprKind::Loop(block, ..) = arm.body.kind;
if let [stmt] = &*block.stmts;
if let [stmt] = block.stmts;
if let hir::StmtKind::Expr(e) = stmt.kind;
if let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind;
if let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind;
Expand Down
Loading