Skip to content

Commit 1e861a2

Browse files
flip1995flip1995
committed
Merge remote-tracking branch 'upstream/master' into rustup2
2 parents 3c5ee33 + 88fec89 commit 1e861a2

28 files changed

+693
-142
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,6 +1508,7 @@ Released 2018-09-13
15081508
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
15091509
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
15101510
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
1511+
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
15111512
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
15121513
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
15131514
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool

‎clippy_lints/src/await_holding_lock.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ declare_clippy_lint! {
1111
/// non-async-aware MutexGuard.
1212
///
1313
/// **Why is this bad?** The Mutex types found in syd::sync and parking_lot
14-
/// are not designed to operator in an async context across await points.
14+
/// are not designed to operate in an async context across await points.
1515
///
1616
/// There are two potential solutions. One is to use an asynx-aware Mutex
1717
/// type. Many asynchronous foundation crates provide such a Mutex type. The

‎clippy_lints/src/dereference.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
7373
fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
7474
match method_name {
7575
"deref" => {
76-
if cx.tcx.lang_items().deref_trait().map_or(false, |id| {
76+
let impls_deref_trait = cx.tcx.lang_items().deref_trait().map_or(false, |id| {
7777
implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
78-
}) {
78+
});
79+
if impls_deref_trait {
7980
span_lint_and_sugg(
8081
cx,
8182
EXPLICIT_DEREF_METHODS,
@@ -88,9 +89,10 @@ fn lint_deref(cx: &LateContext<'_, '_>, method_name: &str, call_expr: &Expr<'_>,
8889
}
8990
},
9091
"deref_mut" => {
91-
if cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
92+
let impls_deref_mut_trait = cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
9293
implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
93-
}) {
94+
});
95+
if impls_deref_mut_trait {
9496
span_lint_and_sugg(
9597
cx,
9698
EXPLICIT_DEREF_METHODS,

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ mod main_recursion;
229229
mod manual_async_fn;
230230
mod manual_non_exhaustive;
231231
mod map_clone;
232+
mod map_identity;
232233
mod map_unit_fn;
233234
mod match_on_vec_items;
234235
mod matches;
@@ -608,6 +609,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
608609
&manual_async_fn::MANUAL_ASYNC_FN,
609610
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
610611
&map_clone::MAP_CLONE,
612+
&map_identity::MAP_IDENTITY,
611613
&map_unit_fn::OPTION_MAP_UNIT_FN,
612614
&map_unit_fn::RESULT_MAP_UNIT_FN,
613615
&match_on_vec_items::MATCH_ON_VEC_ITEMS,
@@ -1057,6 +1059,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10571059
});
10581060
store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns);
10591061
store.register_late_pass(|| box macro_use::MacroUseImports::default());
1062+
store.register_late_pass(|| box map_identity::MapIdentity);
10601063

10611064
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10621065
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1273,6 +1276,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12731276
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
12741277
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
12751278
LintId::of(&map_clone::MAP_CLONE),
1279+
LintId::of(&map_identity::MAP_IDENTITY),
12761280
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
12771281
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
12781282
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
@@ -1550,6 +1554,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15501554
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
15511555
LintId::of(&loops::MUT_RANGE_BOUND),
15521556
LintId::of(&loops::WHILE_LET_LOOP),
1557+
LintId::of(&map_identity::MAP_IDENTITY),
15531558
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
15541559
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
15551560
LintId::of(&matches::MATCH_AS_REF),

‎clippy_lints/src/map_identity.rs

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
use crate::utils::{
2+
is_adjusted, is_type_diagnostic_item, match_path, match_trait_method, match_var, paths, remove_blocks,
3+
span_lint_and_sugg,
4+
};
5+
use if_chain::if_chain;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Body, Expr, ExprKind, Pat, PatKind, QPath, StmtKind};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
11+
declare_clippy_lint! {
12+
/// **What it does:** Checks for instances of `map(f)` where `f` is the identity function.
13+
///
14+
/// **Why is this bad?** It can be written more concisely without the call to `map`.
15+
///
16+
/// **Known problems:** None.
17+
///
18+
/// **Example:**
19+
///
20+
/// ```rust
21+
/// let x = [1, 2, 3];
22+
/// let y: Vec<_> = x.iter().map(|x| x).map(|x| 2*x).collect();
23+
/// ```
24+
/// Use instead:
25+
/// ```rust
26+
/// let x = [1, 2, 3];
27+
/// let y: Vec<_> = x.iter().map(|x| 2*x).collect();
28+
/// ```
29+
pub MAP_IDENTITY,
30+
complexity,
31+
"using iterator.map(|x| x)"
32+
}
33+
34+
declare_lint_pass!(MapIdentity => [MAP_IDENTITY]);
35+
36+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapIdentity {
37+
fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
38+
if expr.span.from_expansion() {
39+
return;
40+
}
41+
42+
if_chain! {
43+
if let Some([caller, func]) = get_map_argument(cx, expr);
44+
if is_expr_identity_function(cx, func);
45+
then {
46+
span_lint_and_sugg(
47+
cx,
48+
MAP_IDENTITY,
49+
expr.span.trim_start(caller.span).unwrap(),
50+
"unnecessary map of the identity function",
51+
"remove the call to `map`",
52+
String::new(),
53+
Applicability::MachineApplicable
54+
)
55+
}
56+
}
57+
}
58+
}
59+
60+
/// Returns the arguments passed into map() if the expression is a method call to
61+
/// map(). Otherwise, returns None.
62+
fn get_map_argument<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option<&'a [Expr<'a>]> {
63+
if_chain! {
64+
if let ExprKind::MethodCall(ref method, _, ref args, _) = expr.kind;
65+
if args.len() == 2 && method.ident.as_str() == "map";
66+
let caller_ty = cx.tables().expr_ty(&args[0]);
67+
if match_trait_method(cx, expr, &paths::ITERATOR)
68+
|| is_type_diagnostic_item(cx, caller_ty, sym!(result_type))
69+
|| is_type_diagnostic_item(cx, caller_ty, sym!(option_type));
70+
then {
71+
Some(args)
72+
} else {
73+
None
74+
}
75+
}
76+
}
77+
78+
/// Checks if an expression represents the identity function
79+
/// Only examines closures and `std::convert::identity`
80+
fn is_expr_identity_function(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
81+
match expr.kind {
82+
ExprKind::Closure(_, _, body_id, _, _) => is_body_identity_function(cx, cx.tcx.hir().body(body_id)),
83+
ExprKind::Path(QPath::Resolved(_, ref path)) => match_path(path, &paths::STD_CONVERT_IDENTITY),
84+
_ => false,
85+
}
86+
}
87+
88+
/// Checks if a function's body represents the identity function
89+
/// Looks for bodies of the form `|x| x`, `|x| return x`, `|x| { return x }` or `|x| {
90+
/// return x; }`
91+
fn is_body_identity_function(cx: &LateContext<'_, '_>, func: &Body<'_>) -> bool {
92+
let params = func.params;
93+
let body = remove_blocks(&func.value);
94+
95+
// if there's less/more than one parameter, then it is not the identity function
96+
if params.len() != 1 {
97+
return false;
98+
}
99+
100+
match body.kind {
101+
ExprKind::Path(QPath::Resolved(None, _)) => match_expr_param(cx, body, params[0].pat),
102+
ExprKind::Ret(Some(ref ret_val)) => match_expr_param(cx, ret_val, params[0].pat),
103+
ExprKind::Block(ref block, _) => {
104+
if_chain! {
105+
if block.stmts.len() == 1;
106+
if let StmtKind::Semi(ref expr) | StmtKind::Expr(ref expr) = block.stmts[0].kind;
107+
if let ExprKind::Ret(Some(ref ret_val)) = expr.kind;
108+
then {
109+
match_expr_param(cx, ret_val, params[0].pat)
110+
} else {
111+
false
112+
}
113+
}
114+
},
115+
_ => false,
116+
}
117+
}
118+
119+
/// Returns true iff an expression returns the same thing as a parameter's pattern
120+
fn match_expr_param(cx: &LateContext<'_, '_>, expr: &Expr<'_>, pat: &Pat<'_>) -> bool {
121+
if let PatKind::Binding(_, _, ident, _) = pat.kind {
122+
match_var(expr, ident.name) && !(cx.tables().hir_owner == Some(expr.hir_id.owner) && is_adjusted(cx, expr))
123+
} else {
124+
false
125+
}
126+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2044,7 +2044,7 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, arg: &hir:
20442044
}
20452045
span_lint_and_then(cx, CLONE_ON_COPY, expr.span, "using `clone` on a `Copy` type", |diag| {
20462046
if let Some((text, snip)) = snip {
2047-
diag.span_suggestion(expr.span, text, snip, Applicability::Unspecified);
2047+
diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable);
20482048
}
20492049
});
20502050
}

