-use crate::utils::{is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then};
+use crate::utils::{is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then, type_is_unsafe_function};
+use if_chain::if_chain;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::ty;
if let ExprKind::Closure(_, ref decl, eid, _, _) = expr.node {
let body = cx.tcx.hir().body(eid);
let ex = &body.value;
- if let ExprKind::Call(ref caller, ref args) = ex.node {
- if args.len() != decl.inputs.len() {
- // Not the same number of arguments, there
- // is no way the closure is the same as the function
- return;
- }
- if is_adjusted(cx, ex) || args.iter().any(|arg| is_adjusted(cx, arg)) {
- // Are the expression or the arguments type-adjusted? Then we need the closure
- return;
- }
+
+ if_chain!(
+ if let ExprKind::Call(ref caller, ref args) = ex.node;
+
+ // Not the same number of arguments, there is no way the closure is the same as the function return;
+ if args.len() == decl.inputs.len();
+
+ // Are the expression or the arguments type-adjusted? Then we need the closure
+ if !(is_adjusted(cx, ex) || args.iter().any(|arg| is_adjusted(cx, arg)));
+
let fn_ty = cx.tables.expr_ty(caller);
- match fn_ty.sty {
- // Is it an unsafe function? They don't implement the closure traits
- ty::FnDef(..) | ty::FnPtr(_) => {
- let sig = fn_ty.fn_sig(cx.tcx);
- if sig.skip_binder().unsafety == Unsafety::Unsafe || sig.skip_binder().output().sty == ty::Never {
- return;
+ if !type_is_unsafe_function(cx, fn_ty);
+
+ if compare_inputs(&mut iter_input_pats(decl, body), &mut args.into_iter());
+
+ then {
+ span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| {
+ if let Some(snippet) = snippet_opt(cx, caller.span) {
+ db.span_suggestion(
+ expr.span,
+ "remove closure as shown",
+ snippet,
+ Applicability::MachineApplicable,
+ );
}
- },
- _ => (),
+ });
}
- for (a1, a2) in iter_input_pats(decl, body).zip(args) {
- if let PatKind::Binding(.., ident, _) = a1.pat.node {
- // XXXManishearth Should I be checking the binding mode here?
- if let ExprKind::Path(QPath::Resolved(None, ref p)) = a2.node {
- if p.segments.len() != 1 {
- // If it's a proper path, it can't be a local variable
- return;
- }
- if p.segments[0].ident.name != ident.name {
- // The two idents should be the same
- return;
- }
- } else {
- return;
- }
- } else {
- return;
- }
- }
- span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| {
- if let Some(snippet) = snippet_opt(cx, caller.span) {
+ );
+
+ if_chain!(
+ if let ExprKind::MethodCall(ref path, _, ref args) = ex.node;
+
+ // Not the same number of arguments, there is no way the closure is the same as the function return;
+ if args.len() == decl.inputs.len();
+
+ // Are the expression or the arguments type-adjusted? Then we need the closure
+ if !(is_adjusted(cx, ex) || args.iter().skip(1).any(|arg| is_adjusted(cx, arg)));
+
+ let method_def_id = cx.tables.type_dependent_defs()[ex.hir_id].def_id();
+ if !type_is_unsafe_function(cx, cx.tcx.type_of(method_def_id));
+
+ if compare_inputs(&mut iter_input_pats(decl, body), &mut args.into_iter());
+
+ if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]);
+
+ then {
+ span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| {
db.span_suggestion(
expr.span,
"remove closure as shown",
- snippet,
+ format!("{}::{}", name, path.ident.name),
Applicability::MachineApplicable,
);
+ });
+ }
+ );
+ }
+}
+
+/// Tries to determine the type for universal function call to be used instead of the closure
+fn get_ufcs_type_name(
+ cx: &LateContext<'_, '_>,
+ method_def_id: def_id::DefId,
+ self_arg: &Expr,
+) -> std::option::Option<String> {
+ let expected_type_of_self = &cx.tcx.fn_sig(method_def_id).inputs_and_output().skip_binder()[0].sty;
+ let actual_type_of_self = &cx.tables.node_id_to_type(self_arg.hir_id).sty;
+
+ if let Some(trait_id) = cx.tcx.trait_of_item(method_def_id) {
+ //if the method expectes &self, ufcs requires explicit borrowing so closure can't be removed
+ return match (expected_type_of_self, actual_type_of_self) {
+ (ty::Ref(_, _, _), ty::Ref(_, _, _)) => Some(cx.tcx.item_path_str(trait_id)),
+ (l, r) => match (l, r) {
+ (ty::Ref(_, _, _), _) | (_, ty::Ref(_, _, _)) => None,
+ (_, _) => Some(cx.tcx.item_path_str(trait_id)),
+ },
+ };
+ }
+
+ cx.tcx.impl_of_method(method_def_id).and_then(|_| {
+ //a type may implicitly implement other types methods (e.g. Deref)
+ if match_types(expected_type_of_self, actual_type_of_self) {
+ return Some(get_type_name(cx, &actual_type_of_self));
+ }
+ None
+ })
+}
+
+fn match_types(lhs: &ty::TyKind<'_>, rhs: &ty::TyKind<'_>) -> bool {
+ match (lhs, rhs) {
+ (ty::Bool, ty::Bool)
+ | (ty::Char, ty::Char)
+ | (ty::Int(_), ty::Int(_))
+ | (ty::Uint(_), ty::Uint(_))
+ | (ty::Str, ty::Str) => true,
+ (ty::Ref(_, t1, _), ty::Ref(_, t2, _))
+ | (ty::Array(t1, _), ty::Array(t2, _))
+ | (ty::Slice(t1), ty::Slice(t2)) => match_types(&t1.sty, &t2.sty),
+ (ty::Adt(def1, _), ty::Adt(def2, _)) => def1 == def2,
+ (_, _) => false,
+ }
+}
+
+fn get_type_name(cx: &LateContext<'_, '_>, kind: &ty::TyKind<'_>) -> String {
+ match kind {
+ ty::Adt(t, _) => cx.tcx.item_path_str(t.did),
+ ty::Ref(_, r, _) => get_type_name(cx, &r.sty),
+ _ => kind.to_string(),
+ }
+}
+
+fn compare_inputs(closure_inputs: &mut dyn Iterator<Item = &Arg>, call_args: &mut dyn Iterator<Item = &Expr>) -> bool {
+ for (closure_input, function_arg) in closure_inputs.zip(call_args) {
+ if let PatKind::Binding(_, _, _, ident, _) = closure_input.pat.node {
+ // XXXManishearth Should I be checking the binding mode here?
+ if let ExprKind::Path(QPath::Resolved(None, ref p)) = function_arg.node {
+ if p.segments.len() != 1 {
+ // If it's a proper path, it can't be a local variable
+ return false;
}
- });
+ if p.segments[0].ident.name != ident.name {
+ // The two idents should be the same
+ return false;
+ }
+ } else {
+ return false;
+ }
+ } else {
+ return false;
}
}
+ true
}
)]
#![warn(clippy::redundant_closure, clippy::needless_borrow)]
+use std::path::PathBuf;
+
fn main() {
let a = Some(1u8).map(|a| foo(a));
meta(|a| foo(a));
// See #515
let a: Option<Box<::std::ops::Deref<Target = [i32]>>> =
Some(vec![1i32, 2]).map(|v| -> Box<::std::ops::Deref<Target = [i32]>> { Box::new(v) });
+
+}
+
+trait TestTrait {
+ fn trait_foo(self) -> bool;
+ fn trait_foo_ref(&self) -> bool;
+}
+
+struct TestStruct<'a> {
+ some_ref: &'a i32
+}
+
+impl<'a> TestStruct<'a> {
+ fn foo(self) -> bool { false }
+ unsafe fn foo_unsafe(self) -> bool { true }
+}
+
+impl<'a> TestTrait for TestStruct<'a> {
+ fn trait_foo(self) -> bool { false }
+ fn trait_foo_ref(&self) -> bool { false }
+}
+
+impl<'a> std::ops::Deref for TestStruct<'a> {
+ type Target = char;
+ fn deref(&self) -> &char { &'a' }
+}
+
+fn test_redundant_closures_containing_method_calls() {
+ let i = 10;
+ let e = Some(TestStruct{some_ref: &i}).map(|a| a.foo());
+ let e = Some(TestStruct{some_ref: &i}).map(TestStruct::foo);
+ let e = Some(TestStruct{some_ref: &i}).map(|a| a.trait_foo());
+ let e = Some(TestStruct{some_ref: &i}).map(|a| a.trait_foo_ref());
+ let e = Some(TestStruct{some_ref: &i}).map(TestTrait::trait_foo);
+ let e = Some(&mut vec!(1,2,3)).map(|v| v.clear());
+ let e = Some(&mut vec!(1,2,3)).map(std::vec::Vec::clear);
+ unsafe {
+ let e = Some(TestStruct{some_ref: &i}).map(|a| a.foo_unsafe());
+ }
+ let e = Some("str").map(|s| s.to_string());
+ let e = Some("str").map(str::to_string);
+ let e = Some('a').map(|s| s.to_uppercase());
+ let e = Some('a').map(char::to_uppercase);
+ let e: std::vec::Vec<usize> = vec!('a','b','c').iter().map(|c| c.len_utf8()).collect();
+ let e: std::vec::Vec<char> = vec!('a','b','c').iter().map(|c| c.to_ascii_uppercase()).collect();
+ let e: std::vec::Vec<char> = vec!('a','b','c').iter().map(char::to_ascii_uppercase).collect();
+ let p = Some(PathBuf::new());
+ let e = p.as_ref().and_then(|s| s.to_str());
+ //let e = p.as_ref().and_then(std::path::Path::to_str);
+ let c = Some(TestStruct{some_ref: &i}).as_ref().map(|c| c.to_ascii_uppercase());
+ //let c = Some(TestStruct{some_ref: &i}).as_ref().map(char::to_ascii_uppercase);
}
fn meta<F>(f: F)
fn generic<T>(_: T) -> u8 {
0
}
+
error: redundant closure found
- --> $DIR/eta.rs:13:27
+ --> $DIR/eta.rs:15:27
|
LL | let a = Some(1u8).map(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo`
= note: `-D clippy::redundant-closure` implied by `-D warnings`
error: redundant closure found
- --> $DIR/eta.rs:14:10
+ --> $DIR/eta.rs:16:10
|
LL | meta(|a| foo(a));
| ^^^^^^^^^^ help: remove closure as shown: `foo`
error: redundant closure found
- --> $DIR/eta.rs:15:27
+ --> $DIR/eta.rs:17:27
|
LL | let c = Some(1u8).map(|a| {1+2; foo}(a));
| ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `{1+2; foo}`
error: this expression borrows a reference that is immediately dereferenced by the compiler
- --> $DIR/eta.rs:17:21
+ --> $DIR/eta.rs:19:21
|
LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted
| ^^^ help: change this to: `&2`
= note: `-D clippy::needless-borrow` implied by `-D warnings`
error: redundant closure found
- --> $DIR/eta.rs:24:27
+ --> $DIR/eta.rs:26:27
|
LL | let e = Some(1u8).map(|a| generic(a));
| ^^^^^^^^^^^^^^ help: remove closure as shown: `generic`
-error: aborting due to 5 previous errors
+error: redundant closure found
+ --> $DIR/eta.rs:60:48
+ |
+LL | let e = Some(TestStruct{some_ref: &i}).map(|a| a.foo());
+ | ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo`
+
+error: redundant closure found
+ --> $DIR/eta.rs:62:48
+ |
+LL | let e = Some(TestStruct{some_ref: &i}).map(|a| a.trait_foo());
+ | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo`
+
+error: redundant closure found
+ --> $DIR/eta.rs:65:40
+ |
+LL | let e = Some(&mut vec!(1,2,3)).map(|v| v.clear());
+ | ^^^^^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::clear`
+
+error: redundant closure found
+ --> $DIR/eta.rs:70:29
+ |
+LL | let e = Some("str").map(|s| s.to_string());
+ | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string`
+
+error: redundant closure found
+ --> $DIR/eta.rs:72:27
+ |
+LL | let e = Some('a').map(|s| s.to_uppercase());
+ | ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase`
+
+error: redundant closure found
+ --> $DIR/eta.rs:75:63
+ |
+LL | let e: std::vec::Vec<char> = vec!('a','b','c').iter().map(|c| c.to_ascii_uppercase()).collect();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase`
+
+error: aborting due to 11 previous errors
projects / rust.git / commitdiff