Skip to content

Commit 53d3ffe

Browse files
Y-NakY-Nak
committed
Move borrowed_box to its own module
1 parent cc3ab1c commit 53d3ffe

File tree

2 files changed

+127
-113
lines changed

2 files changed

+127
-113
lines changed

‎clippy_lints/src/types/borrowed_box.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir::{
3+
self as hir, GenericArg, GenericBounds, GenericParamKind, HirId, Lifetime, MutTy, Mutability, Node, QPath,
4+
SyntheticTyParamKind, TyKind,
5+
};
6+
use rustc_lint::LateContext;
7+
8+
use if_chain::if_chain;
9+
10+
use crate::utils::{match_path, paths, snippet, span_lint_and_sugg};
11+
12+
pub(super) fn check(
13+
cx: &LateContext<'_>,
14+
hir_ty: &hir::Ty<'_>,
15+
is_local: bool,
16+
lt: &Lifetime,
17+
mut_ty: &MutTy<'_>,
18+
) -> bool {
19+
match mut_ty.ty.kind {
20+
TyKind::Path(ref qpath) => {
21+
let hir_id = mut_ty.ty.hir_id;
22+
let def = cx.qpath_res(qpath, hir_id);
23+
if_chain! {
24+
if let Some(def_id) = def.opt_def_id();
25+
if Some(def_id) == cx.tcx.lang_items().owned_box();
26+
if let QPath::Resolved(None, ref path) = *qpath;
27+
if let [ref bx] = *path.segments;
28+
if let Some(ref params) = bx.args;
29+
if !params.parenthesized;
30+
if let Some(inner) = params.args.iter().find_map(|arg| match arg {
31+
GenericArg::Type(ty) => Some(ty),
32+
_ => None,
33+
});
34+
then {
35+
if is_any_trait(inner) {
36+
// Ignore `Box<Any>` types; see issue #1884 for details.
37+
return false;
38+
}
39+
40+
let ltopt = if lt.is_elided() {
41+
String::new()
42+
} else {
43+
format!("{} ", lt.name.ident().as_str())
44+
};
45+
46+
if mut_ty.mutbl == Mutability::Mut {
47+
// Ignore `&mut Box<T>` types; see issue #2907 for
48+
// details.
49+
return false;
50+
}
51+
52+
// When trait objects or opaque types have lifetime or auto-trait bounds,
53+
// we need to add parentheses to avoid a syntax error due to its ambiguity.
54+
// Originally reported as the issue #3128.
55+
let inner_snippet = snippet(cx, inner.span, "..");
56+
let suggestion = match &inner.kind {
57+
TyKind::TraitObject(bounds, lt_bound) if bounds.len() > 1 || !lt_bound.is_elided() => {
58+
format!("&{}({})", ltopt, &inner_snippet)
59+
},
60+
TyKind::Path(qpath)
61+
if get_bounds_if_impl_trait(cx, qpath, inner.hir_id)
62+
.map_or(false, |bounds| bounds.len() > 1) =>
63+
{
64+
format!("&{}({})", ltopt, &inner_snippet)
65+
},
66+
_ => format!("&{}{}", ltopt, &inner_snippet),
67+
};
68+
span_lint_and_sugg(
69+
cx,
70+
super::BORROWED_BOX,
71+
hir_ty.span,
72+
"you seem to be trying to use `&Box<T>`. Consider using just `&T`",
73+
"try",
74+
suggestion,
75+
// To make this `MachineApplicable`, at least one needs to check if it isn't a trait item
76+
// because the trait impls of it will break otherwise;
77+
// and there may be other cases that result in invalid code.
78+
// For example, type coercion doesn't work nicely.
79+
Applicability::Unspecified,
80+
);
81+
return true;
82+
}
83+
};
84+
false
85+
},
86+
_ => false,
87+
}
88+
}
89+
90+
// Returns true if given type is `Any` trait.
91+
fn is_any_trait(t: &hir::Ty<'_>) -> bool {
92+
if_chain! {
93+
if let TyKind::TraitObject(ref traits, _) = t.kind;
94+
if !traits.is_empty();
95+
// Only Send/Sync can be used as additional traits, so it is enough to
96+
// check only the first trait.
97+
if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT);
98+
then {
99+
return true;
100+
}
101+
}
102+
103+
false
104+
}
105+
106+
fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> {
107+
if_chain! {
108+
if let Some(did) = cx.qpath_res(qpath, id).opt_def_id();
109+
if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did);
110+
if let GenericParamKind::Type { synthetic, .. } = generic_param.kind;
111+
if synthetic == Some(SyntheticTyParamKind::ImplTrait);
112+
then {
113+
Some(generic_param.bounds)
114+
} else {
115+
None
116+
}
117+
}
118+
}

‎clippy_lints/src/types/mod.rs

Lines changed: 9 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![allow(rustc::default_hash_types)]
22