‎clippy_lints/src/misc.rs

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ use rustc_ast::ast::LitKind;
33
use rustc_errors::Applicability;
44
use rustc_hir::intravisit::FnKind;
55
use rustc_hir::{
6-
def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt, StmtKind, Ty,
7-
TyKind, UnOp,
6+
self as hir, def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt,
7+
StmtKind, TyKind, UnOp,
88
};
99
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty;
10+
use rustc_middle::ty::{self, Ty};
1111
use rustc_session::{declare_lint_pass, declare_tool_lint};
1212
use rustc_span::hygiene::DesugaringKind;
1313
use rustc_span::source_map::{ExpnKind, Span};
@@ -371,8 +371,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
371371
if op.is_comparison() {
372372
check_nan(cx, left, expr);
373373
check_nan(cx, right, expr);
374-
check_to_owned(cx, left, right);
375-
check_to_owned(cx, right, left);
374+
check_to_owned(cx, left, right, true);
375+
check_to_owned(cx, right, left, false);
376376
}
377377
if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) {
378378
if is_allowed(cx, left) || is_allowed(cx, right) {
@@ -570,19 +570,38 @@ fn is_array(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
570570
matches!(&walk_ptrs_ty(cx.tables().expr_ty(expr)).kind, ty::Array(_, _))
571571
}
572572

573-
fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) {
573+
fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool) {
574+
#[derive(Default)]
575+
struct EqImpl {
576+
ty_eq_other: bool,
577+
other_eq_ty: bool,
578+
}
579+
580+
impl EqImpl {
581+
fn is_implemented(&self) -> bool {
582+
self.ty_eq_other || self.other_eq_ty
583+
}
584+
}
585+
586+
fn symmetric_partial_eq<'tcx>(cx: &LateContext<'_, 'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> Option<EqImpl> {
587+
cx.tcx.lang_items().eq_trait().map(|def_id| EqImpl {
588+
ty_eq_other: implements_trait(cx, ty, def_id, &[other.into()]),
589+
other_eq_ty: implements_trait(cx, other, def_id, &[ty.into()]),
590+
})
591+
}
592+
574593
let (arg_ty, snip) = match expr.kind {
575594
ExprKind::MethodCall(.., ref args, _) if args.len() == 1 => {
576595
if match_trait_method(cx, expr, &paths::TO_STRING) || match_trait_method(cx, expr, &paths::TO_OWNED) {
577-
(cx.tables().expr_ty_adjusted(&args[0]), snippet(cx, args[0].span, ".."))
596+
(cx.tables().expr_ty(&args[0]), snippet(cx, args[0].span, ".."))
578597
} else {
579598
return;
580599
}
581600
},
582601
ExprKind::Call(ref path, ref v) if v.len() == 1 => {
583602
if let ExprKind::Path(ref path) = path.kind {
584603
if match_qpath(path, &["String", "from_str"]) || match_qpath(path, &["String", "from"]) {
585-
(cx.tables().expr_ty_adjusted(&v[0]), snippet(cx, v[0].span, ".."))
604+
(cx.tables().expr_ty(&v[0]), snippet(cx, v[0].span, ".."))
586605
} else {
587606
return;
588607
}
@@ -593,28 +612,19 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) {
593612
_ => return,
594613
};
595614

596-
let other_ty = cx.tables().expr_ty_adjusted(other);
597-
let partial_eq_trait_id = match cx.tcx.lang_items().eq_trait() {
598-
Some(id) => id,
599-
None => return,
600-
};
615+
let other_ty = cx.tables().expr_ty(other);
601616

602-
let deref_arg_impl_partial_eq_other = arg_ty.builtin_deref(true).map_or(false, |tam| {
603-
implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty.into()])
604-
});
605-
let arg_impl_partial_eq_deref_other = other_ty.builtin_deref(true).map_or(false, |tam| {
606-
implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty.into()])
607-
});
608-
let arg_impl_partial_eq_other = implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty.into()]);
617+
let without_deref = symmetric_partial_eq(cx, arg_ty, other_ty).unwrap_or_default();
618+
let with_deref = arg_ty
619+
.builtin_deref(true)
620+
.and_then(|tam| symmetric_partial_eq(cx, tam.ty, other_ty))
621+
.unwrap_or_default();
609622

