Skip to content

Commit

Permalink
Auto merge of #6725 - Y-Nak:refactor-types-lints, r=flip1995
Browse files Browse the repository at this point in the history
Refactor types lints

Ref #6724.
As described in #6724, `types.rs` contains many groups inside it.
In this PR, I reorganize the lints of the `types` group into their own modules.

changelog: none
  • Loading branch information
bors committed Mar 8, 2021
2 parents d0d5232 + db59c35 commit b207f23
Show file tree
Hide file tree
Showing 9 changed files with 485 additions and 357 deletions.
114 changes: 114 additions & 0 deletions clippy_lints/src/types/borrowed_box.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
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};

use super::BORROWED_BOX;

pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, 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,
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
}
}
}
25 changes: 25 additions & 0 deletions clippy_lints/src/types/box_vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
use rustc_hir::{self as hir, def_id::DefId, QPath};
use rustc_lint::LateContext;
use rustc_span::symbol::sym;

use crate::utils::{is_ty_param_diagnostic_item, span_lint_and_help};

use super::BOX_VEC;

pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool {
if Some(def_id) == cx.tcx.lang_items().owned_box()
&& is_ty_param_diagnostic_item(cx, qpath, sym::vec_type).is_some()
{
span_lint_and_help(
cx,
BOX_VEC,
hir_ty.span,
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
None,
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation",
);
true
} else {
false
}
}
22 changes: 22 additions & 0 deletions clippy_lints/src/types/linked_list.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use rustc_hir::{self as hir, def_id::DefId};
use rustc_lint::LateContext;

use crate::utils::{match_def_path, paths, span_lint_and_help};

use super::LINKEDLIST;

pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, def_id: DefId) -> bool {
if match_def_path(cx, def_id, &paths::LINKED_LIST) {
span_lint_and_help(
cx,
LINKEDLIST,
hir_ty.span,
"you seem to be using a `LinkedList`! Perhaps you meant some other data structure?",
None,
"a `VecDeque` might work",
);
true
} else {
false
}
}
Loading

0 comments on commit b207f23

Please sign in to comment.