diff options
| -rw-r--r-- | clippy_lints/src/types/borrowed_box.rs | 118 | ||||
| -rw-r--r-- | clippy_lints/src/types/mod.rs | 122 |
2 files changed, 127 insertions, 113 deletions
diff --git a/clippy_lints/src/types/borrowed_box.rs b/clippy_lints/src/types/borrowed_box.rs new file mode 100644 index 00000000000..4c2cad0ee94 --- /dev/null +++ b/clippy_lints/src/types/borrowed_box.rs @@ -0,0 +1,118 @@ +use rustc_errors::Applicability; +use rustc_hir::{ + self as hir, GenericArg, GenericBounds, GenericParamKind, HirId, Lifetime, MutTy, Mutability, Node, QPath, + SyntheticTyParamKind, TyKind, +}; +use rustc_lint::LateContext; + +use if_chain::if_chain; + +use crate::utils::{match_path, paths, snippet, span_lint_and_sugg}; + +pub(super) fn check( + cx: &LateContext<'_>, + hir_ty: &hir::Ty<'_>, + is_local: bool, + lt: &Lifetime, + mut_ty: &MutTy<'_>, +) -> bool { + match mut_ty.ty.kind { + TyKind::Path(ref qpath) => { + let hir_id = mut_ty.ty.hir_id; + let def = cx.qpath_res(qpath, hir_id); + if_chain! { + if let Some(def_id) = def.opt_def_id(); + if Some(def_id) == cx.tcx.lang_items().owned_box(); + if let QPath::Resolved(None, ref path) = *qpath; + if let [ref bx] = *path.segments; + if let Some(ref params) = bx.args; + if !params.parenthesized; + if let Some(inner) = params.args.iter().find_map(|arg| match arg { + GenericArg::Type(ty) => Some(ty), + _ => None, + }); + then { + if is_any_trait(inner) { + // Ignore `Box<Any>` types; see issue #1884 for details. + return false; + } + + let ltopt = if lt.is_elided() { + String::new() + } else { + format!("{} ", lt.name.ident().as_str()) + }; + + if mut_ty.mutbl == Mutability::Mut { + // Ignore `&mut Box<T>` types; see issue #2907 for + // details. + return false; + } + + // When trait objects or opaque types have lifetime or auto-trait bounds, + // we need to add parentheses to avoid a syntax error due to its ambiguity. + // Originally reported as the issue #3128. + let inner_snippet = snippet(cx, inner.span, ".."); + let suggestion = match &inner.kind { + TyKind::TraitObject(bounds, lt_bound) if bounds.len() > 1 || !lt_bound.is_elided() => { + format!("&{}({})", ltopt, &inner_snippet) + }, + TyKind::Path(qpath) + if get_bounds_if_impl_trait(cx, qpath, inner.hir_id) + .map_or(false, |bounds| bounds.len() > 1) => + { + format!("&{}({})", ltopt, &inner_snippet) + }, + _ => format!("&{}{}", ltopt, &inner_snippet), + }; + span_lint_and_sugg( + cx, + super::BORROWED_BOX, + hir_ty.span, + "you seem to be trying to use `&Box<T>`. Consider using just `&T`", + "try", + suggestion, + // To make this `MachineApplicable`, at least one needs to check if it isn't a trait item + // because the trait impls of it will break otherwise; + // and there may be other cases that result in invalid code. + // For example, type coercion doesn't work nicely. + Applicability::Unspecified, + ); + return true; + } + }; + false + }, + _ => false, + } +} + +// Returns true if given type is `Any` trait. +fn is_any_trait(t: &hir::Ty<'_>) -> bool { + if_chain! { + if let TyKind::TraitObject(ref traits, _) = t.kind; + if !traits.is_empty(); + // Only Send/Sync can be used as additional traits, so it is enough to + // check only the first trait. + if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT); + then { + return true; + } + } + + false +} + +fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> { + if_chain! { + if let Some(did) = cx.qpath_res(qpath, id).opt_def_id(); + if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did); + if let GenericParamKind::Type { synthetic, .. } = generic_param.kind; + if synthetic == Some(SyntheticTyParamKind::ImplTrait); + then { + Some(generic_param.bounds) + } else { + None + } + } +} diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 86a12b8ffa9..8c2fc553085 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -1,5 +1,6 @@ #![allow(rustc::default_hash_types)] +mod borrowed_box; mod box_vec; mod linked_list; mod option_option; @@ -18,9 +19,9 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ - BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericBounds, GenericParamKind, HirId, - ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, - StmtKind, SyntheticTyParamKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp, + BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem, + ImplItemKind, Item, ItemKind, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, TraitFn, + TraitItem, TraitItemKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; @@ -376,7 +377,11 @@ impl Types { QPath::LangItem(..) => {}, } }, - TyKind::Rptr(ref lt, ref mut_ty) => self.check_ty_rptr(cx, hir_ty, is_local, lt, mut_ty), + TyKind::Rptr(ref lt, ref mut_ty) => { + if !borrowed_box::check(cx, hir_ty, is_local, lt, mut_ty) { + self.check_ty(cx, &mut_ty.ty, is_local); + } + }, TyKind::Slice(ref ty) | TyKind::Array(ref ty, _) | TyKind::Ptr(MutTy { ref ty, .. }) => { self.check_ty(cx, ty, is_local) }, @@ -388,115 +393,6 @@ impl Types { _ => {}, } } - - fn check_ty_rptr( - &mut self, - cx: &LateContext<'_>, - hir_ty: &hir::Ty<'_>, - is_local: bool, - lt: &Lifetime, - mut_ty: &MutTy<'_>, - ) { - match mut_ty.ty.kind { - TyKind::Path(ref qpath) => { - let hir_id = mut_ty.ty.hir_id; - let def = cx.qpath_res(qpath, hir_id); - if_chain! { - if let Some(def_id) = def.opt_def_id(); - if Some(def_id) == cx.tcx.lang_items().owned_box(); - if let QPath::Resolved(None, ref path) = *qpath; - if let [ref bx] = *path.segments; - if let Some(ref params) = bx.args; - if !params.parenthesized; - if let Some(inner) = params.args.iter().find_map(|arg| match arg { - GenericArg::Type(ty) => Some(ty), - _ => None, - }); - then { - if is_any_trait(inner) { - // Ignore `Box<Any>` types; see issue #1884 for details. - return; - } - - let ltopt = if lt.is_elided() { - String::new() - } else { - format!("{} ", lt.name.ident().as_str()) - }; - - if mut_ty.mutbl == Mutability::Mut { - // Ignore `&mut Box<T>` types; see issue #2907 for - // details. - return; - } - - // When trait objects or opaque types have lifetime or auto-trait bounds, - // we need to add parentheses to avoid a syntax error due to its ambiguity. - // Originally reported as the issue #3128. - let inner_snippet = snippet(cx, inner.span, ".."); - let suggestion = match &inner.kind { - TyKind::TraitObject(bounds, lt_bound) if bounds.len() > 1 || !lt_bound.is_elided() => { - format!("&{}({})", ltopt, &inner_snippet) - }, - TyKind::Path(qpath) - if get_bounds_if_impl_trait(cx, qpath, inner.hir_id) - .map_or(false, |bounds| bounds.len() > 1) => - { - format!("&{}({})", ltopt, &inner_snippet) - }, - _ => format!("&{}{}", ltopt, &inner_snippet), - }; - span_lint_and_sugg( - cx, - BORROWED_BOX, - hir_ty.span, - "you seem to be trying to use `&Box<T>`. Consider using just `&T`", - "try", - suggestion, - // To make this `MachineApplicable`, at least one needs to check if it isn't a trait item - // because the trait impls of it will break otherwise; - // and there may be other cases that result in invalid code. - // For example, type coercion doesn't work nicely. - Applicability::Unspecified, - ); - return; // don't recurse into the type - } - }; - self.check_ty(cx, &mut_ty.ty, is_local); - }, - _ => self.check_ty(cx, &mut_ty.ty, is_local), - } - } -} - -// Returns true if given type is `Any` trait. -fn is_any_trait(t: &hir::Ty<'_>) -> bool { - if_chain! { - if let TyKind::TraitObject(ref traits, _) = t.kind; - if !traits.is_empty(); - // Only Send/Sync can be used as additional traits, so it is enough to - // check only the first trait. - if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT); - then { - return true; - } - } - - false -} - -fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> { - if_chain! { - if let Some(did) = cx.qpath_res(qpath, id).opt_def_id(); - if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did); - if let GenericParamKind::Type { synthetic, .. } = generic_param.kind; - if synthetic == Some(SyntheticTyParamKind::ImplTrait); - then { - Some(generic_param.bounds) - } else { - None - } - } } declare_clippy_lint! { |