diff --git a/CHANGELOG.md b/CHANGELOG.md index 41c334c68169..2b2c93d565e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2063,6 +2063,7 @@ Released 2018-09-13 [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call [`expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_used [`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy +[`explicit_auto_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref [`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop [`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods [`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 4d0fdadbd85d..82d30ae1a3e1 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -67,7 +67,7 @@ pub fn run(check: bool, verbose: bool) { continue; } - success &= rustfmt(context, &path)?; + success &= rustfmt(context, path)?; } Ok(success) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index b5fb51af1c7f..e9f106cbcdcc 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,11 +1,53 @@ -use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg}; -use if_chain::if_chain; -use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX}; +use crate::{ + needless_borrow::NEEDLESS_BORROW, + utils::{ + expr_sig, get_node_span, get_parent_node, in_macro, is_allowed, match_def_path, paths, peel_hir_ty_refs, + peel_mid_ty_refs, snippet_with_context, span_lint_and_sugg, + }, +}; +use rustc_ast::util::parser::PREC_PREFIX; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{ + self as hir, Arm, Block, BorrowKind, Destination, Expr, ExprKind, FnRetTy, GenericArg, HirId, ImplItem, + ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Path, QPath, TraitItem, TraitItemKind, TyKind, + UnOp, +}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Span; +use rustc_middle::ty::{ + self, + adjustment::{Adjust, Adjustment}, + Ty, TyCtxt, TyS, TypeFoldable, TypeckResults, +}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{symbol::sym, Span}; + +declare_clippy_lint! { + /// **What it does:** Checks for explicit dereferencing which would be covered by + /// auto-dereferencing. + /// + /// **Why is this bad?** This unnecessarily complicates the code. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// use std::ops::Deref; + /// fn foo(_: &str) {} + /// foo(&*String::new()); + /// foo(String::new().deref()); + /// ``` + /// Use instead: + /// ```rust + /// use std::ops::Deref; + /// fn foo(_: &str) {} + /// foo(&String::new()); + /// foo(&String::new()); + /// ``` + pub EXPLICIT_AUTO_DEREF, + style, + "dereferencing when the compiler would automatically dereference" +} declare_clippy_lint! { /// **What it does:** Checks for explicit `deref()` or `deref_mut()` method calls. @@ -34,76 +76,732 @@ declare_clippy_lint! { "Explicit use of deref or deref_mut method while not in a method chain." } -declare_lint_pass!(Dereferencing => [ - EXPLICIT_DEREF_METHODS +impl_lint_pass!(Dereferencing => [ + EXPLICIT_AUTO_DEREF, + EXPLICIT_DEREF_METHODS, + NEEDLESS_BORROW, ]); +#[derive(Default)] +pub struct Dereferencing { + state: Option<(State, StateData)>, + + // While parsing a `deref` method call in ufcs form, the path to the function is itself an + // expression. This is to store the id of that expression so it can be skipped when + // `check_expr` is called for it. + skip_expr: Option, +} + +struct StateData { + /// Span of the top level expression + span: Span, + // HirId of the top level expression + hir_id: HirId, + /// The required mutability + target_mut: Mutability, +} + +enum State { + AddrOf, + // Any number of reference operations which auto-deref would take care of. + // This should take priority over all others. + AutoDeref { + sub_span: Option, + }, + // Any number of deref method calls. + DerefMethod { + // The number of calls in a sequence which changed the referenced type + ty_changed_count: usize, + is_final_ufcs: bool, + }, + NeedlessBorrow { + // The number of borrows remaining + remaining: usize, + }, +} + +// A reference operation considered by this lint pass +enum RefOp { + Method, + Deref, + AddrOf, +} + impl<'tcx> LateLintPass<'tcx> for Dereferencing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if !expr.span.from_expansion(); - if let ExprKind::MethodCall(ref method_name, _, ref args, _) = &expr.kind; - if args.len() == 1; - - then { - if let Some(parent_expr) = get_parent_expr(cx, expr) { - // Check if we have the whole call chain here - if let ExprKind::MethodCall(..) = parent_expr.kind { - return; - } - // Check for Expr that we don't want to be linted - let precedence = parent_expr.precedence(); - match precedence { - // Lint a Call is ok though - ExprPrecedence::Call | ExprPrecedence::AddrOf => (), - _ => { - if precedence.order() >= PREC_PREFIX && precedence.order() <= PREC_POSTFIX { - return; - } + // Skip path expressions from deref calls. e.g. `Deref::deref(e)` + if Some(expr.hir_id) == self.skip_expr.take() { + return; + } + + // Stop processing sub expressions when a macro call is seen + if in_macro(expr.span) { + if let Some((state, data)) = self.state.take() { + report(cx, expr, state, data); + } + return; + } + + let typeck = cx.typeck_results(); + let (kind, sub_expr) = match try_parse_ref_op(cx.tcx, typeck, expr) { + Some(x) => x, + None => { + // The whole chain of reference operations has been seen + if let Some((state, data)) = self.state.take() { + report(cx, expr, state, data); + } + return; + }, + }; + + self.state = match (self.state.take(), kind) { + (None, kind) => { + let parent = get_parent_node(cx.tcx, expr.hir_id); + // format_args will insert an address of expression into the wrong span. + if parent + .and_then(get_node_span) + .and_then(|s| s.ctxt().outer_expn_data().macro_def_id) + .map_or(false, |id| { + match_def_path(cx, id, &paths::FORMAT_ARGS_MACRO) + || match_def_path(cx, id, &paths::FORMAT_ARGS_NL_MACRO) + }) + { + return; + } + + let expr_adjustments = find_adjustments(cx.tcx, typeck, expr); + let expr_ty = typeck.expr_ty(expr); + let data = StateData { + span: expr.span, + hir_id: expr.hir_id, + target_mut: if let ty::Ref(_, _, mutability) = + *expr_adjustments.last().map_or(expr_ty, |a| a.target).kind() + { + mutability + } else { + Mutability::Not + }, + }; + + match (kind, expr_adjustments) { + ( + RefOp::AddrOf, + [Adjustment { + kind: Adjust::Deref(None), + target, + }, Adjustment { + kind: Adjust::Deref(None), + .. + }, adjustments @ ..], + ) if target.is_ref() => { + let count = adjustments + .iter() + .take_while(|&a| matches!(a.kind, Adjust::Deref(None)) && a.target.is_ref()) + .count(); + Some(( + State::NeedlessBorrow { + remaining: if matches!( + adjustments.get(count), + Some(Adjustment { + kind: Adjust::Borrow(_), + .. + }) + ) { + count + } else { + count + 1 + }, + }, + data, + )) + }, + (RefOp::AddrOf, _) if !typeck.expr_ty(sub_expr).is_ref() => Some((State::AddrOf, data)), + // Only check for auto-deref when the resulting type is a reference to a + // non-reference type. + (RefOp::Deref | RefOp::Method, _) + if expr_ty.is_ref() + && !is_allowed(cx, EXPLICIT_AUTO_DEREF, expr.hir_id) + && is_stable_auto_deref_position(cx.tcx, typeck, expr.hir_id) => + { + Some((State::AutoDeref { sub_span: None }, data)) + }, + (RefOp::Method, _) + if !is_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) + && is_linted_explicit_deref_position(parent, expr.hir_id) => + { + Some(( + State::DerefMethod { + ty_changed_count: if deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)) { + 0 + } else { + 1 + }, + is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), + }, + data, + )) + }, + _ => None, + } + }, + (Some((State::AddrOf, data)), RefOp::Deref | RefOp::Method) + if !is_allowed(cx, EXPLICIT_AUTO_DEREF, data.hir_id) + && is_stable_auto_deref_position(cx.tcx, typeck, data.hir_id) => + { + Some(( + State::AutoDeref { + sub_span: Some(expr.span), + }, + data, + )) + }, + (state @ Some((State::AutoDeref { .. }, _)), _) => state, + (Some((State::DerefMethod { ty_changed_count, .. }, data)), RefOp::Method) => Some(( + State::DerefMethod { + ty_changed_count: if deref_method_same_type(typeck.expr_ty(expr), typeck.expr_ty(sub_expr)) { + ty_changed_count + } else { + ty_changed_count + 1 + }, + is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), + }, + data, + )), + (Some((State::NeedlessBorrow { remaining }, data)), RefOp::AddrOf) if remaining != 0 => Some(( + State::NeedlessBorrow { + remaining: remaining - 1, + }, + data, + )), + (Some((state, data)), _) => { + report(cx, expr, state, data); + None + }, + }; + } +} + +fn try_parse_ref_op( + tcx: TyCtxt<'tcx>, + typeck: &'tcx TypeckResults<'_>, + expr: &'tcx Expr<'_>, +) -> Option<(RefOp, &'tcx Expr<'tcx>)> { + let (def_id, arg) = match expr.kind { + ExprKind::MethodCall(_, _, [arg], _) => (typeck.type_dependent_def_id(expr.hir_id)?, arg), + ExprKind::Call( + Expr { + kind: ExprKind::Path(path), + hir_id, + .. + }, + [arg], + ) => (typeck.qpath_res(path, *hir_id).opt_def_id()?, arg), + ExprKind::Unary(UnOp::Deref, sub_expr) if !typeck.expr_ty(sub_expr).is_unsafe_ptr() => { + return Some((RefOp::Deref, sub_expr)); + }, + ExprKind::AddrOf(BorrowKind::Ref, _, sub_expr) => return Some((RefOp::AddrOf, sub_expr)), + _ => return None, + }; + if tcx.is_diagnostic_item(sym::deref_method, def_id) + || tcx.trait_of_item(def_id)? == tcx.lang_items().deref_mut_trait()? + { + Some((RefOp::Method, arg)) + } else { + return None; + } +} + +// Checks whether the type for a deref call actually changed the type, not just the mutability of +// the reference. +fn deref_method_same_type(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { + match (result_ty.kind(), arg_ty.kind()) { + (ty::Ref(_, result_ty, _), ty::Ref(_, arg_ty, _)) => TyS::same_type(result_ty, arg_ty), + + // The result type for a deref method is always a reference + // Not matching the previous pattern means the argument type is not a reference + // This means that the type did change + _ => false, + } +} + +// Adjustments are sometimes made in the parent block rather than the expression itself. +fn find_adjustments( + tcx: TyCtxt<'tcx>, + typeck: &'tcx TypeckResults<'_>, + expr: &'tcx Expr<'_>, +) -> &'tcx [Adjustment<'tcx>] { + let map = tcx.hir(); + let mut iter = map.parent_iter(expr.hir_id); + let mut prev = expr; + + loop { + match typeck.expr_adjustments(prev) { + [] => (), + a => break a, + }; + + match iter.next().map(|(_, x)| x) { + Some(Node::Block(_)) => { + if let Some((_, Node::Expr(e))) = iter.next() { + prev = e; + } else { + // This shouldn't happen. Blocks are always contained in an expression. + break &[]; + } + }, + Some(Node::Expr(&Expr { + kind: ExprKind::Break(Destination { target_id: Ok(id), .. }, _), + .. + })) => { + if let Some(Node::Expr(e)) = map.find(id) { + prev = e; + iter = map.parent_iter(id); + continue; + } else { + // This shouldn't happen. The destination should definitely exist at this point. + break &[]; + } + }, + _ => break &[], + } + } +} + +// Checks whether the parent node is a suitable context for switching from a deref method to the +// deref operator. +fn is_linted_explicit_deref_position(parent: Option>, child_id: HirId) -> bool { + let parent = match parent { + Some(Node::Expr(e)) => e, + _ => return true, + }; + match parent.kind { + // Leave deref calls in the middle of a method chain. + // e.g. x.deref().foo() + ExprKind::MethodCall(_, _, [self_arg, ..], _) if self_arg.hir_id == child_id => false, + + // Leave deref calls resulting in a called function + // e.g. (x.deref())() + ExprKind::Call(func_expr, _) if func_expr.hir_id == child_id => false, + + // Leave deref calls for try and await expressions + // e.g. x.deref()? + ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) => false, + + // Makes an ugly suggestion + // e.g. *x.deref() => *&*x + ExprKind::Unary(UnOp::Deref, _) => false, + + ExprKind::Field(..) | ExprKind::Index(..) | ExprKind::Err => false, + + ExprKind::Box(..) + | ExprKind::ConstBlock(..) + | ExprKind::Array(_) + | ExprKind::Call(..) + | ExprKind::MethodCall(..) + | ExprKind::Tup(..) + | ExprKind::Binary(..) + | ExprKind::Unary(..) + | ExprKind::Lit(..) + | ExprKind::Cast(..) + | ExprKind::Type(..) + | ExprKind::DropTemps(..) + | ExprKind::If(..) + | ExprKind::Loop(..) + | ExprKind::Match(..) + | ExprKind::Closure(..) + | ExprKind::Block(..) + | ExprKind::Assign(..) + | ExprKind::AssignOp(..) + | ExprKind::Path(..) + | ExprKind::AddrOf(..) + | ExprKind::Break(..) + | ExprKind::Continue(..) + | ExprKind::Ret(..) + | ExprKind::InlineAsm(..) + | ExprKind::LlvmInlineAsm(..) + | ExprKind::Struct(..) + | ExprKind::Repeat(..) + | ExprKind::Yield(..) => true, + } +} + +// Checks if the expression for the given id occurs in a position which auto dereferencing applies. +// Note that the target type must not be inferred in a way that may cause auto-deref to select a +// different type, nor may the position be the result of a macro expansion. +// +// e.g. the following should not linted +// macro_rules! foo { ($e:expr) => { let x: &str = $e; }} +// foo!(&*String::new()); +// fn foo(_: &T) {} +// foo(&*String::new()) +fn is_stable_auto_deref_position(tcx: TyCtxt<'tcx>, typeck: &'tcx TypeckResults<'_>, id: HirId) -> bool { + let map = tcx.hir(); + // break expressions can a jump to a different node, so for loops won't work here. + let mut iter = map.parent_iter(id); + let mut child_id = id; + while let Some((parent_id, parent)) = iter.next() { + match parent { + // Local binding + Node::Local(&Local { ty: Some(ty), span, .. }) if !in_macro(span) => { + return is_binding_ty_auto_deref_stable(ty); + }, + + // Static and const bindings. The type of these have to be fully defined. + Node::Item(&Item { + kind: ItemKind::Static(..), + span, + .. + }) + | Node::Item(&Item { + kind: ItemKind::Const(..), + span, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Const(..), + span, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Const(..), + span, + .. + }) if !in_macro(span) => return true, + + // Implicit return from a function. Determine whether auto-deref might change the type. + Node::Item(&Item { + kind: ItemKind::Fn(..), + span, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Fn(..), + span, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Fn(..), + span, + .. + }) if !in_macro(span) => { + let output = tcx + .fn_sig(tcx.hir().local_def_id(parent_id).to_def_id()) + .skip_binder() + .output(); + + return !(output.has_placeholders() || output.has_opaque_types() || output.has_projections()); + }, + + Node::Arm(&Arm { span, body, .. }) if body.hir_id == child_id && !in_macro(span) => (), + + Node::Block(&Block { + span, expr: Some(expr), .. + }) if expr.hir_id == child_id && !in_macro(span) => (), + + Node::Expr(parent) if !in_macro(parent.span) => match parent.kind { + // Determine whether auto-deref might change the return type of the function. + ExprKind::Ret(Some(_)) if !in_macro(map.span(map.local_def_id_to_hir_id(typeck.hir_owner))) => { + let output = tcx.fn_sig(typeck.hir_owner).skip_binder().output(); + return !(output.has_placeholders() || output.has_opaque_types() || output.has_projections()); + }, + + // Determine which argument the child is and whether auto-deref might cause it's + // type to change. + ExprKind::Call(func, args) => { + let arg_pos = if let Some(arg_pos) = args.iter().position(|x| x.hir_id == child_id) { + arg_pos + } else { + return false; + }; + + return expr_sig(tcx, typeck, func).map_or(false, |sig| { + is_param_auto_deref_stable(sig.input(arg_pos).skip_binder()) + }); + }, + + // Determine which argument, ignoring the self parameter, the child is and whether + // auto-deref might cause it's type to change. + ExprKind::MethodCall(_, _, [_, args @ ..], _) => { + let id = typeck.type_dependent_def_id(parent.hir_id).unwrap(); + return if let Some(arg) = args.iter().position(|x| x.hir_id == child_id) { + let arg = &tcx.fn_sig(id).skip_binder().inputs()[arg]; + is_param_auto_deref_stable(arg) + } else { + false + }; + }, + + // Determine which field is being assigned and whether auto-deref might cause the + // type to change + ExprKind::Struct(path, fields, _) => { + let res = typeck.qpath_res(path, parent.hir_id); + let id = res.def_id(); + return if let Some(field) = fields.iter().find(|f| f.expr.hir_id == child_id) { + if let Some(field) = tcx + .adt_def(id) + .variant_of_res(res) + .fields + .iter() + .find(|f| f.ident == field.ident) + { + let field_ty = tcx.type_of(field.did); + is_param_auto_deref_stable(field_ty) + } else { + false } + } else { + false + }; + }, + + // Continue to the parent expression + ExprKind::Block(..) => (), + + ExprKind::If(cond_expr, ..) if cond_expr.hir_id != child_id => (), + + ExprKind::Match(scrutinee_expr, ..) if scrutinee_expr.hir_id != child_id => (), + + // Continue to the loop's parent. + ExprKind::Break(Destination { target_id: Ok(id), .. }, _) => { + iter = map.parent_iter(id); + child_id = id; + continue; + }, + + // All other expressions aren't auto-deref contexts. + _ => return false, + }, + + // All other nodes aren't auto-deref contexts. + _ => return false, + } + + child_id = parent_id; + } + false +} + +// Checks whether auto-dereferencing any type into a binding of the given type will definitely +// produce the same result. +// +// e.g. +// let x = Box::new(Box::new(0u32)); +// let y1: &Box<_> = x.deref(); +// let y2: &Box<_> = &x; +// +// Here `y1` and `y2` would resolve to different types, so the type `&Box<_>` is not stable when +// switching to auto-dereferencing. +fn is_binding_ty_auto_deref_stable(ty: &hir::Ty<'_>) -> bool { + let (ty, count) = peel_hir_ty_refs(ty); + if count != 1 { + return false; + } + + match &ty.kind { + TyKind::Slice(_) + | TyKind::Array(..) + | TyKind::BareFn(_) + | TyKind::Never + | TyKind::Typeof(..) + | TyKind::Tup(_) + | TyKind::Ptr(_) + | TyKind::TraitObject(..) => true, + TyKind::OpaqueDef(..) | TyKind::Infer | TyKind::Err => false, + TyKind::Rptr(_, ty) => is_binding_ty_auto_deref_stable(&ty.ty), + &TyKind::Path(QPath::Resolved( + _, + Path { + segments: [.., path], .. + }, + )) + | &TyKind::Path(QPath::TypeRelative(_, path)) => { + if let Some(args) = path.args { + args.args.iter().all(|arg| { + if let GenericArg::Type(ty) = arg { + !ty_contains_infer(ty) + } else { + true } - } - let name = method_name.ident.as_str(); - lint_deref(cx, &*name, &args[0], args[0].span, expr.span); + }) + } else { + true } - } + }, + TyKind::Path(_) => true, } } -fn lint_deref(cx: &LateContext<'_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) { - match method_name { - "deref" => { - let impls_deref_trait = cx.tcx.lang_items().deref_trait().map_or(false, |id| { - implements_trait(cx, cx.typeck_results().expr_ty(&call_expr), id, &[]) - }); - if impls_deref_trait { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHODS, - expr_span, - "explicit deref method call", - "try this", - format!("&*{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); +// Checks whether a type is inferred at some point. +// e.g. _, Box<_>, [_] +fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool { + match &ty.kind { + TyKind::OpaqueDef(..) | TyKind::Infer | TyKind::Err => true, + TyKind::Never | TyKind::Typeof(_) | TyKind::TraitObject(..) => false, + TyKind::Slice(ty) | TyKind::Array(ty, _) => ty_contains_infer(ty), + TyKind::Ptr(ty) | TyKind::Rptr(_, ty) => ty_contains_infer(&ty.ty), + TyKind::Tup(tys) => tys.iter().any(ty_contains_infer), + TyKind::BareFn(ty) => { + if ty.decl.inputs.iter().any(ty_contains_infer) { + return true; + } + if let FnRetTy::Return(ty) = &ty.decl.output { + ty_contains_infer(ty) + } else { + false } }, - "deref_mut" => { - let impls_deref_mut_trait = cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| { - implements_trait(cx, cx.typeck_results().expr_ty(&call_expr), id, &[]) - }); - if impls_deref_mut_trait { - span_lint_and_sugg( - cx, - EXPLICIT_DEREF_METHODS, - expr_span, - "explicit deref_mut method call", - "try this", - format!("&mut *{}", &snippet(cx, var_span, "..")), - Applicability::MachineApplicable, - ); + &TyKind::Path(QPath::Resolved( + _, + Path { + segments: [.., path], .. + }, + )) + | &TyKind::Path(QPath::TypeRelative(_, path)) => { + if let Some(args) = path.args { + args.args.iter().any(|arg| { + if let GenericArg::Type(ty) = arg { + ty_contains_infer(ty) + } else { + false + } + }) + } else { + false } }, - _ => (), + TyKind::Path(_) => true, + } +} + +// Checks whether a type is stable when switching to auto dereferencing, +fn is_param_auto_deref_stable(ty: Ty<'_>) -> bool { + let (ty, count) = peel_mid_ty_refs(ty); + if count != 1 { + return false; + } + + match ty.peel_refs().kind() { + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Foreign(_) + | ty::Str + | ty::Array(..) + | ty::Slice(..) + | ty::RawPtr(..) + | ty::FnDef(..) + | ty::FnPtr(_) + | ty::Closure(..) + | ty::Generator(..) + | ty::GeneratorWitness(..) + | ty::Never + | ty::Tuple(_) + | ty::Ref(..) => true, + ty::Placeholder(_) + | ty::Infer(_) + | ty::Error(_) + | ty::Param(_) + | ty::Bound(..) + | ty::Opaque(..) + | ty::Projection(_) + | ty::Dynamic(..) => false, + ty::Adt(_, subs) => subs + .types() + .all(|ty| !(ty.has_placeholders() || ty.has_param_types_or_consts() || ty.has_projections())), + } +} + +fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { + match state { + State::AddrOf => (), + State::AutoDeref { sub_span } => { + let (span, needs_ref) = match (sub_span, cx.typeck_results().expr_ty(expr).is_ref()) { + (Some(_), true) => (data.span, None), + (Some(sub_span), false) => (sub_span, None), + (None, is_ref) => (data.span, (!is_ref).then(|| data.target_mut)), + }; + + let mut app = Applicability::MachineApplicable; + let (expr_str, _) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); + + span_lint_and_sugg( + cx, + EXPLICIT_AUTO_DEREF, + span, + "explicit dereference in auto-deref context", + "try this", + match needs_ref { + None => expr_str.into(), + Some(Mutability::Not) => format!("&{}", expr_str), + Some(Mutability::Mut) => format!("&mut {}", expr_str), + }, + app, + ); + }, + State::DerefMethod { + ty_changed_count, + is_final_ufcs, + } => { + let mut app = Applicability::MachineApplicable; + let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); + let ty = cx.typeck_results().expr_ty(expr); + let (_, ref_count) = peel_mid_ty_refs(ty); + let deref_str = if ty_changed_count >= ref_count && ref_count != 0 { + // a deref call changing &T -> &U requires two deref operators the first time + // this occurs. One to remove the reference, a second to call the deref impl. + "*".repeat(ty_changed_count + 1) + } else { + "*".repeat(ty_changed_count) + }; + let addr_of_str = if ty_changed_count < ref_count { + // Check if a reborrow from &mut T -> &T is required. + if data.target_mut == Mutability::Not && matches!(ty.kind(), ty::Ref(_, _, Mutability::Mut)) { + "&*" + } else { + "" + } + } else if data.target_mut == Mutability::Mut { + "&mut " + } else { + "&" + }; + + let expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { + format!("({})", expr_str) + } else { + expr_str.into_owned() + }; + + span_lint_and_sugg( + cx, + EXPLICIT_DEREF_METHODS, + data.span, + "explicit `deref` method call", + "try this", + format!("{}{}{}", addr_of_str, deref_str, expr_str), + app, + ); + }, + State::NeedlessBorrow { .. } => { + let ty = cx.typeck_results().expr_ty(expr); + let mut app = Applicability::MachineApplicable; + let (expr_str, _) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); + + span_lint_and_sugg( + cx, + NEEDLESS_BORROW, + data.span, + &format!( + "this expression borrows a reference (`{}`) that is immediately dereferenced \ + by the compiler", + ty, + ), + "change this to", + expr_str.into(), + app, + ); + }, } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 183778cb55eb..2fe506d46251 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -601,6 +601,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &default::DEFAULT_TRAIT_ACCESS, &default::FIELD_REASSIGN_WITH_DEFAULT, &default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK, + &dereference::EXPLICIT_AUTO_DEREF, &dereference::EXPLICIT_DEREF_METHODS, &derive::DERIVE_HASH_XOR_EQ, &derive::DERIVE_ORD_XOR_PARTIAL_ORD, @@ -1242,7 +1243,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); - store.register_late_pass(|| box dereference::Dereferencing); + store.register_late_pass(|| box dereference::Dereferencing::default()); store.register_late_pass(|| box option_if_let_else::OptionIfLetElse); store.register_late_pass(|| box future_not_send::FutureNotSend); store.register_late_pass(|| box if_let_mutex::IfLetMutex); @@ -1466,6 +1467,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), LintId::of(&default::FIELD_REASSIGN_WITH_DEFAULT), + LintId::of(&dereference::EXPLICIT_AUTO_DEREF), LintId::of(&derive::DERIVE_HASH_XOR_EQ), LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD), LintId::of(&doc::MISSING_SAFETY_DOC), @@ -1630,6 +1632,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE), LintId::of(&needless_bool::BOOL_COMPARISON), LintId::of(&needless_bool::NEEDLESS_BOOL), + LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE), LintId::of(&needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(&needless_update::NEEDLESS_UPDATE), @@ -1745,6 +1748,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&collapsible_match::COLLAPSIBLE_MATCH), LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&default::FIELD_REASSIGN_WITH_DEFAULT), + LintId::of(&dereference::EXPLICIT_AUTO_DEREF), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), LintId::of(&enum_variants::ENUM_VARIANT_NAMES), @@ -1900,6 +1904,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE), LintId::of(&needless_bool::BOOL_COMPARISON), LintId::of(&needless_bool::NEEDLESS_BOOL), + LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE), LintId::of(&needless_question_mark::NEEDLESS_QUESTION_MARK), LintId::of(&needless_update::NEEDLESS_UPDATE), @@ -2055,7 +2060,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&mutex_atomic::MUTEX_INTEGER), - LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(®ex::TRIVIAL_REGEX), diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs index ac1d51e1993b..ea4cedc67545 100644 --- a/clippy_lints/src/manual_map.rs +++ b/clippy_lints/src/manual_map.rs @@ -129,7 +129,7 @@ impl LateLintPass<'_> for ManualMap { // Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or // it's being passed by value. let scrutinee = peel_hir_expr_refs(scrutinee).0; - let scrutinee_str = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app); + let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app); let scrutinee_str = if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX { format!("({})", scrutinee_str) @@ -160,7 +160,7 @@ impl LateLintPass<'_> for ManualMap { "|{}{}| {}", annotation, some_binding, - snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app) + snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app).0 ) }, } @@ -168,8 +168,8 @@ impl LateLintPass<'_> for ManualMap { // TODO: handle explicit reference annotations. format!( "|{}| {}", - snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app), - snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app) + snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0, + snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app).0 ) } else { // Refutable bindings and mixed reference annotations can't be handled by `map`. diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 1453ea6e8975..9946cdedc3f8 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -5,10 +5,9 @@ use crate::utils::{is_automatically_derived, snippet_opt, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BindingAnnotation, BorrowKind, Expr, ExprKind, Item, Mutability, Pat, PatKind}; +use rustc_hir::{BindingAnnotation, Item, Mutability, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; -use rustc_middle::ty::adjustment::{Adjust, Adjustment}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::def_id::LocalDefId; @@ -30,7 +29,7 @@ declare_clippy_lint! { /// let x: &i32 = &5; /// ``` pub NEEDLESS_BORROW, - nursery, + complexity, "taking a reference that is going to be automatically dereferenced" } @@ -42,47 +41,6 @@ pub struct NeedlessBorrow { impl_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW]); impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { - fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - if e.span.from_expansion() || self.derived_item.is_some() { - return; - } - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, ref inner) = e.kind { - if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(inner).kind() { - for adj3 in cx.typeck_results().expr_adjustments(e).windows(3) { - if let [Adjustment { - kind: Adjust::Deref(_), .. - }, Adjustment { - kind: Adjust::Deref(_), .. - }, Adjustment { - kind: Adjust::Borrow(_), - .. - }] = *adj3 - { - span_lint_and_then( - cx, - NEEDLESS_BORROW, - e.span, - &format!( - "this expression borrows a reference (`&{}`) that is immediately dereferenced \ - by the compiler", - ty - ), - |diag| { - if let Some(snippet) = snippet_opt(cx, inner.span) { - diag.span_suggestion( - e.span, - "change this to", - snippet, - Applicability::MachineApplicable, - ); - } - }, - ); - } - } - } - } - } fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { if pat.span.from_expansion() || self.derived_item.is_some() { return; diff --git a/clippy_lints/src/redundant_deref.rs b/clippy_lints/src/redundant_deref.rs new file mode 100644 index 000000000000..029583720d2f --- /dev/null +++ b/clippy_lints/src/redundant_deref.rs @@ -0,0 +1,63 @@ +// use crate::utils::{get_parent_expr, snippet_with_applicability, span_lint_and_sugg}; +// use if_chain::if_chain; +// use rustc_errors::Applicability; +// use rustc_hir::{Expr, ExprKind, UnOp}; +// use rustc_lint::{LateContext, LateLintPass, LintContext}; +// use rustc_middle::lint::in_external_macro; +// use rustc_session::{declare_lint_pass, declare_tool_lint}; + +// declare_clippy_lint! { +// /// **What it does:** Checks for uses of the dereference operator which would be covered by +// /// auto-dereferencing. +// /// +// /// **Why is this bad?** This unnecessarily complicates the code. +// /// +// /// **Known problems:** None. +// /// +// /// **Example:** +// /// +// /// ```rust +// /// fn foo(_: &str) {} +// /// foo(&*String::new()) +// /// ``` +// /// Use instead: +// /// ```rust +// /// fn foo(_: &str) {} +// /// foo(&String::new()) +// /// ``` +// pub REDUNDANT_DEREF, +// style, +// "default lint description" +// } + +// declare_lint_pass!(RedundantDeref => [REDUNDANT_DEREF]); + +// impl LateLintPass<'_> for RedundantDeref { +// fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { +// if_chain! { +// if let ExprKind::AddrOf(_, _, addr_expr) = expr.kind; +// if let ExprKind::Unary(UnOp::UnDeref, deref_expr) = addr_expr.kind; +// if !in_external_macro(cx.sess(), expr.span); +// if let Some(parent_expr) = get_parent_expr(cx, expr); +// if match parent_expr.kind { +// ExprKind::Call(func, _) => func.hir_id != expr.hir_id, +// ExprKind::MethodCall(..) => true, +// _ => false, +// }; +// if !cx.typeck_results().expr_ty(deref_expr).is_unsafe_ptr(); +// then { +// let mut app = Applicability::MachineApplicable; +// let sugg = format!("&{}", snippet_with_applicability(cx, deref_expr.span, "_", &mut app)); +// span_lint_and_sugg( +// cx, +// REDUNDANT_DEREF, +// expr.span, +// "redundant dereference", +// "remove the dereference", +// sugg, +// app, +// ); +// } +// } +// } +// } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 00455d4102f9..5482fbab2bb7 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -62,21 +62,24 @@ use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::Node; use rustc_hir::{ - def, Arm, Block, Body, Constness, Crate, Expr, ExprKind, FnDecl, HirId, ImplItem, ImplItemKind, Item, ItemKind, - MatchSource, Param, Pat, PatKind, Path, PathSegment, QPath, TraitItem, TraitItemKind, TraitRef, TyKind, Unsafety, + def, Arm, Block, Body, Constness, Crate, CrateItem, Expr, ExprKind, FnDecl, ForeignItem, GenericParam, HirId, + ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Local, MacroDef, MatchSource, Param, Pat, PatKind, Path, + PathSegment, QPath, Stmt, StructField, TraitItem, TraitItemKind, TraitRef, TyKind, Unsafety, Variant, Visibility, }; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; use rustc_middle::hir::map::Map; -use rustc_middle::ty::subst::{GenericArg, GenericArgKind}; -use rustc_middle::ty::{self, layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst}; +use rustc_middle::ty::{ + self, layout::IntegerExt, Binder, DefIdTree, FnSig, PredicateKind, Ty, TyCtxt, TypeFoldable, TypeckResults, +}; use rustc_semver::RustcVersion; use rustc_session::Session; use rustc_span::hygiene::{self, ExpnKind, MacroKind}; use rustc_span::source_map::original_sp; use rustc_span::sym; -use rustc_span::symbol::{kw, Symbol}; +use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::{BytePos, Pos, Span, SyntaxContext, DUMMY_SP}; use rustc_target::abi::Integer; use rustc_trait_selection::traits::query::normalize::AtExt; @@ -781,26 +784,31 @@ pub fn snippet_block_with_applicability<'a, T: LintContext>( /// e.g. Given the expression `&vec![]`, getting a snippet from the span for `vec![]` as a HIR node /// would result in `box []`. If given the context of the address of expression, this function will /// correctly get a snippet of `vec![]`. +/// +/// This will also return whether or not the snippet is a macro call. pub fn snippet_with_context( cx: &LateContext<'_>, span: Span, outer: SyntaxContext, default: &'a str, applicability: &mut Applicability, -) -> Cow<'a, str> { +) -> (Cow<'a, str>, bool) { let outer_span = hygiene::walk_chain(span, outer); - let span = if outer_span.ctxt() == outer { - outer_span + let (span, is_macro_call) = if outer_span.ctxt() == outer { + (outer_span, span.ctxt() != outer) } else { // The span is from a macro argument, and the outer context is the macro using the argument if *applicability != Applicability::Unspecified { *applicability = Applicability::MaybeIncorrect; } // TODO: get the argument span. - span + (span, false) }; - snippet_with_applicability(cx, span, default, applicability) + ( + snippet_with_applicability(cx, span, default, applicability), + is_macro_call, + ) } /// Returns a new Span that extends the original Span to the first non-whitespace char of the first @@ -942,21 +950,52 @@ fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option, .join("\n") } +/// Gets the span of the node, if there is one. +pub fn get_node_span(node: Node<'_>) -> Option { + match node { + Node::Param(Param { span, .. }) + | Node::Item(Item { span, .. }) + | Node::ForeignItem(ForeignItem { span, .. }) + | Node::TraitItem(TraitItem { span, .. }) + | Node::ImplItem(ImplItem { span, .. }) + | Node::Variant(Variant { span, .. }) + | Node::Field(StructField { span, .. }) + | Node::Expr(Expr { span, .. }) + | Node::Stmt(Stmt { span, .. }) + | Node::PathSegment(PathSegment { + ident: Ident { span, .. }, + .. + }) + | Node::Ty(hir::Ty { span, .. }) + | Node::TraitRef(TraitRef { + path: Path { span, .. }, + .. + }) + | Node::Binding(Pat { span, .. }) + | Node::Pat(Pat { span, .. }) + | Node::Arm(Arm { span, .. }) + | Node::Block(Block { span, .. }) + | Node::Local(Local { span, .. }) + | Node::MacroDef(MacroDef { span, .. }) + | Node::Lifetime(Lifetime { span, .. }) + | Node::GenericParam(GenericParam { span, .. }) + | Node::Visibility(Visibility { span, .. }) + | Node::Crate(CrateItem { span, .. }) => Some(*span), + Node::Ctor(_) | Node::AnonConst(_) => None, + } +} + +/// Gets the parent node, if any. +pub fn get_parent_node(tcx: TyCtxt<'_>, id: HirId) -> Option> { + tcx.hir().parent_iter(id).next().map(|(_, node)| node) +} + /// Gets the parent expression, if any –- this is useful to constrain a lint. pub fn get_parent_expr<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { - let map = &cx.tcx.hir(); - let hir_id = e.hir_id; - let parent_id = map.get_parent_node(hir_id); - if hir_id == parent_id { - return None; - } - map.find(parent_id).and_then(|node| { - if let Node::Expr(parent) = node { - Some(parent) - } else { - None - } - }) + match get_parent_node(cx.tcx, e.hir_id) { + Some(Node::Expr(parent)) => Some(parent), + _ => None, + } } pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Block<'tcx>> { @@ -1726,6 +1765,96 @@ where match_expr_list } +/// If expr is a path, resolves it. Otherwise returns `None`. +pub fn expr_res(typeck: &TypeckResults<'_>, expr: &Expr<'_>) -> Option { + if let ExprKind::Path(p) = &expr.kind { + Some(typeck.qpath_res(p, expr.hir_id)) + } else { + None + } +} + +/// A signature for a function like type. +pub enum ExprFnSig<'tcx> { + Sig(Binder>), + Closure(Binder>), + Trait(Binder>), +} +impl ExprFnSig<'tcx> { + /// Gets the argument type at the given offset. + pub fn input(&self, i: usize) -> Binder> { + match self { + Self::Sig(sig) => sig.input(i), + Self::Closure(sig) => sig.input(0).map_bound(|ty| ty.tuple_element_ty(i).unwrap()), + Self::Trait(sig) => sig.map_bound(|ty| ty.tuple_element_ty(i).unwrap()), + } + } +} + +/// If expr is function like, get the signature for it. +pub fn expr_sig(tcx: TyCtxt<'tcx>, typeck: &TypeckResults<'tcx>, expr: &Expr<'_>) -> Option> { + match expr_res(typeck, expr) { + Some(Res::Def(DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn, id) | Res::SelfCtor(id)) => { + Some(ExprFnSig::Sig(tcx.fn_sig(id))) + }, + _ => { + let ty = typeck.expr_ty_adjusted(expr).peel_refs(); + match *ty.kind() { + ty::Closure(_, subs) => Some(ExprFnSig::Closure(subs.as_closure().sig())), + ty::FnDef(id, subs) => Some(ExprFnSig::Sig(tcx.fn_sig(id).subst(tcx, subs))), + ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)), + ty::Dynamic(bounds, _) => { + let lang_items = tcx.lang_items(); + match bounds.principal() { + Some(bound) + if Some(bound.def_id()) == lang_items.fn_trait() + || Some(bound.def_id()) == lang_items.fn_once_trait() + || Some(bound.def_id()) == lang_items.fn_mut_trait() => + { + Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)))) + }, + _ => None, + } + }, + ty::Param(_) | ty::Projection(..) => { + let mut id = typeck.hir_owner.to_def_id(); + loop { + let predicates = tcx.predicates_of(id); + let lang_items = tcx.lang_items(); + if let Some(res) = predicates + .predicates + .iter() + .find_map(|&(p, _)| { + p.kind() + .map_bound(|kind| match kind { + PredicateKind::Trait(p, _) + if (lang_items.fn_trait() == Some(p.def_id()) + || lang_items.fn_mut_trait() == Some(p.def_id()) + || lang_items.fn_once_trait() == Some(p.def_id())) + && p.self_ty() == ty => + { + Some(p.trait_ref.substs.type_at(1)) + }, + _ => None, + }) + .transpose() + }) + .map(ExprFnSig::Trait) + { + break Some(res); + } else if let Some(parent_id) = predicates.parent { + id = parent_id; + } else { + break None; + } + } + }, + _ => None, + } + }, + } +} + /// Peels off all references on the pattern. Returns the underlying pattern and the number of /// references removed. pub fn peel_hir_pat_refs(pat: &'a Pat<'a>) -> (&'a Pat<'a>, usize) { @@ -1751,6 +1880,19 @@ pub fn peel_n_hir_expr_refs(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>, f(expr, 0, count) } +/// Peels off all references on the type. Returns the underlying type and the number of references +/// removed. +pub fn peel_hir_ty_refs(ty: &'tcx hir::Ty<'_>) -> (&'tcx hir::Ty<'tcx>, usize) { + fn peel(ty: &'tcx hir::Ty<'_>, count: usize) -> (&'tcx hir::Ty<'tcx>, usize) { + if let TyKind::Rptr(_, ty) = &ty.kind { + peel(ty.ty, count + 1) + } else { + (ty, count) + } + } + peel(ty, 0) +} + /// Peels off all references on the expression. Returns the underlying expression and the number of /// references removed. pub fn peel_hir_expr_refs(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) { diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index e61786796475..c2db23ed884c 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -40,6 +40,8 @@ pub const F32_EPSILON: [&str; 4] = ["core", "f32", "", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "", "EPSILON"]; pub const FILE: [&str; 3] = ["std", "fs", "File"]; pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; +pub const FORMAT_ARGS_MACRO: [&str; 4] = ["core", "macros", "builtin", "format_args"]; +pub const FORMAT_ARGS_NL_MACRO: [&str; 4] = ["core", "macros", "builtin", "format_args_nl"]; pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; diff --git a/tests/ui/borrow_interior_mutable_const/others.rs b/tests/ui/borrow_interior_mutable_const/others.rs index ea25729d11d4..4327f12c37c8 100644 --- a/tests/ui/borrow_interior_mutable_const/others.rs +++ b/tests/ui/borrow_interior_mutable_const/others.rs @@ -1,5 +1,9 @@ #![warn(clippy::borrow_interior_mutable_const)] -#![allow(clippy::declare_interior_mutable_const, clippy::ref_in_deref)] +#![allow( + clippy::declare_interior_mutable_const, + clippy::ref_in_deref, + clippy::needless_borrow +)] #![allow(const_item_mutation)] use std::borrow::Cow; diff --git a/tests/ui/borrow_interior_mutable_const/others.stderr b/tests/ui/borrow_interior_mutable_const/others.stderr index 9a908cf30e94..f146b97cf611 100644 --- a/tests/ui/borrow_interior_mutable_const/others.stderr +++ b/tests/ui/borrow_interior_mutable_const/others.stderr @@ -1,5 +1,5 @@ error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:54:5 + --> $DIR/others.rs:58:5 | LL | ATOMIC.store(1, Ordering::SeqCst); //~ ERROR interior mutability | ^^^^^^ @@ -8,7 +8,7 @@ LL | ATOMIC.store(1, Ordering::SeqCst); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:55:16 + --> $DIR/others.rs:59:16 | LL | assert_eq!(ATOMIC.load(Ordering::SeqCst), 5); //~ ERROR interior mutability | ^^^^^^ @@ -16,7 +16,7 @@ LL | assert_eq!(ATOMIC.load(Ordering::SeqCst), 5); //~ ERROR interior mutabi = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:58:22 + --> $DIR/others.rs:62:22 | LL | let _once_ref = &ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -24,7 +24,7 @@ LL | let _once_ref = &ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:59:25 + --> $DIR/others.rs:63:25 | LL | let _once_ref_2 = &&ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -32,7 +32,7 @@ LL | let _once_ref_2 = &&ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:60:27 + --> $DIR/others.rs:64:27 | LL | let _once_ref_4 = &&&&ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -40,7 +40,7 @@ LL | let _once_ref_4 = &&&&ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:61:26 + --> $DIR/others.rs:65:26 | LL | let _once_mut = &mut ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -48,7 +48,7 @@ LL | let _once_mut = &mut ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:72:14 + --> $DIR/others.rs:76:14 | LL | let _ = &ATOMIC_TUPLE; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -56,7 +56,7 @@ LL | let _ = &ATOMIC_TUPLE; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:73:14 + --> $DIR/others.rs:77:14 | LL | let _ = &ATOMIC_TUPLE.0; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -64,7 +64,7 @@ LL | let _ = &ATOMIC_TUPLE.0; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:74:19 + --> $DIR/others.rs:78:19 | LL | let _ = &(&&&&ATOMIC_TUPLE).0; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | let _ = &(&&&&ATOMIC_TUPLE).0; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:75:14 + --> $DIR/others.rs:79:14 | LL | let _ = &ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | let _ = &ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:76:13 + --> $DIR/others.rs:80:13 | LL | let _ = ATOMIC_TUPLE.0[0].load(Ordering::SeqCst); //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = ATOMIC_TUPLE.0[0].load(Ordering::SeqCst); //~ ERROR interior mu = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:82:13 + --> $DIR/others.rs:86:13 | LL | let _ = ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -96,7 +96,7 @@ LL | let _ = ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:87:5 + --> $DIR/others.rs:91:5 | LL | CELL.set(2); //~ ERROR interior mutability | ^^^^ @@ -104,7 +104,7 @@ LL | CELL.set(2); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a `const` item with interior mutability should not be borrowed - --> $DIR/others.rs:88:16 + --> $DIR/others.rs:92:16 | LL | assert_eq!(CELL.get(), 6); //~ ERROR interior mutability | ^^^^ diff --git a/tests/ui/collapsible_match2.rs b/tests/ui/collapsible_match2.rs index 8372a2124773..542e69484276 100644 --- a/tests/ui/collapsible_match2.rs +++ b/tests/ui/collapsible_match2.rs @@ -1,5 +1,10 @@ #![warn(clippy::collapsible_match)] -#![allow(clippy::needless_return, clippy::no_effect, clippy::single_match)] +#![allow( + clippy::needless_return, + clippy::no_effect, + clippy::single_match, + clippy::needless_borrow +)] fn lint_cases(opt_opt: Option>, res_opt: Result, String>) { // if guards on outer match diff --git a/tests/ui/collapsible_match2.stderr b/tests/ui/collapsible_match2.stderr index c8a445ef369d..ffef32d1fde9 100644 --- a/tests/ui/collapsible_match2.stderr +++ b/tests/ui/collapsible_match2.stderr @@ -1,5 +1,5 @@ error: unnecessary nested match - --> $DIR/collapsible_match2.rs:8:34 + --> $DIR/collapsible_match2.rs:13:34 | LL | Ok(val) if make() => match val { | __________________________________^ @@ -10,7 +10,7 @@ LL | | }, | = note: `-D clippy::collapsible-match` implied by `-D warnings` help: the outer pattern can be modified to include the inner pattern - --> $DIR/collapsible_match2.rs:8:16 + --> $DIR/collapsible_match2.rs:13:16 | LL | Ok(val) if make() => match val { | ^^^ replace this binding @@ -18,7 +18,7 @@ LL | Some(n) => foo(n), | ^^^^^^^ with this pattern error: unnecessary nested match - --> $DIR/collapsible_match2.rs:15:24 + --> $DIR/collapsible_match2.rs:20:24 | LL | Ok(val) => match val { | ________________________^ @@ -28,7 +28,7 @@ LL | | }, | |_____________^ | help: the outer pattern can be modified to include the inner pattern - --> $DIR/collapsible_match2.rs:15:16 + --> $DIR/collapsible_match2.rs:20:16 | LL | Ok(val) => match val { | ^^^ replace this binding @@ -36,7 +36,7 @@ LL | Some(n) => foo(n), | ^^^^^^^ with this pattern error: unnecessary nested match - --> $DIR/collapsible_match2.rs:29:29 + --> $DIR/collapsible_match2.rs:34:29 | LL | $pat => match $e { | _____________________________^ @@ -49,7 +49,7 @@ LL | mac!(res_opt => Ok(val), val => Some(n), foo(n)); | ------------------------------------------------- in this macro invocation | help: the outer pattern can be modified to include the inner pattern - --> $DIR/collapsible_match2.rs:41:28 + --> $DIR/collapsible_match2.rs:46:28 | LL | mac!(res_opt => Ok(val), val => Some(n), foo(n)); | ^^^ ^^^^^^^ with this pattern @@ -58,7 +58,7 @@ LL | mac!(res_opt => Ok(val), val => Some(n), foo(n)); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: unnecessary nested match - --> $DIR/collapsible_match2.rs:46:20 + --> $DIR/collapsible_match2.rs:51:20 | LL | Some(s) => match *s { | ____________________^ @@ -68,7 +68,7 @@ LL | | }, | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> $DIR/collapsible_match2.rs:46:14 + --> $DIR/collapsible_match2.rs:51:14 | LL | Some(s) => match *s { | ^ replace this binding @@ -76,7 +76,7 @@ LL | [n] => foo(n), | ^^^ with this pattern error: unnecessary nested match - --> $DIR/collapsible_match2.rs:55:24 + --> $DIR/collapsible_match2.rs:60:24 | LL | Some(ref s) => match &*s { | ________________________^ @@ -86,7 +86,7 @@ LL | | }, | |_________^ | help: the outer pattern can be modified to include the inner pattern - --> $DIR/collapsible_match2.rs:55:14 + --> $DIR/collapsible_match2.rs:60:14 | LL | Some(ref s) => match &*s { | ^^^^^ replace this binding diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs index d26f48fc68f8..13e2b6c7a2e7 100644 --- a/tests/ui/escape_analysis.rs +++ b/tests/ui/escape_analysis.rs @@ -101,7 +101,7 @@ fn warn_match() { let x = box A; match &x { // not moved - ref y => (), + y => (), } } @@ -111,7 +111,7 @@ fn nowarn_large_array() { let x = box [1; 10000]; match &x { // not moved - ref y => (), + y => (), } } diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed new file mode 100644 index 000000000000..12c7ae065e0b --- /dev/null +++ b/tests/ui/explicit_auto_deref.fixed @@ -0,0 +1,88 @@ +// run-rustfix + +#![allow( + unused_imports, + dead_code, + clippy::borrowed_box, + clippy::deref_addrof, + clippy::useless_vec +)] +#![warn(clippy::explicit_auto_deref)] + +use std::{ + ops::{Deref, DerefMut}, + path::{Path, PathBuf}, +}; + +fn main() { + let _: &str = &String::new(); + let _: &Path = &PathBuf::new(); + let _: &[_] = &vec![0u32, 1u32, 2u32]; + + fn f2(_: &[T]) {} + f2(&vec![0u32]); + + fn f3() -> &'static str { + static S: String = String::new(); + &S + } + + fn f4(_: &(T1, T2)) {} + f4(&Box::new((0u32, 0i32))); + + let f5 = |_: &str| {}; + f5(&String::new()); + + fn f6(f: impl Fn(&str)) { + f(&String::new()); + } + + fn f7(f: &dyn Fn(&str)) { + f(&String::new()); + } + + let _: &Box<_> = &*Box::new(Box::new(0)); + let _: Box = *Box::new(Box::new(0)); + + fn f8(_: &T) {} + f8(&*String::new()); + + struct S1(T); + impl S1 { + fn f(&self, f: impl Fn(&str)) { + f(&String::new()); + (self.0)(&String::new()); + } + } + + fn f9(f: &mut T) + where + T: Iterator, + ::Item: Fn(&str), + { + f.next().unwrap()(&String::new()) + } + + struct S2<'a> { + x: &'a str, + } + let _ = S2 { x: &String::new() }; + + struct S3<'a>(&'a str); + let _ = S3(&String::new()); + + macro_rules! m1 { + ($e:expr) => {{ + fn f(_: &str) {} + f($e); + }}; + } + m1!(&*String::new()); + + macro_rules! m2 { + ($e:expr) => { + &$e + }; + } + let _: &str = m2!(String::new()); +} diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs new file mode 100644 index 000000000000..9ce133e897a2 --- /dev/null +++ b/tests/ui/explicit_auto_deref.rs @@ -0,0 +1,88 @@ +// run-rustfix + +#![allow( + unused_imports, + dead_code, + clippy::borrowed_box, + clippy::deref_addrof, + clippy::useless_vec +)] +#![warn(clippy::explicit_auto_deref)] + +use std::{ + ops::{Deref, DerefMut}, + path::{Path, PathBuf}, +}; + +fn main() { + let _: &str = &*String::new(); + let _: &Path = &*PathBuf::new(); + let _: &[_] = vec![0u32, 1u32, 2u32].deref(); + + fn f2(_: &[T]) {} + f2(vec![0u32].deref()); + + fn f3() -> &'static str { + static S: String = String::new(); + &*S + } + + fn f4(_: &(T1, T2)) {} + f4(Box::new((0u32, 0i32)).deref()); + + let f5 = |_: &str| {}; + f5(&*String::new()); + + fn f6(f: impl Fn(&str)) { + f(&*String::new()); + } + + fn f7(f: &dyn Fn(&str)) { + f(&*String::new()); + } + + let _: &Box<_> = &*Box::new(Box::new(0)); + let _: Box = *Box::new(Box::new(0)); + + fn f8(_: &T) {} + f8(&*String::new()); + + struct S1(T); + impl S1 { + fn f(&self, f: impl Fn(&str)) { + f(&*String::new()); + (self.0)(&*String::new()); + } + } + + fn f9(f: &mut T) + where + T: Iterator, + ::Item: Fn(&str), + { + f.next().unwrap()(&*String::new()) + } + + struct S2<'a> { + x: &'a str, + } + let _ = S2 { x: &*String::new() }; + + struct S3<'a>(&'a str); + let _ = S3(&*String::new()); + + macro_rules! m1 { + ($e:expr) => {{ + fn f(_: &str) {} + f($e); + }}; + } + m1!(&*String::new()); + + macro_rules! m2 { + ($e:expr) => { + &$e + }; + } + let _: &str = &**m2!(String::new()); +} diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr new file mode 100644 index 000000000000..1aef045ae92e --- /dev/null +++ b/tests/ui/explicit_auto_deref.stderr @@ -0,0 +1,94 @@ +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:18:20 + | +LL | let _: &str = &*String::new(); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + | + = note: `-D clippy::explicit-auto-deref` implied by `-D warnings` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:19:21 + | +LL | let _: &Path = &*PathBuf::new(); + | ^^^^^^^^^^^^^^^ help: try this: `PathBuf::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:20:19 + | +LL | let _: &[_] = vec![0u32, 1u32, 2u32].deref(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&vec![0u32, 1u32, 2u32]` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:23:8 + | +LL | f2(vec![0u32].deref()); + | ^^^^^^^^^^^^^^^^^^ help: try this: `&vec![0u32]` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:27:10 + | +LL | &*S + | ^^ help: try this: `S` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:31:8 + | +LL | f4(Box::new((0u32, 0i32)).deref()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&Box::new((0u32, 0i32))` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:34:9 + | +LL | f5(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:37:12 + | +LL | f(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:41:12 + | +LL | f(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:53:16 + | +LL | f(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:54:23 + | +LL | (self.0)(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:63:28 + | +LL | f.next().unwrap()(&*String::new()) + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:69:22 + | +LL | let _ = S2 { x: &*String::new() }; + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:72:17 + | +LL | let _ = S3(&*String::new()); + | ^^^^^^^^^^^^^^ help: try this: `String::new()` + +error: explicit dereference in auto-deref context + --> $DIR/explicit_auto_deref.rs:87:19 + | +LL | let _: &str = &**m2!(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `m2!(String::new())` + +error: aborting due to 15 previous errors + diff --git a/tests/ui/dereference.fixed b/tests/ui/explicit_deref_methods.fixed similarity index 85% rename from tests/ui/dereference.fixed rename to tests/ui/explicit_deref_methods.fixed index 459ca91b93b9..c1d8e42720fc 100644 --- a/tests/ui/dereference.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -1,6 +1,11 @@ // run-rustfix -#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![allow( + unused_variables, + clippy::many_single_char_names, + clippy::clone_double_ref, + clippy::explicit_auto_deref +)] #![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; @@ -29,7 +34,7 @@ fn main() { let b: &str = &*a; - let b: &mut str = &mut *a; + let b: &mut str = &mut **a; // both derefs should get linted here let b: String = format!("{}, {}", &*a, &*a); @@ -43,11 +48,11 @@ fn main() { let b: String = concat(&*a); - let b = &*just_return(a); + let b = just_return(a); - let b: String = concat(&*just_return(a)); + let b: String = concat(just_return(a)); - let b: &str = &*a.deref(); + let b: &str = &**a; let opt_a = Some(a.clone()); let b = &*opt_a.unwrap(); diff --git a/tests/ui/dereference.rs b/tests/ui/explicit_deref_methods.rs similarity index 93% rename from tests/ui/dereference.rs rename to tests/ui/explicit_deref_methods.rs index 8dc5272e67fa..49e83cbc64b1 100644 --- a/tests/ui/dereference.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -1,6 +1,11 @@ // run-rustfix -#![allow(unused_variables, clippy::many_single_char_names, clippy::clone_double_ref)] +#![allow( + unused_variables, + clippy::many_single_char_names, + clippy::clone_double_ref, + clippy::explicit_auto_deref +)] #![warn(clippy::explicit_deref_methods)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/dereference.stderr b/tests/ui/explicit_deref_methods.stderr similarity index 55% rename from tests/ui/dereference.stderr rename to tests/ui/explicit_deref_methods.stderr index d26b462a4336..b0f7c80200f7 100644 --- a/tests/ui/dereference.stderr +++ b/tests/ui/explicit_deref_methods.stderr @@ -1,67 +1,67 @@ -error: explicit deref method call - --> $DIR/dereference.rs:30:19 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:35:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` | = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` -error: explicit deref_mut method call - --> $DIR/dereference.rs:32:23 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:37:23 | LL | let b: &mut str = a.deref_mut(); - | ^^^^^^^^^^^^^ help: try this: `&mut *a` + | ^^^^^^^^^^^^^ help: try this: `&mut **a` -error: explicit deref method call - --> $DIR/dereference.rs:35:39 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:40:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: explicit deref method call - --> $DIR/dereference.rs:35:50 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:40:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: explicit deref method call - --> $DIR/dereference.rs:37:20 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:42:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: explicit deref method call - --> $DIR/dereference.rs:40:11 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:45:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` -error: explicit deref method call - --> $DIR/dereference.rs:44:28 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:49:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: explicit deref method call - --> $DIR/dereference.rs:46:13 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:51:13 | LL | let b = just_return(a).deref(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` -error: explicit deref method call - --> $DIR/dereference.rs:48:28 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:53:28 | LL | let b: String = concat(just_return(a).deref()); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*just_return(a)` + | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` -error: explicit deref method call - --> $DIR/dereference.rs:50:19 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:55:19 | LL | let b: &str = a.deref().deref(); - | ^^^^^^^^^^^^^^^^^ help: try this: `&*a.deref()` + | ^^^^^^^^^^^^^^^^^ help: try this: `&**a` -error: explicit deref method call - --> $DIR/dereference.rs:53:13 +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:58:13 | LL | let b = opt_a.unwrap().deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs index 19101522163f..cef71cf79d79 100644 --- a/tests/ui/implicit_clone.rs +++ b/tests/ui/implicit_clone.rs @@ -66,7 +66,7 @@ fn main() { let _ = vec.to_vec(); let vec_ref = &vec; - let _ = return_owned_from_slice(&vec_ref); + let _ = return_owned_from_slice(vec_ref); let _ = vec_ref.to_owned(); let _ = vec_ref.to_vec(); diff --git a/tests/ui/into_iter_on_ref.fixed b/tests/ui/into_iter_on_ref.fixed index 7f92d0dbdc97..b77f17944d89 100644 --- a/tests/ui/into_iter_on_ref.fixed +++ b/tests/ui/into_iter_on_ref.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::needless_borrow)] #![warn(clippy::into_iter_on_ref)] struct X; diff --git a/tests/ui/into_iter_on_ref.rs b/tests/ui/into_iter_on_ref.rs index 416056d3fdb9..3854bb05af8f 100644 --- a/tests/ui/into_iter_on_ref.rs +++ b/tests/ui/into_iter_on_ref.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::needless_borrow)] #![warn(clippy::into_iter_on_ref)] struct X; diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 5ae4a0e79b99..164c6f5bb972 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -19,15 +19,28 @@ fn main() { let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` h(&"foo"); // should not error, because the `&&str` is required, due to `&Trait` if let Some(cake) = Some(&5) {} + let ref_a = &a; let garbl = match 42 { 44 => &a, 45 => { println!("foo"); - &&a // FIXME: this should lint, too + &a }, 46 => &a, + #[allow(clippy::never_loop)] + 47 => loop { + break ref_a; + }, + 48 => + { + #[allow(clippy::never_loop)] + loop { + break ref_a; + } + }, _ => panic!(), }; + x(&Box::new(0)); // shouldn't error. } fn f(y: &T) -> T { diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 1e281316c8a3..fe85cffc6654 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -19,15 +19,28 @@ fn main() { let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` h(&"foo"); // should not error, because the `&&str` is required, due to `&Trait` if let Some(ref cake) = Some(&5) {} + let ref_a = &a; let garbl = match 42 { 44 => &a, 45 => { println!("foo"); - &&a // FIXME: this should lint, too + &&a }, 46 => &&a, + #[allow(clippy::never_loop)] + 47 => loop { + break &ref_a; + }, + 48 => + { + #[allow(clippy::never_loop)] + loop { + break &ref_a; + } + }, _ => panic!(), }; + x(&Box::new(0)); // shouldn't error. } fn f(y: &T) -> T { diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index bea4b41b803d..b5acff003f0e 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -13,16 +13,34 @@ LL | if let Some(ref cake) = Some(&5) {} | ^^^^^^^^ help: change this to: `cake` error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:28:15 + --> $DIR/needless_borrow.rs:27:13 + | +LL | &&a + | ^^^ help: change this to: `&a` + +error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:29:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` +error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:32:19 + | +LL | break &ref_a; + | ^^^^^^ help: change this to: `ref_a` + +error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:38:23 + | +LL | break &ref_a; + | ^^^^^^ help: change this to: `ref_a` + error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:51:31 + --> $DIR/needless_borrow.rs:64:31 | LL | let _ = v.iter().filter(|&ref a| a.is_empty()); | ^^^^^ help: change this to: `a` -error: aborting due to 4 previous errors +error: aborting due to 7 previous errors diff --git a/tests/ui/ref_option_ref.rs b/tests/ui/ref_option_ref.rs index b2c275d68afa..2df45c927d71 100644 --- a/tests/ui/ref_option_ref.rs +++ b/tests/ui/ref_option_ref.rs @@ -9,7 +9,7 @@ static THRESHOLD: i32 = 10; static REF_THRESHOLD: &Option<&i32> = &Some(&THRESHOLD); const CONST_THRESHOLD: &i32 = &10; -const REF_CONST: &Option<&i32> = &Some(&CONST_THRESHOLD); +const REF_CONST: &Option<&i32> = &Some(CONST_THRESHOLD); type RefOptRefU32<'a> = &'a Option<&'a u32>; type RefOptRef<'a, T> = &'a Option<&'a T>; diff --git a/tests/ui/ref_option_ref.stderr b/tests/ui/ref_option_ref.stderr index 4e7fc8000611..b61334758e85 100644 --- a/tests/ui/ref_option_ref.stderr +++ b/tests/ui/ref_option_ref.stderr @@ -9,7 +9,7 @@ LL | static REF_THRESHOLD: &Option<&i32> = &Some(&THRESHOLD); error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` --> $DIR/ref_option_ref.rs:12:18 | -LL | const REF_CONST: &Option<&i32> = &Some(&CONST_THRESHOLD); +LL | const REF_CONST: &Option<&i32> = &Some(CONST_THRESHOLD); | ^^^^^^^^^^^^^ help: try: `Option<&i32>` error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` diff --git a/tests/ui/stable_sort_primitive.fixed b/tests/ui/stable_sort_primitive.fixed index 8f8f56659315..f5f18169df29 100644 --- a/tests/ui/stable_sort_primitive.fixed +++ b/tests/ui/stable_sort_primitive.fixed @@ -20,7 +20,7 @@ fn main() { // Negative examples: behavior changes if made unstable let mut vec = vec![1, 3, 2]; vec.sort_by_key(|i| i / 2); - vec.sort_by(|a, b| (a + b).cmp(&b)); + vec.sort_by(|&a, &b| (a + b).cmp(&b)); // negative examples - Not of a primitive type let mut vec_of_complex = vec![String::from("hello"), String::from("world!")]; vec_of_complex.sort(); diff --git a/tests/ui/stable_sort_primitive.rs b/tests/ui/stable_sort_primitive.rs index f9bd97790671..8149c5638e0f 100644 --- a/tests/ui/stable_sort_primitive.rs +++ b/tests/ui/stable_sort_primitive.rs @@ -20,7 +20,7 @@ fn main() { // Negative examples: behavior changes if made unstable let mut vec = vec![1, 3, 2]; vec.sort_by_key(|i| i / 2); - vec.sort_by(|a, b| (a + b).cmp(&b)); + vec.sort_by(|&a, &b| (a + b).cmp(&b)); // negative examples - Not of a primitive type let mut vec_of_complex = vec![String::from("hello"), String::from("world!")]; vec_of_complex.sort(); diff --git a/tests/ui/useless_asref.fixed b/tests/ui/useless_asref.fixed index e356f13d087b..28542d22339f 100644 --- a/tests/ui/useless_asref.fixed +++ b/tests/ui/useless_asref.fixed @@ -39,7 +39,7 @@ fn not_ok() { let mut mrslice: &mut [i32] = &mut [1, 2, 3]; { - let rslice: &[i32] = &*mrslice; + let rslice: &[i32] = mrslice; foo_rstr(rstr); foo_rstr(rstr); foo_rslice(rslice); diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs index 2a80291f5d83..fc9af069bff7 100644 --- a/tests/ui/useless_asref.rs +++ b/tests/ui/useless_asref.rs @@ -39,7 +39,7 @@ fn not_ok() { let mut mrslice: &mut [i32] = &mut [1, 2, 3]; { - let rslice: &[i32] = &*mrslice; + let rslice: &[i32] = mrslice; foo_rstr(rstr.as_ref()); foo_rstr(rstr); foo_rslice(rslice.as_ref());