diff --git a/CHANGELOG.md b/CHANGELOG.md index bc50ff3a44a8..9bc93c1cb42c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3400,6 +3400,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/update_lints.rs b/clippy_dev/src/update_lints.rs index 115f5f0064fe..2e0659f42d7b 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -413,7 +413,7 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec) -> io .find("declare_lint_pass!") .unwrap_or_else(|| panic!("failed to find `impl_lint_pass`")) }); - let mut impl_lint_pass_end = (&content[impl_lint_pass_start..]) + let mut impl_lint_pass_end = content[impl_lint_pass_start..] .find(']') .expect("failed to find `impl_lint_pass` terminator"); diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index b47441eff374..59dcc1ebf191 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,20 +1,24 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; -use clippy_utils::ty::peel_mid_ty_refs; -use clippy_utils::{get_parent_expr, get_parent_node, is_lint_allowed, path_to_local}; +use clippy_utils::ty::{expr_sig, peel_mid_ty_refs, variant_of_res}; +use clippy_utils::{get_parent_expr, is_lint_allowed, path_to_local, walk_to_expr_usage}; use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX}; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; +use rustc_hir::intravisit::{walk_ty, Visitor}; use rustc_hir::{ - BindingAnnotation, Body, BodyId, BorrowKind, Destination, Expr, ExprKind, HirId, MatchSource, Mutability, Node, - Pat, PatKind, UnOp, + self as hir, BindingAnnotation, Body, BodyId, BorrowKind, Expr, ExprKind, GenericArg, HirId, ImplItem, + ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem, + TraitItemKind, TyKind, UnOp, }; +use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; -use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeckResults}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{symbol::sym, Span}; +use rustc_span::{symbol::sym, Span, Symbol}; +use rustc_trait_selection::infer::InferCtxtExt; declare_clippy_lint! { /// ### What it does @@ -104,10 +108,34 @@ declare_clippy_lint! { "`ref` binding to a reference" } +declare_clippy_lint! { + /// ### What it does + /// Checks for dereferencing expressions which would be covered by auto-deref. + /// + /// ### Why is this bad? + /// This unnecessarily complicates the code. + /// + /// ### Example + /// ```rust + /// let x = String::new(); + /// let y: &str = &*x; + /// ``` + /// Use instead: + /// ```rust + /// let x = String::new(); + /// let y: &str = &x; + /// ``` + #[clippy::version = "1.60.0"] + pub EXPLICIT_AUTO_DEREF, + complexity, + "dereferencing when the compiler would automatically dereference" +} + impl_lint_pass!(Dereferencing => [ EXPLICIT_DEREF_METHODS, NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE, + EXPLICIT_AUTO_DEREF, ]); #[derive(Default)] @@ -136,6 +164,12 @@ struct StateData { /// Span of the top level expression span: Span, hir_id: HirId, + position: Position, +} + +struct DerefedBorrow { + count: usize, + msg: &'static str, } enum State { @@ -147,11 +181,19 @@ enum State { /// The required mutability target_mut: Mutability, }, - DerefedBorrow { - count: usize, - required_precedence: i8, - msg: &'static str, + DerefedBorrow(DerefedBorrow), + ExplicitDeref { + // Span and id of the top-level deref expression if the parent expression is a borrow. + deref_span_id: Option<(Span, HirId)>, + }, + ExplicitDerefField { + name: Symbol, }, + Reborrow { + deref_span: Span, + deref_hir_id: HirId, + }, + Borrow, } // A reference operation considered by this lint pass @@ -207,13 +249,28 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { match (self.state.take(), kind) { (None, kind) => { - let parent = get_parent_node(cx.tcx, expr.hir_id); let expr_ty = typeck.expr_ty(expr); + let (position, adjustments) = walk_parents(cx, expr); match kind { + RefOp::Deref => { + if let Position::FieldAccess(name) = position + && !ty_contains_field(typeck.expr_ty(sub_expr), name) + { + self.state = Some(( + State::ExplicitDerefField { name }, + StateData { span: expr.span, hir_id: expr.hir_id, position }, + )); + } else if position.is_deref_stable() { + self.state = Some(( + State::ExplicitDeref { deref_span_id: None }, + StateData { span: expr.span, hir_id: expr.hir_id, position }, + )); + } + } RefOp::Method(target_mut) if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) - && is_linted_explicit_deref_position(parent, expr.hir_id, expr.span) => + && position.lint_explicit_deref() => { self.state = Some(( State::DerefMethod { @@ -228,12 +285,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { StateData { span: expr.span, hir_id: expr.hir_id, + position }, )); }, RefOp::AddrOf => { // Find the number of times the borrow is auto-derefed. - let mut iter = find_adjustments(cx.tcx, typeck, expr).iter(); + let mut iter = adjustments.iter(); let mut deref_count = 0usize; let next_adjust = loop { match iter.next() { @@ -274,40 +332,43 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { "this expression creates a reference which is immediately dereferenced by the compiler"; let borrow_msg = "this expression borrows a value the compiler would automatically borrow"; - let (required_refs, required_precedence, msg) = if is_auto_borrow_position(parent, expr.hir_id) - { - (1, PREC_POSTFIX, if deref_count == 1 { borrow_msg } else { deref_msg }) + let (required_refs, msg) = if position.can_auto_borrow() { + (1, if deref_count == 1 { borrow_msg } else { deref_msg }) } else if let Some(&Adjust::Borrow(AutoBorrow::Ref(_, mutability))) = next_adjust.map(|a| &a.kind) { - if matches!(mutability, AutoBorrowMutability::Mut { .. }) - && !is_auto_reborrow_position(parent) + if matches!(mutability, AutoBorrowMutability::Mut { .. }) && !position.is_reborrow_stable() { - (3, 0, deref_msg) + (3, deref_msg) } else { - (2, 0, deref_msg) + (2, deref_msg) } } else { - (2, 0, deref_msg) + (2, deref_msg) }; if deref_count >= required_refs { self.state = Some(( - State::DerefedBorrow { + State::DerefedBorrow(DerefedBorrow { // One of the required refs is for the current borrow expression, the remaining ones // can't be removed without breaking the code. See earlier comment. count: deref_count - required_refs, - required_precedence, msg, - }, + }), + StateData { span: expr.span, hir_id: expr.hir_id, position }, + )); + } else if position.is_deref_stable() { + self.state = Some(( + State::Borrow, StateData { span: expr.span, hir_id: expr.hir_id, + position }, )); } }, - _ => (), + RefOp::Method(..) => (), } }, ( @@ -334,26 +395,90 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { data, )); }, + (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) if state.count != 0 => { + self.state = Some(( + State::DerefedBorrow(DerefedBorrow { + count: state.count - 1, + ..state + }), + data, + )); + }, + (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) => { + let position = data.position; + report(cx, expr, State::DerefedBorrow(state), data); + if position.is_deref_stable() { + self.state = Some(( + State::Borrow, + StateData { + span: expr.span, + hir_id: expr.hir_id, + position, + }, + )); + } + }, + (Some((State::DerefedBorrow(state), data)), RefOp::Deref) => { + let position = data.position; + report(cx, expr, State::DerefedBorrow(state), data); + if let Position::FieldAccess(name) = position + && !ty_contains_field(typeck.expr_ty(sub_expr), name) + { + self.state = Some(( + State::ExplicitDerefField { name }, + StateData { span: expr.span, hir_id: expr.hir_id, position }, + )); + } else if position.is_deref_stable() { + self.state = Some(( + State::ExplicitDeref { deref_span_id: None }, + StateData { span: expr.span, hir_id: expr.hir_id, position }, + )); + } + }, + + (Some((State::Borrow, data)), RefOp::Deref) => { + if typeck.expr_ty(sub_expr).is_ref() { + self.state = Some(( + State::Reborrow { + deref_span: expr.span, + deref_hir_id: expr.hir_id, + }, + data, + )); + } else { + self.state = Some(( + State::ExplicitDeref { + deref_span_id: Some((expr.span, expr.hir_id)), + }, + data, + )); + } + }, ( Some(( - State::DerefedBorrow { - count, - required_precedence, - msg, + State::Reborrow { + deref_span, + deref_hir_id, }, data, )), - RefOp::AddrOf, - ) if count != 0 => { + RefOp::Deref, + ) => { self.state = Some(( - State::DerefedBorrow { - count: count - 1, - required_precedence, - msg, + State::ExplicitDeref { + deref_span_id: Some((deref_span, deref_hir_id)), }, data, )); }, + (state @ Some((State::ExplicitDeref { .. }, _)), RefOp::Deref) => { + self.state = state; + }, + (Some((State::ExplicitDerefField { name }, data)), RefOp::Deref) + if !ty_contains_field(typeck.expr_ty(sub_expr), name) => + { + self.state = Some((State::ExplicitDerefField { name }, data)); + }, (Some((state, data)), _) => report(cx, expr, state, data), } @@ -473,131 +598,362 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { } } -// 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, child_span: Span) -> bool { - let parent = match parent { - Some(Node::Expr(e)) if e.span.ctxt() == child_span.ctxt() => 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, - - // Makes an ugly suggestion - // e.g. *x.deref() => *&*x - ExprKind::Unary(UnOp::Deref, _) - // Postfix expressions would require parens - | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) - | 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::Let(..) - | ExprKind::Closure{..} - | ExprKind::Block(..) - | ExprKind::Assign(..) - | ExprKind::AssignOp(..) - | ExprKind::Path(..) - | ExprKind::AddrOf(..) - | ExprKind::Break(..) - | ExprKind::Continue(..) - | ExprKind::Ret(..) - | ExprKind::InlineAsm(..) - | ExprKind::Struct(..) - | ExprKind::Repeat(..) - | ExprKind::Yield(..) => true, - } +/// The position of an expression relative to it's parent. +#[derive(Clone, Copy)] +enum Position { + MethodReceiver, + /// The method is defined on a reference type. e.g. `impl Foo for &T` + MethodReceiverRefImpl, + Callee, + FieldAccess(Symbol), + Postfix, + Deref, + /// Any other location which will trigger auto-deref to a specific time. + DerefStable(i8), + /// Any other location which will trigger auto-reborrowing. + ReborrowStable(i8), + Other(i8), } +impl Position { + fn is_deref_stable(self) -> bool { + matches!(self, Self::DerefStable(_)) + } -/// Checks if the given expression is in a position which can be auto-reborrowed. -/// Note: This is only correct assuming auto-deref is already occurring. -fn is_auto_reborrow_position(parent: Option>) -> bool { - match parent { - Some(Node::Expr(parent)) => matches!(parent.kind, ExprKind::MethodCall(..) | ExprKind::Call(..)), - Some(Node::Local(_)) => true, - _ => false, + fn is_reborrow_stable(self) -> bool { + matches!(self, Self::DerefStable(_) | Self::ReborrowStable(_)) } -} -/// Checks if the given expression is a position which can auto-borrow. -fn is_auto_borrow_position(parent: Option>, child_id: HirId) -> bool { - if let Some(Node::Expr(parent)) = parent { - match parent.kind { - // ExprKind::MethodCall(_, [self_arg, ..], _) => self_arg.hir_id == child_id, - ExprKind::Field(..) => true, - ExprKind::Call(f, _) => f.hir_id == child_id, - _ => false, - } - } else { - false + fn can_auto_borrow(self) -> bool { + matches!(self, Self::MethodReceiver | Self::FieldAccess(_) | Self::Callee) } -} -/// Adjustments are sometimes made in the parent block rather than the expression itself. -fn find_adjustments<'tcx>( - tcx: TyCtxt<'tcx>, - typeck: &'tcx TypeckResults<'tcx>, - expr: &'tcx Expr<'tcx>, -) -> &'tcx [Adjustment<'tcx>] { - let map = tcx.hir(); - let mut iter = map.parent_iter(expr.hir_id); - let mut prev = expr; + fn lint_explicit_deref(self) -> bool { + matches!(self, Self::Other(_) | Self::DerefStable(_) | Self::ReborrowStable(_)) + } - loop { - match typeck.expr_adjustments(prev) { - [] => (), - a => break a, - }; + fn precedence(self) -> i8 { + match self { + Self::MethodReceiver + | Self::MethodReceiverRefImpl + | Self::Callee + | Self::FieldAccess(_) + | Self::Postfix => PREC_POSTFIX, + Self::Deref => PREC_PREFIX, + Self::DerefStable(p) | Self::ReborrowStable(p) | Self::Other(p) => p, + } + } +} - match iter.next().map(|(_, x)| x) { - Some(Node::Block(_)) => { - if let Some((_, Node::Expr(e))) = iter.next() { - prev = e; +/// Walks up the parent expressions attempting to determine both how stable the auto-deref result +/// is, and which adjustments will be applied to it. Note this will not consider auto-borrow +/// locations as those follow different rules. +#[allow(clippy::too_many_lines)] +fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &'tcx [Adjustment<'tcx>]) { + let mut adjustments = [].as_slice(); + let mut precedence = 0i8; + let ctxt = e.span.ctxt(); + let position = walk_to_expr_usage(cx, e, &mut |parent, child_id| { + // LocalTableInContext returns the wrong lifetime, so go use `expr_adjustments` instead. + if adjustments.is_empty() && let Node::Expr(e) = cx.tcx.hir().get(child_id) { + adjustments = cx.typeck_results().expr_adjustments(e); + } + match parent { + Node::Local(Local { ty: Some(ty), span, .. }) if span.ctxt() == ctxt => { + Some(binding_ty_auto_deref_stability(ty, precedence)) + }, + Node::Item(&Item { + kind: ItemKind::Static(..) | ItemKind::Const(..), + def_id, + span, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Const(..), + def_id, + span, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Const(..), + def_id, + span, + .. + }) if span.ctxt() == ctxt => { + let ty = cx.tcx.type_of(def_id); + Some(if ty.is_ref() { + Position::DerefStable(precedence) } else { - // This shouldn't happen. Blocks are always contained in an expression. - break &[]; - } + Position::Other(precedence) + }) }, - Some(Node::Expr(&Expr { - kind: ExprKind::Break(Destination { target_id: Ok(id), .. }, _), + + Node::Item(&Item { + kind: ItemKind::Fn(..), + def_id, + span, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Fn(..), + def_id, + span, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Fn(..), + def_id, + span, .. - })) => { - if let Some(Node::Expr(e)) = map.find(id) { - prev = e; - iter = map.parent_iter(id); + }) if span.ctxt() == ctxt => { + let output = cx.tcx.fn_sig(def_id.to_def_id()).skip_binder().output(); + Some(if !output.is_ref() { + Position::Other(precedence) + } else if output.has_placeholders() || output.has_opaque_types() { + Position::ReborrowStable(precedence) + } else { + Position::DerefStable(precedence) + }) + }, + + Node::Expr(parent) if parent.span.ctxt() == ctxt => match parent.kind { + ExprKind::Ret(_) => { + let output = cx + .tcx + .fn_sig(cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap())) + .skip_binder() + .output(); + Some(if !output.is_ref() { + Position::Other(precedence) + } else if output.has_placeholders() || output.has_opaque_types() { + Position::ReborrowStable(precedence) + } else { + Position::DerefStable(precedence) + }) + }, + ExprKind::Call(func, _) if func.hir_id == child_id => (child_id == e.hir_id).then(|| Position::Callee), + ExprKind::Call(func, args) => args + .iter() + .position(|arg| arg.hir_id == child_id) + .zip(expr_sig(cx, func)) + .and_then(|(i, sig)| sig.input_with_hir(i)) + .map(|(hir_ty, ty)| match hir_ty { + // Type inference for closures can depend on how they're called. Only go by the explicit + // types here. + Some(ty) => binding_ty_auto_deref_stability(ty, precedence), + None => param_auto_deref_stability(ty.skip_binder(), precedence), + }), + ExprKind::MethodCall(_, args, _) => { + let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap(); + args.iter().position(|arg| arg.hir_id == child_id).map(|i| { + if i == 0 { + // Check for calls to trait methods where the trait is implemented on a reference. + // Two cases need to be handled: + // * `self` methods on `&T` will never have auto-borrow + // * `&self` methods on `&T` can have auto-borrow, but `&self` methods on `T` will take + // priority. + if e.hir_id != child_id { + Position::ReborrowStable(precedence) + } else if let Some(trait_id) = cx.tcx.trait_of_item(id) + && let arg_ty = cx.tcx.erase_regions(cx.typeck_results().expr_ty_adjusted(e)) + && let ty::Ref(_, sub_ty, _) = *arg_ty.kind() + && let subs = cx.typeck_results().node_substs_opt(child_id).unwrap_or_else( + || cx.tcx.mk_substs([].iter()) + ) && let impl_ty = if cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref() { + // Trait methods taking `&self` + sub_ty + } else { + // Trait methods taking `self` + arg_ty + } && impl_ty.is_ref() + && cx.tcx.infer_ctxt().enter(|infcx| + infcx + .type_implements_trait(trait_id, impl_ty, subs, cx.param_env) + .must_apply_modulo_regions() + ) + { + Position::MethodReceiverRefImpl + } else { + Position::MethodReceiver + } + } else { + param_auto_deref_stability(cx.tcx.fn_sig(id).skip_binder().inputs()[i], precedence) + } + }) + }, + ExprKind::Struct(path, fields, _) => { + let variant = variant_of_res(cx, cx.qpath_res(path, parent.hir_id)); + fields + .iter() + .find(|f| f.expr.hir_id == child_id) + .zip(variant) + .and_then(|(field, variant)| variant.fields.iter().find(|f| f.name == field.ident.name)) + .map(|field| param_auto_deref_stability(cx.tcx.type_of(field.did), precedence)) + }, + ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(Position::FieldAccess(name.name)), + ExprKind::Unary(UnOp::Deref, child) if child.hir_id == e.hir_id => Some(Position::Deref), + ExprKind::Match(child, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) + | ExprKind::Index(child, _) + if child.hir_id == e.hir_id => + { + Some(Position::Postfix) + }, + _ if child_id == e.hir_id => { + precedence = parent.precedence().order(); + None + }, + _ => None, + }, + _ => None, + } + }) + .unwrap_or(Position::Other(precedence)); + (position, adjustments) +} + +// Checks the stability of auto-deref when assigned to a binding with the given explicit type. +// +// 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 binding_ty_auto_deref_stability(ty: &hir::Ty<'_>, precedence: i8) -> Position { + let TyKind::Rptr(_, ty) = &ty.kind else { + return Position::Other(precedence); + }; + let mut ty = ty; + + loop { + break match ty.ty.kind { + TyKind::Rptr(_, ref ref_ty) => { + ty = ref_ty; + continue; + }, + TyKind::Path( + QPath::TypeRelative(_, path) + | QPath::Resolved( + _, + Path { + segments: [.., path], .. + }, + ), + ) => { + if let Some(args) = path.args + && args.args.iter().any(|arg| match arg { + GenericArg::Infer(_) => true, + GenericArg::Type(ty) => ty_contains_infer(ty), + _ => false, + }) + { + Position::ReborrowStable(precedence) } else { - // This shouldn't happen. The destination should exist. - break &[]; + Position::DerefStable(precedence) } }, - _ => break &[], + TyKind::Slice(_) + | TyKind::Array(..) + | TyKind::BareFn(_) + | TyKind::Never + | TyKind::Tup(_) + | TyKind::Ptr(_) + | TyKind::TraitObject(..) + | TyKind::Path(_) => Position::DerefStable(precedence), + TyKind::OpaqueDef(..) + | TyKind::Infer + | TyKind::Typeof(..) + | TyKind::Err => Position::ReborrowStable(precedence), + }; + } +} + +// Checks whether a type is inferred at some point. +// e.g. `_`, `Box<_>`, `[_]` +fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool { + struct V(bool); + impl Visitor<'_> for V { + fn visit_ty(&mut self, ty: &hir::Ty<'_>) { + if self.0 + || matches!( + ty.kind, + TyKind::OpaqueDef(..) | TyKind::Infer | TyKind::Typeof(_) | TyKind::Err + ) + { + self.0 = true; + } else { + walk_ty(self, ty); + } } + + fn visit_generic_arg(&mut self, arg: &GenericArg<'_>) { + if self.0 || matches!(arg, GenericArg::Infer(_)) { + self.0 = true; + } else if let GenericArg::Type(ty) = arg { + self.visit_ty(ty); + } + } + } + let mut v = V(false); + v.visit_ty(ty); + v.0 +} + +// Checks whether a type is stable when switching to auto dereferencing, +fn param_auto_deref_stability(ty: Ty<'_>, precedence: i8) -> Position { + let ty::Ref(_, mut ty, _) = *ty.kind() else { + return Position::Other(precedence); + }; + + loop { + break match *ty.kind() { + ty::Ref(_, ref_ty, _) => { + ty = ref_ty; + continue; + }, + ty::Infer(_) + | ty::Error(_) + | ty::Param(_) + | ty::Bound(..) + | ty::Opaque(..) + | ty::Placeholder(_) + | ty::Dynamic(..) => Position::ReborrowStable(precedence), + ty::Adt(..) if ty.has_placeholders() || ty.has_param_types_or_consts() => { + Position::ReborrowStable(precedence) + }, + ty::Adt(..) + | 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::Projection(_) => Position::DerefStable(precedence), + }; + } +} + +fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool { + if let ty::Adt(adt, _) = *ty.kind() { + adt.is_struct() && adt.all_fields().any(|f| f.name == name) + } else { + false } } -#[expect(clippy::needless_pass_by_value)] -fn report<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, state: State, data: StateData) { +#[expect(clippy::needless_pass_by_value, clippy::too_many_lines)] +fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { match state { State::DerefMethod { ty_changed_count, @@ -647,15 +1003,14 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, state: State, data: S app, ); }, - State::DerefedBorrow { - required_precedence, - msg, - .. - } => { + State::DerefedBorrow(state) => { let mut app = Applicability::MachineApplicable; - let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0; - span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, msg, |diag| { - let sugg = if required_precedence > expr.precedence().order() && !has_enclosing_paren(&snip) { + let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); + span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, state.msg, |diag| { + let sugg = if !snip_is_macro + && expr.precedence().order() < data.position.precedence() + && !has_enclosing_paren(&snip) + { format!("({})", snip) } else { snip.into() @@ -663,6 +1018,48 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, state: State, data: S diag.span_suggestion(data.span, "change this to", sugg, app); }); }, + State::ExplicitDeref { deref_span_id } => { + let (span, hir_id, precedence) = if let Some((span, hir_id)) = deref_span_id + && !cx.typeck_results().expr_ty(expr).is_ref() + { + (span, hir_id, PREC_PREFIX) + } else { + (data.span, data.hir_id, data.position.precedence()) + }; + span_lint_hir_and_then( + cx, + EXPLICIT_AUTO_DEREF, + hir_id, + span, + "deref which would be done by auto-deref", + |diag| { + let mut app = Applicability::MachineApplicable; + let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, span.ctxt(), "..", &mut app); + let sugg = + if !snip_is_macro && expr.precedence().order() < precedence && !has_enclosing_paren(&snip) { + format!("({})", snip) + } else { + snip.into() + }; + diag.span_suggestion(span, "try this", sugg, app); + }, + ); + }, + State::ExplicitDerefField { .. } => { + span_lint_hir_and_then( + cx, + EXPLICIT_AUTO_DEREF, + data.hir_id, + data.span, + "deref which would be done by auto-deref", + |diag| { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0; + diag.span_suggestion(data.span, "try this", snip.into_owned(), app); + }, + ); + }, + State::Borrow | State::Reborrow { .. } => (), } } diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 27acdd7c726e..563ad891603a 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -39,6 +39,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF), LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT), LintId::of(default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY), + LintId::of(dereference::EXPLICIT_AUTO_DEREF), LintId::of(dereference::NEEDLESS_BORROW), LintId::of(derivable_impls::DERIVABLE_IMPLS), LintId::of(derive::DERIVE_HASH_XOR_EQ), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index ed5446f58441..3784d3c68dce 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -9,6 +9,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(bytes_count_to_len::BYTES_COUNT_TO_LEN), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::UNNECESSARY_CAST), + LintId::of(dereference::EXPLICIT_AUTO_DEREF), LintId::of(derivable_impls::DERIVABLE_IMPLS), LintId::of(double_parens::DOUBLE_PARENS), LintId::of(explicit_write::EXPLICIT_WRITE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index af7226f242f3..d3c75f8b5191 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -104,6 +104,7 @@ store.register_lints(&[ default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY, default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK, default_union_representation::DEFAULT_UNION_REPRESENTATION, + dereference::EXPLICIT_AUTO_DEREF, dereference::EXPLICIT_DEREF_METHODS, dereference::NEEDLESS_BORROW, dereference::REF_BINDING_TO_REFERENCE, diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index 16fefea55201..15513de7d860 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -285,7 +285,7 @@ impl<'a> NormalizedPat<'a> { // TODO: Handle negative integers. They're currently treated as a wild match. ExprKind::Lit(lit) => match lit.node { LitKind::Str(sym, _) => Self::LitStr(sym), - LitKind::ByteStr(ref bytes) => Self::LitBytes(&**bytes), + LitKind::ByteStr(ref bytes) => Self::LitBytes(bytes), LitKind::Byte(val) => Self::LitInt(val.into()), LitKind::Char(val) => Self::LitInt(val.into()), LitKind::Int(val, _) => Self::LitInt(val), diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 9df2db45dcf8..5ae4a65acaf3 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -55,7 +55,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e cx, (ex, expr), (bind_names, matched_vars), - &*snippet_body, + &snippet_body, &mut applicability, Some(span), ); @@ -88,7 +88,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e cx, (ex, expr), (bind_names, matched_vars), - &*snippet_body, + &snippet_body, &mut applicability, None, ); diff --git a/clippy_lints/src/matches/match_str_case_mismatch.rs b/clippy_lints/src/matches/match_str_case_mismatch.rs index 8302ce426e57..fa3b8d1fceaa 100644 --- a/clippy_lints/src/matches/match_str_case_mismatch.rs +++ b/clippy_lints/src/matches/match_str_case_mismatch.rs @@ -118,7 +118,7 @@ fn lint(cx: &LateContext<'_>, case_method: &CaseMethod, bad_case_span: Span, bad MATCH_STR_CASE_MISMATCH, bad_case_span, "this `match` arm has a differing case than its expression", - &*format!("consider changing the case of this arm to respect `{}`", method_str), + &format!("consider changing the case of this arm to respect `{}`", method_str), format!("\"{}\"", suggestion), Applicability::MachineApplicable, ); diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 0ea3f3b673b7..65cecd333f1c 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -362,9 +362,9 @@ fn find_good_method_for_match<'a>( .qpath_res(path_right, arms[1].pat.hir_id) .opt_def_id()?; let body_node_pair = if match_def_path(cx, left_id, expected_left) && match_def_path(cx, right_id, expected_right) { - (&(*arms[0].body).kind, &(*arms[1].body).kind) + (&arms[0].body.kind, &arms[1].body.kind) } else if match_def_path(cx, right_id, expected_left) && match_def_path(cx, right_id, expected_right) { - (&(*arms[1].body).kind, &(*arms[0].body).kind) + (&arms[1].body.kind, &arms[0].body.kind) } else { return None; }; diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index c93b4b2f205f..b96af06b8d7c 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -326,7 +326,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> { // add the pattern after the expression because the bindings aren't available // yet in the init // expression - SimilarNamesNameVisitor(self).visit_pat(&*local.pat); + SimilarNamesNameVisitor(self).visit_pat(&local.pat); } fn visit_block(&mut self, blk: &'tcx Block) { self.single_char_names.push(vec![]); diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index b06eba13d2fd..5678b8f6ca68 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -574,14 +574,13 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &'tcx Body<'_>, args: Some((Node::Expr(e), child_id)) => match e.kind { ExprKind::Call(f, expr_args) => { let i = expr_args.iter().position(|arg| arg.hir_id == child_id).unwrap_or(0); - if expr_sig(self.cx, f) - .map(|sig| sig.input(i).skip_binder().peel_refs()) - .map_or(true, |ty| match *ty.kind() { + if expr_sig(self.cx, f).and_then(|sig| sig.input(i)).map_or(true, |ty| { + match *ty.skip_binder().peel_refs().kind() { ty::Param(_) => true, ty::Adt(def, _) => def.did() == args.ty_did, _ => false, - }) - { + } + }) { // Passed to a function taking the non-dereferenced type. set_skip_flag(); } diff --git a/clippy_lints/src/redundant_static_lifetimes.rs b/clippy_lints/src/redundant_static_lifetimes.rs index 0825f00f421c..f8801f769e83 100644 --- a/clippy_lints/src/redundant_static_lifetimes.rs +++ b/clippy_lints/src/redundant_static_lifetimes.rs @@ -87,7 +87,7 @@ impl RedundantStaticLifetimes { _ => {}, } } - self.visit_type(&*borrow_type.ty, cx, reason); + self.visit_type(&borrow_type.ty, cx, reason); }, _ => {}, } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 73c1bdd0e3f4..a2772edf7383 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2058,6 +2058,21 @@ pub fn peel_hir_expr_refs<'a>(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) { (e, count) } +/// Peels off all references on the type. Returns the underlying type and the number of references +/// removed. +pub fn peel_hir_ty_refs<'a>(mut ty: &'a hir::Ty<'a>) -> (&'a hir::Ty<'a>, usize) { + let mut count = 0; + loop { + match &ty.kind { + TyKind::Rptr(_, ref_ty) => { + ty = ref_ty.ty; + count += 1; + }, + _ => break (ty, count), + } + } +} + /// Removes `AddrOf` operators (`&`) or deref operators (`*`), but only if a reference type is /// dereferenced. An overloaded deref such as `Vec` to slice would not be removed. pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> { @@ -2110,7 +2125,7 @@ fn with_test_item_names<'tcx>(tcx: TyCtxt<'tcx>, module: LocalDefId, f: impl Fn( } } names.sort_unstable(); - f(&*entry.insert(names)) + f(entry.insert(names)) }, } } @@ -2168,6 +2183,50 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool { && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests") } +/// Walks the HIR tree from the given expression, up to the node where the value produced by the +/// expression is consumed. Calls the function for every node encountered this way until it returns +/// `Some`. +/// +/// This allows walking through `if`, `match`, `break`, block expressions to find where the value +/// produced by the expression is consumed. +pub fn walk_to_expr_usage<'tcx, T>( + cx: &LateContext<'tcx>, + e: &Expr<'tcx>, + mut f: impl FnMut(Node<'tcx>, HirId) -> Option, +) -> Option { + let map = cx.tcx.hir(); + let mut iter = map.parent_iter(e.hir_id); + let mut child_id = e.hir_id; + + while let Some((parent_id, parent)) = iter.next() { + if let Some(x) = f(parent, child_id) { + return Some(x); + } + let parent = match parent { + Node::Expr(e) => e, + Node::Block(Block { expr: Some(body), .. }) | Node::Arm(Arm { body, .. }) if body.hir_id == child_id => { + child_id = parent_id; + continue; + }, + Node::Arm(a) if a.body.hir_id == child_id => { + child_id = parent_id; + continue; + }, + _ => return None, + }; + match parent.kind { + ExprKind::If(child, ..) | ExprKind::Match(child, ..) if child.hir_id != child_id => child_id = parent_id, + ExprKind::Break(Destination { target_id: Ok(id), .. }, _) => { + child_id = id; + iter = map.parent_iter(id); + }, + ExprKind::Block(..) => child_id = parent_id, + _ => return None, + } + } + None +} + macro_rules! op_utils { ($($name:ident $assign:ident)*) => { /// Binary operation traits like `LangItem::Add` diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 4c079151f24d..6ca36eed4e65 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -6,16 +6,16 @@ use core::ops::ControlFlow; use rustc_ast::ast::Mutability; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir as hir; -use rustc_hir::def::{CtorKind, DefKind, Res}; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; -use rustc_hir::{Expr, LangItem, TyKind, Unsafety}; +use rustc_hir::{Expr, FnDecl, LangItem, TyKind, Unsafety}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::mir::interpret::{ConstValue, Scalar}; use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst}; use rustc_middle::ty::{ - self, AdtDef, Binder, BoundRegion, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, Region, RegionKind, Ty, - TyCtxt, TypeFoldable, TypeSuperFoldable, TypeVisitor, UintTy, VariantDiscr, + self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, ProjectionTy, + Region, RegionKind, Ty, TyCtxt, TypeFoldable, TypeSuperFoldable, TypeVisitor, UintTy, VariantDef, VariantDiscr, }; use rustc_span::symbol::Ident; use rustc_span::{sym, Span, Symbol, DUMMY_SP}; @@ -502,16 +502,46 @@ pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> impl Iterator { Sig(Binder<'tcx, FnSig<'tcx>>), - Closure(Binder<'tcx, FnSig<'tcx>>), + Closure(Option<&'tcx FnDecl<'tcx>>, Binder<'tcx, FnSig<'tcx>>), Trait(Binder<'tcx, Ty<'tcx>>, Option>>), } impl<'tcx> ExprFnSig<'tcx> { - /// Gets the argument type at the given offset. - pub fn input(self, i: usize) -> Binder<'tcx, Ty<'tcx>> { + /// Gets the argument type at the given offset. This will return `None` when the index is out of + /// bounds only for variadic functions, otherwise this will panic. + pub fn input(self, i: usize) -> Option>> { match self { - Self::Sig(sig) => sig.input(i), - Self::Closure(sig) => sig.input(0).map_bound(|ty| ty.tuple_fields()[i]), - Self::Trait(inputs, _) => inputs.map_bound(|ty| ty.tuple_fields()[i]), + Self::Sig(sig) => { + if sig.c_variadic() { + sig.inputs().map_bound(|inputs| inputs.get(i).copied()).transpose() + } else { + Some(sig.input(i)) + } + }, + Self::Closure(_, sig) => Some(sig.input(0).map_bound(|ty| ty.tuple_fields()[i])), + Self::Trait(inputs, _) => Some(inputs.map_bound(|ty| ty.tuple_fields()[i])), + } + } + + /// Gets the argument type at the given offset. For closures this will also get the type as + /// written. This will return `None` when the index is out of bounds only for variadic + /// functions, otherwise this will panic. + pub fn input_with_hir(self, i: usize) -> Option<(Option<&'tcx hir::Ty<'tcx>>, Binder<'tcx, Ty<'tcx>>)> { + match self { + Self::Sig(sig) => { + if sig.c_variadic() { + sig.inputs() + .map_bound(|inputs| inputs.get(i).copied()) + .transpose() + .map(|arg| (None, arg)) + } else { + Some((None, sig.input(i))) + } + }, + Self::Closure(decl, sig) => Some(( + decl.and_then(|decl| decl.inputs.get(i)), + sig.input(0).map_bound(|ty| ty.tuple_fields()[i]), + )), + Self::Trait(inputs, _) => Some((None, inputs.map_bound(|ty| ty.tuple_fields()[i]))), } } @@ -519,7 +549,7 @@ impl<'tcx> ExprFnSig<'tcx> { /// specified. pub fn output(self) -> Option>> { match self { - Self::Sig(sig) | Self::Closure(sig) => Some(sig.output()), + Self::Sig(sig) | Self::Closure(_, sig) => Some(sig.output()), Self::Trait(_, output) => output, } } @@ -530,74 +560,123 @@ pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option Some(ExprFnSig::Closure(subs.as_closure().sig())), - ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs))), - ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)), - ty::Dynamic(bounds, _) => { - let lang_items = cx.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() => - { - let output = bounds - .projection_bounds() - .find(|p| lang_items.fn_once_output().map_or(false, |id| id == p.item_def_id())) - .map(|p| p.map_bound(|p| p.term.ty().expect("return type was a const"))); - Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)), output)) - }, - _ => None, + ty_sig(cx, cx.typeck_results().expr_ty_adjusted(expr).peel_refs()) + } +} + +fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option> { + match *ty.kind() { + ty::Closure(id, subs) => { + let decl = id + .as_local() + .and_then(|id| cx.tcx.hir().fn_decl_by_hir_id(cx.tcx.hir().local_def_id_to_hir_id(id))); + Some(ExprFnSig::Closure(decl, subs.as_closure().sig())) + }, + ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs))), + ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)), + ty::Dynamic(bounds, _) => { + let lang_items = cx.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() => + { + let output = bounds + .projection_bounds() + .find(|p| lang_items.fn_once_output().map_or(false, |id| id == p.item_def_id())) + .map(|p| p.map_bound(|p| p.term.ty().unwrap())); + Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)), output)) + }, + _ => None, + } + }, + ty::Projection(proj) => match cx.tcx.try_normalize_erasing_regions(cx.param_env, ty) { + Ok(normalized_ty) if normalized_ty != ty => ty_sig(cx, normalized_ty), + _ => sig_for_projection(cx, proj).or_else(|| sig_from_bounds(cx, ty)), + }, + ty::Param(_) => sig_from_bounds(cx, ty), + _ => None, + } +} + +fn sig_from_bounds<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option> { + let mut inputs = None; + let mut output = None; + let lang_items = cx.tcx.lang_items(); + + for (pred, _) in all_predicates_of(cx.tcx, cx.typeck_results().hir_owner.to_def_id()) { + match pred.kind().skip_binder() { + 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 => + { + if inputs.is_some() { + // Multiple different fn trait impls. Is this even allowed? + return None; } + inputs = Some(pred.kind().rebind(p.trait_ref.substs.type_at(1))); }, - ty::Param(_) | ty::Projection(..) => { - let mut inputs = None; - let mut output = None; - let lang_items = cx.tcx.lang_items(); - - for (pred, _) in all_predicates_of(cx.tcx, cx.typeck_results().hir_owner.to_def_id()) { - let mut is_input = false; - if let Some(ty) = pred - .kind() - .map_bound(|pred| match pred { - 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 => - { - is_input = true; - Some(p.trait_ref.substs.type_at(1)) - }, - PredicateKind::Projection(p) - if Some(p.projection_ty.item_def_id) == lang_items.fn_once_output() - && p.projection_ty.self_ty() == ty => - { - is_input = false; - p.term.ty() - }, - _ => None, - }) - .transpose() - { - if is_input && inputs.is_none() { - inputs = Some(ty); - } else if !is_input && output.is_none() { - output = Some(ty); - } else { - // Multiple different fn trait impls. Is this even allowed? - return None; - } - } + PredicateKind::Projection(p) + if Some(p.projection_ty.item_def_id) == lang_items.fn_once_output() + && p.projection_ty.self_ty() == ty => + { + if output.is_some() { + // Multiple different fn trait impls. Is this even allowed? + return None; } + output = Some(pred.kind().rebind(p.term.ty().unwrap())); + }, + _ => (), + } + } + + inputs.map(|ty| ExprFnSig::Trait(ty, output)) +} - inputs.map(|ty| ExprFnSig::Trait(ty, output)) +fn sig_for_projection<'tcx>(cx: &LateContext<'tcx>, ty: ProjectionTy<'tcx>) -> Option> { + let mut inputs = None; + let mut output = None; + let lang_items = cx.tcx.lang_items(); + + for pred in cx + .tcx + .bound_explicit_item_bounds(ty.item_def_id) + .transpose_iter() + .map(|x| x.map_bound(|(p, _)| p)) + { + match pred.0.kind().skip_binder() { + 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())) => + { + if inputs.is_some() { + // Multiple different fn trait impls. Is this even allowed? + return None; + } + inputs = Some( + pred.map_bound(|pred| pred.kind().rebind(p.trait_ref.substs.type_at(1))) + .subst(cx.tcx, ty.substs), + ); }, - _ => None, + PredicateKind::Projection(p) if Some(p.projection_ty.item_def_id) == lang_items.fn_once_output() => { + if output.is_some() { + // Multiple different fn trait impls. Is this even allowed? + return None; + } + output = Some( + pred.map_bound(|pred| pred.kind().rebind(p.term.ty().unwrap())) + .subst(cx.tcx, ty.substs), + ); + }, + _ => (), } } + + inputs.map(|ty| ExprFnSig::Trait(ty, output)) } #[derive(Clone, Copy)] @@ -695,3 +774,18 @@ pub fn for_each_top_level_late_bound_region( } ty.visit_with(&mut V { index: 0, f }) } + +/// Gets the struct or enum variant from the given `Res` +pub fn variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option<&'tcx VariantDef> { + match res { + Res::Def(DefKind::Struct, id) => Some(cx.tcx.adt_def(id).non_enum_variant()), + Res::Def(DefKind::Variant, id) => Some(cx.tcx.adt_def(cx.tcx.parent(id)).variant_with_id(id)), + Res::Def(DefKind::Ctor(CtorOf::Struct, _), id) => Some(cx.tcx.adt_def(cx.tcx.parent(id)).non_enum_variant()), + Res::Def(DefKind::Ctor(CtorOf::Variant, _), id) => { + let var_id = cx.tcx.parent(id); + Some(cx.tcx.adt_def(cx.tcx.parent(var_id)).variant_with_id(var_id)) + }, + Res::SelfCtor(id) => Some(cx.tcx.type_of(id).ty_adt_def().unwrap().non_enum_variant()), + _ => None, + } +} diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed new file mode 100644 index 000000000000..d4ff1b1566dc --- /dev/null +++ b/tests/ui/explicit_auto_deref.fixed @@ -0,0 +1,214 @@ +// run-rustfix + +#![warn(clippy::explicit_auto_deref)] +#![allow( + dead_code, + unused_braces, + clippy::borrowed_box, + clippy::needless_borrow, + clippy::needless_return, + clippy::ptr_arg, + clippy::redundant_field_names, + clippy::too_many_arguments, + clippy::borrow_deref_ref, + clippy::let_unit_value +)] + +trait CallableStr { + type T: Fn(&str); + fn callable_str(&self) -> Self::T; +} +impl CallableStr for () { + type T = fn(&str); + fn callable_str(&self) -> Self::T { + fn f(_: &str) {} + f + } +} +impl CallableStr for i32 { + type T = <() as CallableStr>::T; + fn callable_str(&self) -> Self::T { + ().callable_str() + } +} + +trait CallableT { + type T: Fn(&U); + fn callable_t(&self) -> Self::T; +} +impl CallableT for () { + type T = fn(&U); + fn callable_t(&self) -> Self::T { + fn f(_: &U) {} + f:: + } +} +impl CallableT for i32 { + type T = <() as CallableT>::T; + fn callable_t(&self) -> Self::T { + ().callable_t() + } +} + +fn f_str(_: &str) {} +fn f_string(_: &String) {} +fn f_t(_: T) {} +fn f_ref_t(_: &T) {} + +fn f_str_t(_: &str, _: T) {} + +fn f_box_t(_: &Box) {} + +extern "C" { + fn var(_: u32, ...); +} + +fn main() { + let s = String::new(); + + let _: &str = &s; + let _ = &*s; // Don't lint. Inferred type would change. + let _: &_ = &*s; // Don't lint. Inferred type would change. + + f_str(&s); + f_t(&*s); // Don't lint. Inferred type would change. + f_ref_t(&*s); // Don't lint. Inferred type would change. + + f_str_t(&s, &*s); // Don't lint second param. + + let b = Box::new(Box::new(Box::new(5))); + let _: &Box = &b; + let _: &Box<_> = &**b; // Don't lint. Inferred type would change. + + f_box_t(&**b); // Don't lint. Inferred type would change. + + let c = |_x: &str| (); + c(&s); + + let c = |_x| (); + c(&*s); // Don't lint. Inferred type would change. + + fn _f(x: &String) -> &str { + x + } + + fn _f1(x: &String) -> &str { + { x } + } + + fn _f2(x: &String) -> &str { + { x } + } + + fn _f3(x: &Box>>) -> &Box { + x + } + + fn _f4( + x: String, + f1: impl Fn(&str), + f2: &dyn Fn(&str), + f3: fn(&str), + f4: impl CallableStr, + f5: <() as CallableStr>::T, + f6: ::T, + f7: &dyn CallableStr, + f8: impl CallableT, + f9: <() as CallableT>::T, + f10: >::T, + f11: &dyn CallableT, + ) { + f1(&x); + f2(&x); + f3(&x); + f4.callable_str()(&x); + f5(&x); + f6(&x); + f7.callable_str()(&x); + f8.callable_t()(&x); + f9(&x); + f10(&x); + f11.callable_t()(&x); + } + + struct S1<'a>(&'a str); + let _ = S1(&s); + + struct S2<'a> { + s: &'a str, + } + let _ = S2 { s: &s }; + + struct S3<'a, T: ?Sized>(&'a T); + let _ = S3(&*s); // Don't lint. Inferred type would change. + + struct S4<'a, T: ?Sized> { + s: &'a T, + } + let _ = S4 { s: &*s }; // Don't lint. Inferred type would change. + + enum E1<'a> { + S1(&'a str), + S2 { s: &'a str }, + } + impl<'a> E1<'a> { + fn m1(s: &'a String) { + let _ = Self::S1(s); + let _ = Self::S2 { s: s }; + } + } + let _ = E1::S1(&s); + let _ = E1::S2 { s: &s }; + + enum E2<'a, T: ?Sized> { + S1(&'a T), + S2 { s: &'a T }, + } + let _ = E2::S1(&*s); // Don't lint. Inferred type would change. + let _ = E2::S2 { s: &*s }; // Don't lint. Inferred type would change. + + let ref_s = &s; + let _: &String = &*ref_s; // Don't lint reborrow. + f_string(&*ref_s); // Don't lint reborrow. + + struct S5 { + foo: u32, + } + let b = Box::new(Box::new(S5 { foo: 5 })); + let _ = b.foo; + let _ = b.foo; + let _ = b.foo; + + struct S6 { + foo: S5, + } + impl core::ops::Deref for S6 { + type Target = S5; + fn deref(&self) -> &Self::Target { + &self.foo + } + } + let s6 = S6 { foo: S5 { foo: 5 } }; + let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` + + let ref_str = &"foo"; + let _ = f_str(ref_str); + let ref_ref_str = &ref_str; + let _ = f_str(ref_ref_str); + + fn _f5(x: &u32) -> u32 { + if true { + *x + } else { + return *x; + } + } + + f_str(&&ref_str); // `needless_borrow` will suggest removing both references + f_str(&ref_str); // `needless_borrow` will suggest removing only one reference + + let x = &&40; + unsafe { + var(0, &**x); + } +} diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs new file mode 100644 index 000000000000..99294a7947bf --- /dev/null +++ b/tests/ui/explicit_auto_deref.rs @@ -0,0 +1,214 @@ +// run-rustfix + +#![warn(clippy::explicit_auto_deref)] +#![allow( + dead_code, + unused_braces, + clippy::borrowed_box, + clippy::needless_borrow, + clippy::needless_return, + clippy::ptr_arg, + clippy::redundant_field_names, + clippy::too_many_arguments, + clippy::borrow_deref_ref, + clippy::let_unit_value +)] + +trait CallableStr { + type T: Fn(&str); + fn callable_str(&self) -> Self::T; +} +impl CallableStr for () { + type T = fn(&str); + fn callable_str(&self) -> Self::T { + fn f(_: &str) {} + f + } +} +impl CallableStr for i32 { + type T = <() as CallableStr>::T; + fn callable_str(&self) -> Self::T { + ().callable_str() + } +} + +trait CallableT { + type T: Fn(&U); + fn callable_t(&self) -> Self::T; +} +impl CallableT for () { + type T = fn(&U); + fn callable_t(&self) -> Self::T { + fn f(_: &U) {} + f:: + } +} +impl CallableT for i32 { + type T = <() as CallableT>::T; + fn callable_t(&self) -> Self::T { + ().callable_t() + } +} + +fn f_str(_: &str) {} +fn f_string(_: &String) {} +fn f_t(_: T) {} +fn f_ref_t(_: &T) {} + +fn f_str_t(_: &str, _: T) {} + +fn f_box_t(_: &Box) {} + +extern "C" { + fn var(_: u32, ...); +} + +fn main() { + let s = String::new(); + + let _: &str = &*s; + let _ = &*s; // Don't lint. Inferred type would change. + let _: &_ = &*s; // Don't lint. Inferred type would change. + + f_str(&*s); + f_t(&*s); // Don't lint. Inferred type would change. + f_ref_t(&*s); // Don't lint. Inferred type would change. + + f_str_t(&*s, &*s); // Don't lint second param. + + let b = Box::new(Box::new(Box::new(5))); + let _: &Box = &**b; + let _: &Box<_> = &**b; // Don't lint. Inferred type would change. + + f_box_t(&**b); // Don't lint. Inferred type would change. + + let c = |_x: &str| (); + c(&*s); + + let c = |_x| (); + c(&*s); // Don't lint. Inferred type would change. + + fn _f(x: &String) -> &str { + &**x + } + + fn _f1(x: &String) -> &str { + { &**x } + } + + fn _f2(x: &String) -> &str { + &**{ x } + } + + fn _f3(x: &Box>>) -> &Box { + &***x + } + + fn _f4( + x: String, + f1: impl Fn(&str), + f2: &dyn Fn(&str), + f3: fn(&str), + f4: impl CallableStr, + f5: <() as CallableStr>::T, + f6: ::T, + f7: &dyn CallableStr, + f8: impl CallableT, + f9: <() as CallableT>::T, + f10: >::T, + f11: &dyn CallableT, + ) { + f1(&*x); + f2(&*x); + f3(&*x); + f4.callable_str()(&*x); + f5(&*x); + f6(&*x); + f7.callable_str()(&*x); + f8.callable_t()(&*x); + f9(&*x); + f10(&*x); + f11.callable_t()(&*x); + } + + struct S1<'a>(&'a str); + let _ = S1(&*s); + + struct S2<'a> { + s: &'a str, + } + let _ = S2 { s: &*s }; + + struct S3<'a, T: ?Sized>(&'a T); + let _ = S3(&*s); // Don't lint. Inferred type would change. + + struct S4<'a, T: ?Sized> { + s: &'a T, + } + let _ = S4 { s: &*s }; // Don't lint. Inferred type would change. + + enum E1<'a> { + S1(&'a str), + S2 { s: &'a str }, + } + impl<'a> E1<'a> { + fn m1(s: &'a String) { + let _ = Self::S1(&**s); + let _ = Self::S2 { s: &**s }; + } + } + let _ = E1::S1(&*s); + let _ = E1::S2 { s: &*s }; + + enum E2<'a, T: ?Sized> { + S1(&'a T), + S2 { s: &'a T }, + } + let _ = E2::S1(&*s); // Don't lint. Inferred type would change. + let _ = E2::S2 { s: &*s }; // Don't lint. Inferred type would change. + + let ref_s = &s; + let _: &String = &*ref_s; // Don't lint reborrow. + f_string(&*ref_s); // Don't lint reborrow. + + struct S5 { + foo: u32, + } + let b = Box::new(Box::new(S5 { foo: 5 })); + let _ = b.foo; + let _ = (*b).foo; + let _ = (**b).foo; + + struct S6 { + foo: S5, + } + impl core::ops::Deref for S6 { + type Target = S5; + fn deref(&self) -> &Self::Target { + &self.foo + } + } + let s6 = S6 { foo: S5 { foo: 5 } }; + let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` + + let ref_str = &"foo"; + let _ = f_str(*ref_str); + let ref_ref_str = &ref_str; + let _ = f_str(**ref_ref_str); + + fn _f5(x: &u32) -> u32 { + if true { + *x + } else { + return *x; + } + } + + f_str(&&*ref_str); // `needless_borrow` will suggest removing both references + f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference + + let x = &&40; + unsafe { + var(0, &**x); + } +} diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr new file mode 100644 index 000000000000..55f956e37aed --- /dev/null +++ b/tests/ui/explicit_auto_deref.stderr @@ -0,0 +1,196 @@ +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:69:20 + | +LL | let _: &str = &*s; + | ^^ help: try this: `s` + | + = note: `-D clippy::explicit-auto-deref` implied by `-D warnings` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:73:12 + | +LL | f_str(&*s); + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:77:14 + | +LL | f_str_t(&*s, &*s); // Don't lint second param. + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:80:25 + | +LL | let _: &Box = &**b; + | ^^^ help: try this: `b` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:86:8 + | +LL | c(&*s); + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:92:9 + | +LL | &**x + | ^^^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:96:11 + | +LL | { &**x } + | ^^^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:100:9 + | +LL | &**{ x } + | ^^^^^^^^ help: try this: `{ x }` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:104:9 + | +LL | &***x + | ^^^^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:121:13 + | +LL | f1(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:122:13 + | +LL | f2(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:123:13 + | +LL | f3(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:124:28 + | +LL | f4.callable_str()(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:125:13 + | +LL | f5(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:126:13 + | +LL | f6(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:127:28 + | +LL | f7.callable_str()(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:128:26 + | +LL | f8.callable_t()(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:129:13 + | +LL | f9(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:130:14 + | +LL | f10(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:131:27 + | +LL | f11.callable_t()(&*x); + | ^^ help: try this: `x` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:135:17 + | +LL | let _ = S1(&*s); + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:140:22 + | +LL | let _ = S2 { s: &*s }; + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:156:30 + | +LL | let _ = Self::S1(&**s); + | ^^^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:157:35 + | +LL | let _ = Self::S2 { s: &**s }; + | ^^^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:160:21 + | +LL | let _ = E1::S1(&*s); + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:161:26 + | +LL | let _ = E1::S2 { s: &*s }; + | ^^ help: try this: `s` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:179:13 + | +LL | let _ = (*b).foo; + | ^^^^ help: try this: `b` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:180:13 + | +LL | let _ = (**b).foo; + | ^^^^^ help: try this: `b` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:195:19 + | +LL | let _ = f_str(*ref_str); + | ^^^^^^^^ help: try this: `ref_str` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:197:19 + | +LL | let _ = f_str(**ref_ref_str); + | ^^^^^^^^^^^^^ help: try this: `ref_ref_str` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:207:13 + | +LL | f_str(&&*ref_str); // `needless_borrow` will suggest removing both references + | ^^^^^^^^ help: try this: `ref_str` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:208:12 + | +LL | f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference + | ^^^^^^^^^^ help: try this: `ref_str` + +error: aborting due to 32 previous errors + diff --git a/tests/ui/explicit_deref_methods.fixed b/tests/ui/explicit_deref_methods.fixed index 92f27e68549a..523cae183ee6 100644 --- a/tests/ui/explicit_deref_methods.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -4,7 +4,8 @@ unused_variables, clippy::clone_double_ref, clippy::needless_borrow, - clippy::borrow_deref_ref + clippy::borrow_deref_ref, + clippy::explicit_auto_deref )] #![warn(clippy::explicit_deref_methods)] diff --git a/tests/ui/explicit_deref_methods.rs b/tests/ui/explicit_deref_methods.rs index d118607f992b..0bbc1ae57cdf 100644 --- a/tests/ui/explicit_deref_methods.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -4,7 +4,8 @@ unused_variables, clippy::clone_double_ref, clippy::needless_borrow, - clippy::borrow_deref_ref + clippy::borrow_deref_ref, + clippy::explicit_auto_deref )] #![warn(clippy::explicit_deref_methods)] diff --git a/tests/ui/explicit_deref_methods.stderr b/tests/ui/explicit_deref_methods.stderr index 8e8b358972be..4b10ed1377b0 100644 --- a/tests/ui/explicit_deref_methods.stderr +++ b/tests/ui/explicit_deref_methods.stderr @@ -1,5 +1,5 @@ error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:35:19 + --> $DIR/explicit_deref_methods.rs:36:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,67 +7,67 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` error: explicit `deref_mut` method call - --> $DIR/explicit_deref_methods.rs:37:23 + --> $DIR/explicit_deref_methods.rs:38:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut **a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:40:39 + --> $DIR/explicit_deref_methods.rs:41:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:40:50 + --> $DIR/explicit_deref_methods.rs:41:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:42:20 + --> $DIR/explicit_deref_methods.rs:43:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:45:11 + --> $DIR/explicit_deref_methods.rs:46:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:49:28 + --> $DIR/explicit_deref_methods.rs:50:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:51:13 + --> $DIR/explicit_deref_methods.rs:52:13 | LL | let b = just_return(a).deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:53:28 + --> $DIR/explicit_deref_methods.rs:54:28 | LL | let b: String = concat(just_return(a).deref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:55:19 + --> $DIR/explicit_deref_methods.rs:56:19 | LL | let b: &str = a.deref().deref(); | ^^^^^^^^^^^^^^^^^ help: try this: `&**a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:58:13 + --> $DIR/explicit_deref_methods.rs:59:13 | LL | let b = opt_a.unwrap().deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:84:31 + --> $DIR/explicit_deref_methods.rs:85:31 | LL | let b: &str = expr_deref!(a.deref()); | ^^^^^^^^^ help: try this: `&*a` diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index e7a483c05829..cb0051224360 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -62,7 +62,18 @@ fn main() { 0 => &mut x, _ => &mut *x, }; - + let y: &mut i32 = match 0 { + // Lint here. The type given above triggers auto-borrow. + 0 => x, + _ => &mut *x, + }; + fn ref_mut_i32(_: &mut i32) {} + ref_mut_i32(match 0 { + // Lint here. The type given above triggers auto-borrow. + 0 => x, + _ => &mut *x, + }); + // use 'x' after to make sure it's still usable in the fixed code. *x = 5; let s = String::new(); @@ -74,6 +85,36 @@ fn main() { let _ = x.0; let x = &x as *const (i32, i32); let _ = unsafe { (*x).0 }; + + // Issue #8367 + trait Foo { + fn foo(self); + } + impl Foo for &'_ () { + fn foo(self) {} + } + (&()).foo(); // Don't lint. `()` doesn't implement `Foo` + (&()).foo(); + + impl Foo for i32 { + fn foo(self) {} + } + impl Foo for &'_ i32 { + fn foo(self) {} + } + (&5).foo(); // Don't lint. `5` will call `::foo` + (&5).foo(); + + trait FooRef { + fn foo_ref(&self); + } + impl FooRef for () { + fn foo_ref(&self) {} + } + impl FooRef for &'_ () { + fn foo_ref(&self) {} + } + (&&()).foo_ref(); // Don't lint. `&()` will call `<() as FooRef>::foo_ref` } #[allow(clippy::needless_borrowed_reference)] diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 1d6bf46405a2..d636a4010037 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -62,7 +62,18 @@ fn main() { 0 => &mut x, _ => &mut *x, }; - + let y: &mut i32 = match 0 { + // Lint here. The type given above triggers auto-borrow. + 0 => &mut x, + _ => &mut *x, + }; + fn ref_mut_i32(_: &mut i32) {} + ref_mut_i32(match 0 { + // Lint here. The type given above triggers auto-borrow. + 0 => &mut x, + _ => &mut *x, + }); + // use 'x' after to make sure it's still usable in the fixed code. *x = 5; let s = String::new(); @@ -74,6 +85,36 @@ fn main() { let _ = (&x).0; let x = &x as *const (i32, i32); let _ = unsafe { (&*x).0 }; + + // Issue #8367 + trait Foo { + fn foo(self); + } + impl Foo for &'_ () { + fn foo(self) {} + } + (&()).foo(); // Don't lint. `()` doesn't implement `Foo` + (&&()).foo(); + + impl Foo for i32 { + fn foo(self) {} + } + impl Foo for &'_ i32 { + fn foo(self) {} + } + (&5).foo(); // Don't lint. `5` will call `::foo` + (&&5).foo(); + + trait FooRef { + fn foo_ref(&self); + } + impl FooRef for () { + fn foo_ref(&self) {} + } + impl FooRef for &'_ () { + fn foo_ref(&self) {} + } + (&&()).foo_ref(); // Don't lint. `&()` will call `<() as FooRef>::foo_ref` } #[allow(clippy::needless_borrowed_reference)] diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index be59d8f546d2..8a2e2b989590 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -84,17 +84,41 @@ error: this expression creates a reference which is immediately dereferenced by LL | let y: &mut i32 = &mut &mut x; | ^^^^^^^^^^^ help: change this to: `x` +error: this expression creates a reference which is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:67:14 + | +LL | 0 => &mut x, + | ^^^^^^ help: change this to: `x` + +error: this expression creates a reference which is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:73:14 + | +LL | 0 => &mut x, + | ^^^^^^ help: change this to: `x` + error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:74:13 + --> $DIR/needless_borrow.rs:85:13 | LL | let _ = (&x).0; | ^^^^ help: change this to: `x` error: this expression borrows a value the compiler would automatically borrow - --> $DIR/needless_borrow.rs:76:22 + --> $DIR/needless_borrow.rs:87:22 | LL | let _ = unsafe { (&*x).0 }; | ^^^^^ help: change this to: `(*x)` -error: aborting due to 16 previous errors +error: this expression creates a reference which is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:97:5 + | +LL | (&&()).foo(); + | ^^^^^^ help: change this to: `(&())` + +error: this expression creates a reference which is immediately dereferenced by the compiler + --> $DIR/needless_borrow.rs:106:5 + | +LL | (&&5).foo(); + | ^^^^^ help: change this to: `(&5)` + +error: aborting due to 20 previous errors diff --git a/tests/ui/needless_borrow_pat.rs b/tests/ui/needless_borrow_pat.rs index 04b6283da3c3..222e8e617995 100644 --- a/tests/ui/needless_borrow_pat.rs +++ b/tests/ui/needless_borrow_pat.rs @@ -1,7 +1,7 @@ // FIXME: run-rustfix waiting on multi-span suggestions #![warn(clippy::needless_borrow)] -#![allow(clippy::needless_borrowed_reference)] +#![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)] fn f1(_: &str) {} macro_rules! m1 { diff --git a/tests/ui/ref_binding_to_reference.rs b/tests/ui/ref_binding_to_reference.rs index 570ef406e4a9..c8d0e56b197f 100644 --- a/tests/ui/ref_binding_to_reference.rs +++ b/tests/ui/ref_binding_to_reference.rs @@ -2,7 +2,7 @@ #![feature(lint_reasons)] #![warn(clippy::ref_binding_to_reference)] -#![allow(clippy::needless_borrowed_reference)] +#![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)] fn f1(_: &str) {} macro_rules! m2 { diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 6831fb2cf59e..5190c5304c75 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/tests/ui/search_is_some_fixable_none.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index 767518ab0c0f..310d87333a93 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/tests/ui/search_is_some_fixable_none.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed index 7c940a2b069e..5a2aee465d1b 100644 --- a/tests/ui/search_is_some_fixable_some.fixed +++ b/tests/ui/search_is_some_fixable_some.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs index 77fd52e4ce76..0e98ae18a217 100644 --- a/tests/ui/search_is_some_fixable_some.rs +++ b/tests/ui/search_is_some_fixable_some.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/useless_asref.fixed b/tests/ui/useless_asref.fixed index e431661d180d..90cb8945e77f 100644 --- a/tests/ui/useless_asref.fixed +++ b/tests/ui/useless_asref.fixed @@ -1,6 +1,7 @@ // run-rustfix #![deny(clippy::useless_asref)] +#![allow(clippy::explicit_auto_deref)] use std::fmt::Debug; diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs index 6ae931d7aa48..cb9f8ae5909a 100644 --- a/tests/ui/useless_asref.rs +++ b/tests/ui/useless_asref.rs @@ -1,6 +1,7 @@ // run-rustfix #![deny(clippy::useless_asref)] +#![allow(clippy::explicit_auto_deref)] use std::fmt::Debug; diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr index 5876b54aca8f..b21c67bb3645 100644 --- a/tests/ui/useless_asref.stderr +++ b/tests/ui/useless_asref.stderr @@ -1,5 +1,5 @@ error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:43:18 + --> $DIR/useless_asref.rs:44:18 | LL | foo_rstr(rstr.as_ref()); | ^^^^^^^^^^^^^ help: try this: `rstr` @@ -11,61 +11,61 @@ LL | #![deny(clippy::useless_asref)] | ^^^^^^^^^^^^^^^^^^^^^ error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:45:20 + --> $DIR/useless_asref.rs:46:20 | LL | foo_rslice(rslice.as_ref()); | ^^^^^^^^^^^^^^^ help: try this: `rslice` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:49:21 + --> $DIR/useless_asref.rs:50:21 | LL | foo_mrslice(mrslice.as_mut()); | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:51:20 + --> $DIR/useless_asref.rs:52:20 | LL | foo_rslice(mrslice.as_ref()); | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:58:20 + --> $DIR/useless_asref.rs:59:20 | LL | foo_rslice(rrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^ help: try this: `rrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:60:18 + --> $DIR/useless_asref.rs:61:18 | LL | foo_rstr(rrrrrstr.as_ref()); | ^^^^^^^^^^^^^^^^^ help: try this: `rrrrrstr` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:65:21 + --> $DIR/useless_asref.rs:66:21 | LL | foo_mrslice(mrrrrrslice.as_mut()); | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:67:20 + --> $DIR/useless_asref.rs:68:20 | LL | foo_rslice(mrrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:71:16 + --> $DIR/useless_asref.rs:72:16 | LL | foo_rrrrmr((&&&&MoreRef).as_ref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&&&&MoreRef)` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:121:13 + --> $DIR/useless_asref.rs:122:13 | LL | foo_mrt(mrt.as_mut()); | ^^^^^^^^^^^^ help: try this: `mrt` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:123:12 + --> $DIR/useless_asref.rs:124:12 | LL | foo_rt(mrt.as_ref()); | ^^^^^^^^^^^^ help: try this: `mrt`