610-
if !deref_arg_impl_partial_eq_other && !arg_impl_partial_eq_deref_other && !arg_impl_partial_eq_other {
623+
if !with_deref.is_implemented() && !without_deref.is_implemented() {
611624
return;
612625
}
613626

614-
let other_gets_derefed = match other.kind {
615-
ExprKind::Unary(UnOp::UnDeref, _) => true,
616-
_ => false,
617-
};
627+
let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::UnDeref, _));
618628

619629
let lint_span = if other_gets_derefed {
620630
expr.span.to(other.span)
@@ -634,18 +644,34 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) {
634644
return;
635645
}
636646

637-
let try_hint = if deref_arg_impl_partial_eq_other {
638-
// suggest deref on the left
639-
format!("*{}", snip)
647+
let expr_snip;
648+
let eq_impl;
649+
if with_deref.is_implemented() {
650+
expr_snip = format!("*{}", snip);
651+
eq_impl = with_deref;
640652
} else {
641-
// suggest dropping the to_owned on the left
642-
snip.to_string()
653+
expr_snip = snip.to_string();
654+
eq_impl = without_deref;
643655
};
644656

657+
let span;
658+
let hint;
659+
if (eq_impl.ty_eq_other && left) || (eq_impl.other_eq_ty && !left) {
660+
span = expr.span;
661+
hint = expr_snip;
662+
} else {
663+
span = expr.span.to(other.span);
664+
if eq_impl.ty_eq_other {
665+
hint = format!("{} == {}", expr_snip, snippet(cx, other.span, ".."));
666+
} else {
667+
hint = format!("{} == {}", snippet(cx, other.span, ".."), expr_snip);
668+
}
669+
}
670+
645671
diag.span_suggestion(
646-
lint_span,
672+
span,
647673
"try",
648-
try_hint,
674+
hint,
649675
Applicability::MachineApplicable, // snippet
650676
);
651677
},
@@ -694,7 +720,7 @@ fn non_macro_local(cx: &LateContext<'_, '_>, res: def::Res) -> bool {
694720
}
695721
}
696722

697-
fn check_cast(cx: &LateContext<'_, '_>, span: Span, e: &Expr<'_>, ty: &Ty<'_>) {
723+
fn check_cast(cx: &LateContext<'_, '_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) {
698724
if_chain! {
699725
if let TyKind::Ptr(ref mut_ty) = ty.kind;
700726
if let ExprKind::Lit(ref lit) = e.kind;

‎clippy_lints/src/needless_pass_by_value.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@ declare_clippy_lint! {
4040
/// assert_eq!(v.len(), 42);
4141
/// }
4242
/// ```
43-
///
43+
/// should be
4444
/// ```rust
45-
/// // should be
4645
/// fn foo(v: &[i32]) {
4746
/// assert_eq!(v.len(), 42);
4847
/// }

0 commit comments

Comments
 (0)