3+
mod borrowed_box;
34
mod box_vec;
45
mod linked_list;
56
mod option_option;
@@ -18,9 +19,9 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
1819
use rustc_hir as hir;
1920
use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
2021
use rustc_hir::{
21-
BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericBounds, GenericParamKind, HirId,
22-
ImplItem, ImplItemKind, Item, ItemKind, Lifetime, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt,
23-
StmtKind, SyntheticTyParamKind, TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
22+
BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem,
23+
ImplItemKind, Item, ItemKind, Lit, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind, TraitFn,
24+
TraitItem, TraitItemKind, TyKind, UnOp,
2425
};
2526
use rustc_lint::{LateContext, LateLintPass, LintContext};
2627
use rustc_middle::hir::map::Map;
@@ -376,7 +377,11 @@ impl Types {
376377
QPath::LangItem(..) => {},
377378
}
378379
},
379-
TyKind::Rptr(ref lt, ref mut_ty) => self.check_ty_rptr(cx, hir_ty, is_local, lt, mut_ty),
380+
TyKind::Rptr(ref lt, ref mut_ty) => {
381+
if !borrowed_box::check(cx, hir_ty, is_local, lt, mut_ty) {
382+
self.check_ty(cx, &mut_ty.ty, is_local);
383+
}
384+
},
380385
TyKind::Slice(ref ty) | TyKind::Array(ref ty, _) | TyKind::Ptr(MutTy { ref ty, .. }) => {
381386
self.check_ty(cx, ty, is_local)
382387
},
@@ -388,115 +393,6 @@ impl Types {
388393
_ => {},
389394
}
390395
}
391-
392-
fn check_ty_rptr(
393-
&mut self,
394-
cx: &LateContext<'_>,
395-
hir_ty: &hir::Ty<'_>,
396-
is_local: bool,
397-
lt: &Lifetime,
398-
mut_ty: &MutTy<'_>,
399-
) {
400-
match mut_ty.ty.kind {
401-
TyKind::Path(ref qpath) => {
402-
let hir_id = mut_ty.ty.hir_id;
403-
let def = cx.qpath_res(qpath, hir_id);
404-
if_chain! {
405-
if let Some(def_id) = def.opt_def_id();
406-
if Some(def_id) == cx.tcx.lang_items().owned_box();
407-
if let QPath::Resolved(None, ref path) = *qpath;
408-
if let [ref bx] = *path.segments;
409-
if let Some(ref params) = bx.args;
410-
if !params.parenthesized;
411-
if let Some(inner) = params.args.iter().find_map(|arg| match arg {
412-
GenericArg::Type(ty) => Some(ty),
413-
_ => None,
414-
});
415-
then {
416-
if is_any_trait(inner) {
417-
// Ignore `Box<Any>` types; see issue #1884 for details.
418-
return;
419-
}
420-
421-
let ltopt = if lt.is_elided() {
422-
String::new()
423-
} else {
424-
format!("{} ", lt.name.ident().as_str())
425-
};
426-
427-
if mut_ty.mutbl == Mutability::Mut {
428-
// Ignore `&mut Box<T>` types; see issue #2907 for
429-
// details.
430-
return;
431-
}
432-
433-
// When trait objects or opaque types have lifetime or auto-trait bounds,
434-
// we need to add parentheses to avoid a syntax error due to its ambiguity.
435-
// Originally reported as the issue #3128.
436-
let inner_snippet = snippet(cx, inner.span, "..");
437-
let suggestion = match &inner.kind {
438-
TyKind::TraitObject(bounds, lt_bound) if bounds.len() > 1 || !lt_bound.is_elided() => {
439-
format!("&{}({})", ltopt, &inner_snippet)
440-
},
441-
TyKind::Path(qpath)
442-
if get_bounds_if_impl_trait(cx, qpath, inner.hir_id)
443-
.map_or(false, |bounds| bounds.len() > 1) =>
444-
{
445-
format!("&{}({})", ltopt, &inner_snippet)
446-
},
447-
_ => format!("&{}{}", ltopt, &inner_snippet),
448-
};
449-
span_lint_and_sugg(
450-
cx,
451-
BORROWED_BOX,
452-
hir_ty.span,
453-
"you seem to be trying to use `&Box<T>`. Consider using just `&T`",
454-
"try",
455-
suggestion,
456-
// To make this `MachineApplicable`, at least one needs to check if it isn't a trait item
457-
// because the trait impls of it will break otherwise;
458-
// and there may be other cases that result in invalid code.
459-
// For example, type coercion doesn't work nicely.
460-
Applicability::Unspecified,
461-
);
462-
return; // don't recurse into the type
463-
}
464-
};
465-
self.check_ty(cx, &mut_ty.ty, is_local);
466-
},
467-
_ => self.check_ty(cx, &mut_ty.ty, is_local),
468-
}
469-
}
470-
}
471-
472-
// Returns true if given type is `Any` trait.
473-
fn is_any_trait(t: &hir::Ty<'_>) -> bool {
474-
if_chain! {
475-
if let TyKind::TraitObject(ref traits, _) = t.kind;
476-
if !traits.is_empty();
477-
// Only Send/Sync can be used as additional traits, so it is enough to
478-
// check only the first trait.
479-
if match_path(&traits[0].trait_ref.path, &paths::ANY_TRAIT);
480-
then {
481-
return true;
482-
}
483-
}
484-
485-
false
486-
}
487-
488-
fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option<GenericBounds<'tcx>> {
489-
if_chain! {
490-
if let Some(did) = cx.qpath_res(qpath, id).opt_def_id();
491-
if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did);
492-
if let GenericParamKind::Type { synthetic, .. } = generic_param.kind;
493-
if synthetic == Some(SyntheticTyParamKind::ImplTrait);
494-
then {
495-
Some(generic_param.bounds)
496-
} else {
497-
None
498-
}
499-
}
500396
}
501397

502398
declare_clippy_lint! {

0 commit comments

Comments
 (0)