From 057243f16b4f42337b6178a627dcf3ae4c09f056 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 2 Oct 2018 12:51:38 +0200 Subject: [PATCH] relicensing: Remove map_clone This removes the code added in https://github.com/rust-lang-nursery/rust-clippy/pull/427 --- clippy_lints/src/lib.rs | 4 - clippy_lints/src/map_clone.rs | 140 ---------------------------------- tests/ui/map_clone.rs | 105 ------------------------- tests/ui/map_clone.stderr | 102 ------------------------- 4 files changed, 351 deletions(-) delete mode 100644 clippy_lints/src/map_clone.rs delete mode 100644 tests/ui/map_clone.rs delete mode 100644 tests/ui/map_clone.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 19564dbf9be..689dbfa7da9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -129,7 +129,6 @@ pub mod let_if_seq; pub mod lifetimes; pub mod literal_representation; pub mod loops; -pub mod map_clone; pub mod map_unit_fn; pub mod matches; pub mod mem_forget; @@ -346,7 +345,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box needless_borrow::NeedlessBorrow); reg.register_late_lint_pass(box needless_borrowed_ref::NeedlessBorrowedRef); reg.register_late_lint_pass(box no_effect::Pass); - reg.register_late_lint_pass(box map_clone::Pass); reg.register_late_lint_pass(box temporary_assignment::Pass); reg.register_late_lint_pass(box transmute::Transmute); reg.register_late_lint_pass( @@ -585,7 +583,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::WHILE_IMMUTABLE_CONDITION, loops::WHILE_LET_LOOP, loops::WHILE_LET_ON_ITERATOR, - map_clone::MAP_CLONE, map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, matches::MATCH_AS_REF, @@ -745,7 +742,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::FOR_KV_MAP, loops::NEEDLESS_RANGE_LOOP, loops::WHILE_LET_ON_ITERATOR, - map_clone::MAP_CLONE, matches::MATCH_BOOL, matches::MATCH_OVERLAPPING_ARM, matches::MATCH_REF_PATS, diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs deleted file mode 100644 index 239602c6db5..00000000000 --- a/clippy_lints/src/map_clone.rs +++ /dev/null @@ -1,140 +0,0 @@ -use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use crate::rustc::{declare_tool_lint, lint_array}; -use if_chain::if_chain; -use crate::rustc::hir::*; -use crate::rustc::ty; -use crate::syntax::ast; -use crate::utils::{get_arg_ident, is_adjusted, iter_input_pats, match_qpath, match_trait_method, match_type, - paths, remove_blocks, snippet, span_help_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq}; - -/// **What it does:** Checks for mapping `clone()` over an iterator. -/// -/// **Why is this bad?** It makes the code less readable than using the -/// `.cloned()` adapter. -/// -/// **Known problems:** Sometimes `.cloned()` requires stricter trait -/// bound than `.map(|e| e.clone())` (which works because of the coercion). -/// See [#498](https://github.com/rust-lang-nursery/rust-clippy/issues/498). -/// -/// **Example:** -/// ```rust -/// x.map(|e| e.clone()); -/// ``` -declare_clippy_lint! { - pub MAP_CLONE, - style, - "using `.map(|x| x.clone())` to clone an iterator or option's contents" -} - -#[derive(Copy, Clone)] -pub struct Pass; - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - // call to .map() - if let ExprKind::MethodCall(ref method, _, ref args) = expr.node { - if method.ident.name == "map" && args.len() == 2 { - match args[1].node { - ExprKind::Closure(_, ref decl, closure_eid, _, _) => { - let body = cx.tcx.hir.body(closure_eid); - let closure_expr = remove_blocks(&body.value); - if_chain! { - // nothing special in the argument, besides reference bindings - // (e.g. .map(|&x| x) ) - if let Some(first_arg) = iter_input_pats(decl, body).next(); - if let Some(arg_ident) = get_arg_ident(&first_arg.pat); - // the method is being called on a known type (option or iterator) - if let Some(type_name) = get_type_name(cx, expr, &args[0]); - then { - // We know that body.arguments is not empty at this point - let ty = cx.tables.pat_ty(&body.arguments[0].pat); - // look for derefs, for .map(|x| *x) - if only_derefs(cx, &*closure_expr, arg_ident) && - // .cloned() only removes one level of indirection, don't lint on more - walk_ptrs_ty_depth(cx.tables.pat_ty(&first_arg.pat)).1 == 1 - { - // the argument is not an &mut T - if let ty::Ref(_, _, mutbl) = ty.sty { - if mutbl == MutImmutable { - span_help_and_lint(cx, MAP_CLONE, expr.span, &format!( - "you seem to be using .map() to clone the contents of an {}, consider \ - using `.cloned()`", type_name), - &format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); - } - } - } - // explicit clone() calls ( .map(|x| x.clone()) ) - else if let ExprKind::MethodCall(ref clone_call, _, ref clone_args) = closure_expr.node { - if clone_call.ident.name == "clone" && - clone_args.len() == 1 && - match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) && - expr_eq_name(cx, &clone_args[0], arg_ident) - { - span_help_and_lint(cx, MAP_CLONE, expr.span, &format!( - "you seem to be using .map() to clone the contents of an {}, consider \ - using `.cloned()`", type_name), - &format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); - } - } - } - } - }, - ExprKind::Path(ref path) => if match_qpath(path, &paths::CLONE) { - let type_name = get_type_name(cx, expr, &args[0]).unwrap_or("_"); - span_help_and_lint( - cx, - MAP_CLONE, - expr.span, - &format!( - "you seem to be using .map() to clone the contents of an \ - {}, consider using `.cloned()`", - type_name - ), - &format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")), - ); - }, - _ => (), - } - } - } - } -} - -fn expr_eq_name(cx: &LateContext<'_, '_>, expr: &Expr, id: ast::Ident) -> bool { - match expr.node { - ExprKind::Path(QPath::Resolved(None, ref path)) => { - let arg_segment = [ - PathSegment { - ident: id, - args: None, - infer_types: true, - }, - ]; - !path.is_global() && SpanlessEq::new(cx).eq_path_segments(&path.segments[..], &arg_segment) - }, - _ => false, - } -} - -fn get_type_name(cx: &LateContext<'_, '_>, expr: &Expr, arg: &Expr) -> Option<&'static str> { - if match_trait_method(cx, expr, &paths::ITERATOR) { - Some("iterator") - } else if match_type(cx, walk_ptrs_ty(cx.tables.expr_ty(arg)), &paths::OPTION) { - Some("Option") - } else { - None - } -} - -fn only_derefs(cx: &LateContext<'_, '_>, expr: &Expr, id: ast::Ident) -> bool { - match expr.node { - ExprKind::Unary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => only_derefs(cx, subexpr, id), - _ => expr_eq_name(cx, expr, id), - } -} - -impl LintPass for Pass { - fn get_lints(&self) -> LintArray { - lint_array!(MAP_CLONE) - } -} diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs deleted file mode 100644 index 90c95be2c1c..00000000000 --- a/tests/ui/map_clone.rs +++ /dev/null @@ -1,105 +0,0 @@ -#![feature(tool_lints)] - - -#![warn(clippy::map_clone)] - -#![allow(clippy::clone_on_copy, unused)] - -use std::ops::Deref; - -fn map_clone_iter() { - let x = [1,2,3]; - x.iter().map(|y| y.clone()); - - x.iter().map(|&y| y); - - x.iter().map(|y| *y); - - x.iter().map(|y| { y.clone() }); - - x.iter().map(|&y| { y }); - - x.iter().map(|y| { *y }); - - x.iter().map(Clone::clone); - -} - -fn map_clone_option() { - let x = Some(4); - x.as_ref().map(|y| y.clone()); - - x.as_ref().map(|&y| y); - - x.as_ref().map(|y| *y); - -} - -fn not_linted_option() { - let x = Some(5); - - // Not linted: other statements - x.as_ref().map(|y| { - println!("y: {}", y); - y.clone() - }); - - // Not linted: argument bindings - let x = Some((6, 7)); - x.map(|(y, _)| y.clone()); - - // Not linted: cloning something else - x.map(|y| y.0.clone()); - - // Not linted: no dereferences - x.map(|y| y); - - // Not linted: multiple dereferences - let _: Option<(i32, i32)> = x.as_ref().as_ref().map(|&&x| x); -} - -#[derive(Copy, Clone)] -struct Wrapper(T); -impl Wrapper { - fn map U>(self, f: F) -> Wrapper { - Wrapper(f(self.0)) - } -} - -fn map_clone_other() { - let eight = 8; - let x = Wrapper(&eight); - - // Not linted: not a linted type - x.map(|y| y.clone()); - x.map(|&y| y); - x.map(|y| *y); -} - -#[derive(Copy, Clone)] -struct UnusualDeref; -static NINE: i32 = 9; - -impl Deref for UnusualDeref { - type Target = i32; - fn deref(&self) -> &i32 { &NINE } -} - -fn map_clone_deref() { - let x = Some(UnusualDeref); - let _: Option = x.as_ref().map(|y| *y); - - - // Not linted: using deref conversion - let _: Option = x.map(|y| *y); - - // Not linted: using regular deref but also deref conversion - let _: Option = x.as_ref().map(|y| **y); -} - -// stuff that used to be a false positive -fn former_false_positive() { - vec![1].iter_mut().map(|x| *x); // #443 -} - -fn main() { } diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr deleted file mode 100644 index afad65b0071..00000000000 --- a/tests/ui/map_clone.stderr +++ /dev/null @@ -1,102 +0,0 @@ -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:12:5 - | -12 | x.iter().map(|y| y.clone()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::map-clone` implied by `-D warnings` - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:14:5 - | -14 | x.iter().map(|&y| y); - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:16:5 - | -16 | x.iter().map(|y| *y); - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:18:5 - | -18 | x.iter().map(|y| { y.clone() }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:20:5 - | -20 | x.iter().map(|&y| { y }); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:22:5 - | -22 | x.iter().map(|y| { *y }); - | ^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()` - --> $DIR/map_clone.rs:24:5 - | -24 | x.iter().map(Clone::clone); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.iter().cloned() - -error: you seem to be using .map() to clone the contents of an Option, consider using `.cloned()` - --> $DIR/map_clone.rs:30:5 - | -30 | x.as_ref().map(|y| y.clone()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.as_ref().cloned() - -error: you seem to be using .map() to clone the contents of an Option, consider using `.cloned()` - --> $DIR/map_clone.rs:32:5 - | -32 | x.as_ref().map(|&y| y); - | ^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.as_ref().cloned() - -error: you seem to be using .map() to clone the contents of an Option, consider using `.cloned()` - --> $DIR/map_clone.rs:34:5 - | -34 | x.as_ref().map(|y| *y); - | ^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.as_ref().cloned() - -error: you seem to be using .map() to clone the contents of an Option, consider using `.cloned()` - --> $DIR/map_clone.rs:90:35 - | -90 | let _: Option = x.as_ref().map(|y| *y); - | ^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try - x.as_ref().cloned() - -error: aborting due to 11 previous errors -