diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9af4850b15c..3779b09ecf3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -328,6 +328,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box strings::StringAdd); reg.register_early_lint_pass(box returns::ReturnPass); reg.register_late_lint_pass(box methods::Pass); + reg.register_late_lint_pass(box map_clone::Pass); reg.register_late_lint_pass(box shadow::Pass); reg.register_late_lint_pass(box types::LetPass); reg.register_late_lint_pass(box types::UnitCmp); @@ -346,7 +347,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( @@ -792,6 +792,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { returns::NEEDLESS_RETURN, strings::STRING_LIT_AS_BYTES, types::FN_TO_NUMERIC_CAST, + types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, @@ -921,7 +922,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { transmute::WRONG_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::CAST_PTR_ALIGNMENT, - types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, unused_io_amount::UNUSED_IO_AMOUNT, diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 239602c6db5..e16a8af7641 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -1,136 +1,45 @@ +use crate::rustc::hir; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; +use crate::syntax::source_map::Span; +use crate::utils::paths; +use crate::utils::{ + in_macro, match_trait_method, match_type, + remove_blocks, snippet, + span_lint_and_sugg, +}; 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}; +use crate::syntax::ast::Ident; -/// **What it does:** Checks for mapping `clone()` over an iterator. +#[derive(Clone)] +pub struct Pass; + +/// **What it does:** Checks for usage of `iterator.map(|x| x.clone())` and suggests +/// `iterator.cloned()` instead /// -/// **Why is this bad?** It makes the code less readable than using the -/// `.cloned()` adapter. +/// **Why is this bad?** Readability, this can be written more concisely /// -/// **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). +/// **Known problems:** None. /// /// **Example:** +/// /// ```rust -/// x.map(|e| e.clone()); +/// let x = vec![42, 43]; +/// let y = x.iter(); +/// let z = y.map(|i| *i); +/// ``` +/// +/// The correct use would be: +/// +/// ```rust +/// let x = vec![42, 43]; +/// let y = x.iter(); +/// let z = y.cloned(); /// ``` 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), - } + "using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types" } impl LintPass for Pass { @@ -138,3 +47,52 @@ fn get_lints(&self) -> LintArray { lint_array!(MAP_CLONE) } } + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr) { + if in_macro(e.span) { + return; + } + + if_chain! { + if let hir::ExprKind::MethodCall(ref method, _, ref args) = e.node; + if args.len() == 2; + if method.ident.as_str() == "map"; + let ty = cx.tables.expr_ty(&args[0]); + if match_type(cx, ty, &paths::OPTION) || match_trait_method(cx, e, &paths::ITERATOR); + if let hir::ExprKind::Closure(_, _, body_id, _, _) = args[1].node; + let closure_body = cx.tcx.hir.body(body_id); + let closure_expr = remove_blocks(&closure_body.value); + then { + match closure_body.arguments[0].pat.node { + hir::PatKind::Ref(ref inner, _) => if let hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, name, None) = inner.node { + lint(cx, e.span, args[0].span, name, closure_expr); + }, + hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, name, None) => match closure_expr.node { + hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner) => lint(cx, e.span, args[0].span, name, inner), + hir::ExprKind::MethodCall(ref method, _, ref obj) => if method.ident.as_str() == "clone" && match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT) { + lint(cx, e.span, args[0].span, name, &obj[0]); + } + _ => {}, + }, + _ => {}, + } + } + } + } +} + +fn lint(cx: &LateContext<'_, '_>, replace: Span, root: Span, name: Ident, path: &hir::Expr) { + if let hir::ExprKind::Path(hir::QPath::Resolved(None, ref path)) = path.node { + if path.segments.len() == 1 && path.segments[0].ident == name { + span_lint_and_sugg( + cx, + MAP_CLONE, + replace, + "You are using an explicit closure for cloning elements", + "Consider calling the dedicated `cloned` method", + format!("{}.cloned()", snippet(cx, root, "..")), + ) + } + } +} diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 6f024e59610..2e660088380 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -701,40 +701,6 @@ fn is_unit_literal(expr: &Expr) -> bool { "cast to the same type, e.g. `x as i32` where `x: i32`" } -/// **What it does:** Checks for casts of a function pointer to a numeric type not enough to store address. -/// -/// **Why is this bad?** Casting a function pointer to not eligible type could truncate the address value. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust -/// fn test_fn() -> i16; -/// let _ = test_fn as i32 -/// ``` -declare_clippy_lint! { - pub FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - correctness, - "cast function pointer to the numeric type with value truncation" -} - -/// **What it does:** Checks for casts of a function pointer to a numeric type except `usize`. -/// -/// **Why is this bad?** Casting a function pointer to something other than `usize` is not a good style. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust -/// fn test_fn() -> i16; -/// let _ = test_fn as i128 -/// ``` -declare_clippy_lint! { - pub FN_TO_NUMERIC_CAST, - style, - "cast function pointer to the numeric type" -} - /// **What it does:** Checks for casts from a less-strictly-aligned pointer to a /// more-strictly-aligned pointer /// @@ -754,6 +720,59 @@ fn is_unit_literal(expr: &Expr) -> bool { "cast from a pointer to a more-strictly-aligned pointer" } +/// **What it does:** Checks for casts of function pointers to something other than usize +/// +/// **Why is this bad?** +/// Casting a function pointer to anything other than usize/isize is not portable across +/// architectures, because you end up losing bits if the target type is too small or end up with a +/// bunch of extra bits that waste space and add more instructions to the final binary than +/// strictly necessary for the problem +/// +/// Casting to isize also doesn't make sense since there are no signed addresses. +/// +/// **Example** +/// +/// ```rust +/// // Bad +/// fn fun() -> i32 {} +/// let a = fun as i64; +/// +/// // Good +/// fn fun2() -> i32 {} +/// let a = fun2 as usize; +/// ``` +declare_clippy_lint! { + pub FN_TO_NUMERIC_CAST, + style, + "casting a function pointer to a numeric type other than usize" +} + +/// **What it does:** Checks for casts of a function pointer to a numeric type not wide enough to +/// store address. +/// +/// **Why is this bad?** +/// Such a cast discards some bits of the function's address. If this is intended, it would be more +/// clearly expressed by casting to usize first, then casting the usize to the intended type (with +/// a comment) to perform the truncation. +/// +/// **Example** +/// +/// ```rust +/// // Bad +/// fn fn1() -> i16 { 1 }; +/// let _ = fn1 as i32; +/// +/// // Better: Cast to usize first, then comment with the reason for the truncation +/// fn fn2() -> i16 { 1 }; +/// let fn_ptr = fn2 as usize; +/// let fn_ptr_truncated = fn_ptr as i32; +/// ``` +declare_clippy_lint! { + pub FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + style, + "casting a function pointer to a numeric type not wide enough to store the address" +} + /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_, '_, '_>) -> u64 { @@ -948,8 +967,7 @@ fn get_lints(&self) -> LintArray { CAST_LOSSLESS, UNNECESSARY_CAST, CAST_PTR_ALIGNMENT, - FN_TO_NUMERIC_CAST, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + FN_TO_NUMERIC_CAST ) } } @@ -958,6 +976,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprKind::Cast(ref ex, _) = expr.node { let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr)); + lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); if let ExprKind::Lit(ref lit) = ex.node { use crate::syntax::ast::{LitIntType, LitKind}; match lit.node { @@ -1034,37 +1053,6 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } - match &cast_from.sty { - ty::FnDef(..) | - ty::FnPtr(..) => { - if cast_to.is_numeric() && cast_to.sty != ty::Uint(UintTy::Usize){ - let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); - let pointer_nbits = cx.tcx.data_layout.pointer_size.bits(); - if to_nbits < pointer_nbits || (to_nbits == pointer_nbits && cast_to.is_signed()) { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST_WITH_TRUNCATION, - expr.span, - &format!("casting a `{}` to `{}` may truncate the function address value.", cast_from, cast_to), - "if you need the address of the function, consider", - format!("{} as usize", &snippet(cx, ex.span, "x")) - ); - } else { - span_lint_and_sugg( - cx, - FN_TO_NUMERIC_CAST, - expr.span, - &format!("casting a `{}` to `{}` is bad style.", cast_from, cast_to), - "if you need the address of the function, consider", - format!("{} as usize", &snippet(cx, ex.span, "x")) - ); - - }; - } - } - _ => () - } - if_chain!{ if let ty::RawPtr(from_ptr_ty) = &cast_from.sty; if let ty::RawPtr(to_ptr_ty) = &cast_to.sty; @@ -1089,6 +1077,37 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } +fn lint_fn_to_numeric_cast(cx: &LateContext<'_, '_>, expr: &Expr, cast_expr: &Expr, cast_from: Ty<'_>, cast_to: Ty<'_>) { + match cast_from.sty { + ty::FnDef(..) | ty::FnPtr(_) => { + let from_snippet = snippet(cx, cast_expr.span, "x"); + + let to_nbits = int_ty_to_nbits(cast_to, cx.tcx); + if to_nbits < cx.tcx.data_layout.pointer_size.bits() { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST_WITH_TRUNCATION, + expr.span, + &format!("casting function pointer `{}` to `{}`, which truncates the value", from_snippet, cast_to), + "try", + format!("{} as usize", from_snippet) + ); + + } else if cast_to.sty != ty::Uint(UintTy::Usize) { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST, + expr.span, + &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), + "try", + format!("{} as usize", from_snippet) + ); + } + }, + _ => {} + } +} + /// **What it does:** Checks for types used in structs, parameters and `let` /// declarations above a certain complexity threshold. /// diff --git a/tests/ui/fn_to_numeric_cast.rs b/tests/ui/fn_to_numeric_cast.rs new file mode 100644 index 00000000000..fc8aa19dcf0 --- /dev/null +++ b/tests/ui/fn_to_numeric_cast.rs @@ -0,0 +1,50 @@ +// only-64bit +#![feature(tool_lints)] + +#![warn(clippy::fn_to_numeric_cast, clippy::fn_to_numeric_cast_with_truncation)] + +fn foo() -> String { String::new() } + +fn test_function_to_numeric_cast() { + let _ = foo as i8; + let _ = foo as i16; + let _ = foo as i32; + let _ = foo as i64; + let _ = foo as i128; + let _ = foo as isize; + + let _ = foo as u8; + let _ = foo as u16; + let _ = foo as u32; + let _ = foo as u64; + let _ = foo as u128; + + // Casting to usize is OK and should not warn + let _ = foo as usize; +} + +fn test_function_var_to_numeric_cast() { + let abc: fn() -> String = foo; + + let _ = abc as i8; + let _ = abc as i16; + let _ = abc as i32; + let _ = abc as i64; + let _ = abc as i128; + let _ = abc as isize; + + let _ = abc as u8; + let _ = abc as u16; + let _ = abc as u32; + let _ = abc as u64; + let _ = abc as u128; + + // Casting to usize is OK and should not warn + let _ = abc as usize; +} + +fn fn_with_fn_args(f: fn(i32) -> i32) -> i32 { + f as i32 +} + +fn main() {} diff --git a/tests/ui/fn_to_numeric_cast.stderr b/tests/ui/fn_to_numeric_cast.stderr new file mode 100644 index 00000000000..29320f0d8ed --- /dev/null +++ b/tests/ui/fn_to_numeric_cast.stderr @@ -0,0 +1,144 @@ +error: casting function pointer `foo` to `i8`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:9:13 + | +9 | let _ = foo as i8; + | ^^^^^^^^^ help: try: `foo as usize` + | + = note: `-D clippy::fn-to-numeric-cast-with-truncation` implied by `-D warnings` + +error: casting function pointer `foo` to `i16`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:10:13 + | +10 | let _ = foo as i16; + | ^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `i32`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:11:13 + | +11 | let _ = foo as i32; + | ^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `i64` + --> $DIR/fn_to_numeric_cast.rs:12:13 + | +12 | let _ = foo as i64; + | ^^^^^^^^^^ help: try: `foo as usize` + | + = note: `-D clippy::fn-to-numeric-cast` implied by `-D warnings` + +error: casting function pointer `foo` to `i128` + --> $DIR/fn_to_numeric_cast.rs:13:13 + | +13 | let _ = foo as i128; + | ^^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `isize` + --> $DIR/fn_to_numeric_cast.rs:14:13 + | +14 | let _ = foo as isize; + | ^^^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `u8`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:16:13 + | +16 | let _ = foo as u8; + | ^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `u16`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:17:13 + | +17 | let _ = foo as u16; + | ^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `u32`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:18:13 + | +18 | let _ = foo as u32; + | ^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `u64` + --> $DIR/fn_to_numeric_cast.rs:19:13 + | +19 | let _ = foo as u64; + | ^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `foo` to `u128` + --> $DIR/fn_to_numeric_cast.rs:20:13 + | +20 | let _ = foo as u128; + | ^^^^^^^^^^^ help: try: `foo as usize` + +error: casting function pointer `abc` to `i8`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:29:13 + | +29 | let _ = abc as i8; + | ^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `i16`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:30:13 + | +30 | let _ = abc as i16; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `i32`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:31:13 + | +31 | let _ = abc as i32; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `i64` + --> $DIR/fn_to_numeric_cast.rs:32:13 + | +32 | let _ = abc as i64; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `i128` + --> $DIR/fn_to_numeric_cast.rs:33:13 + | +33 | let _ = abc as i128; + | ^^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `isize` + --> $DIR/fn_to_numeric_cast.rs:34:13 + | +34 | let _ = abc as isize; + | ^^^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `u8`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:36:13 + | +36 | let _ = abc as u8; + | ^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `u16`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:37:13 + | +37 | let _ = abc as u16; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `u32`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:38:13 + | +38 | let _ = abc as u32; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `u64` + --> $DIR/fn_to_numeric_cast.rs:39:13 + | +39 | let _ = abc as u64; + | ^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `abc` to `u128` + --> $DIR/fn_to_numeric_cast.rs:40:13 + | +40 | let _ = abc as u128; + | ^^^^^^^^^^^ help: try: `abc as usize` + +error: casting function pointer `f` to `i32`, which truncates the value + --> $DIR/fn_to_numeric_cast.rs:47:5 + | +47 | f as i32 + | ^^^^^^^^ help: try: `f as usize` + +error: aborting due to 23 previous errors + diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs index 90c95be2c1c..11a5316a367 100644 --- a/tests/ui/map_clone.rs +++ b/tests/ui/map_clone.rs @@ -1,105 +1,9 @@ #![feature(tool_lints)] +#![warn(clippy::all, clippy::pedantic)] +#![allow(clippy::missing_docs_in_private_items)] - -#![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 main() { + let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); + let _: Vec = vec![String::new()].iter().map(|x| x.clone()).collect(); + let _: Vec = vec![42, 43].iter().map(|&x| x).collect(); } - -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 index afad65b0071..e80983cdbf7 100644 --- a/tests/ui/map_clone.stderr +++ b/tests/ui/map_clone.stderr @@ -1,102 +1,22 @@ -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 are using an explicit closure for cloning elements + --> $DIR/map_clone.rs:6:22 + | +6 | let _: Vec = vec![5_i8; 6].iter().map(|x| *x).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![5_i8; 6].iter().cloned()` + | + = note: `-D clippy::map-clone` implied by `-D warnings` -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 are using an explicit closure for cloning elements + --> $DIR/map_clone.rs:7:26 + | +7 | let _: Vec = vec![String::new()].iter().map(|x| x.clone()).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![String::new()].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 are using an explicit closure for cloning elements + --> $DIR/map_clone.rs:8:23 + | +8 | let _: Vec = vec![42, 43].iter().map(|&x| x).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `cloned` method: `vec![42, 43].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 +error: aborting due to 3 previous errors diff --git a/tests/ui/types_fn_to_int.rs b/tests/ui/types_fn_to_int.rs deleted file mode 100644 index 8387586c3e9..00000000000 --- a/tests/ui/types_fn_to_int.rs +++ /dev/null @@ -1,22 +0,0 @@ -enum Foo { - A(usize), - B -} - -fn bar() -> i32 { - 0i32 -} - -fn main() { - let x = Foo::A; - let _y = x as i32; - let _y1 = Foo::A as i32; - let _y = x as u32; - let _z = bar as u32; - let _y = bar as i64; - let _y = bar as u64; - let _z = Foo::A as i128; - let _z = Foo::A as u128; - let _z = bar as i128; - let _z = bar as u128; -} diff --git a/tests/ui/types_fn_to_int.stderr b/tests/ui/types_fn_to_int.stderr deleted file mode 100644 index a06809b9bfd..00000000000 --- a/tests/ui/types_fn_to_int.stderr +++ /dev/null @@ -1,66 +0,0 @@ -error: casting a `fn(usize) -> Foo {Foo::A}` to `i32` may truncate the function address value. - --> $DIR/types_fn_to_int.rs:12:14 - | -12 | let _y = x as i32; - | ^^^^^^^^ help: if you need the address of the function, consider: `x as usize` - | - = note: #[deny(clippy::fn_to_numeric_cast_with_truncation)] on by default - -error: casting a `fn(usize) -> Foo {Foo::A}` to `i32` may truncate the function address value. - --> $DIR/types_fn_to_int.rs:13:15 - | -13 | let _y1 = Foo::A as i32; - | ^^^^^^^^^^^^^ help: if you need the address of the function, consider: `Foo::A as usize` - -error: casting a `fn(usize) -> Foo {Foo::A}` to `u32` may truncate the function address value. - --> $DIR/types_fn_to_int.rs:14:14 - | -14 | let _y = x as u32; - | ^^^^^^^^ help: if you need the address of the function, consider: `x as usize` - -error: casting a `fn() -> i32 {bar}` to `u32` may truncate the function address value. - --> $DIR/types_fn_to_int.rs:15:14 - | -15 | let _z = bar as u32; - | ^^^^^^^^^^ help: if you need the address of the function, consider: `bar as usize` - -error: casting a `fn() -> i32 {bar}` to `i64` may truncate the function address value. - --> $DIR/types_fn_to_int.rs:16:14 - | -16 | let _y = bar as i64; - | ^^^^^^^^^^ help: if you need the address of the function, consider: `bar as usize` - -error: casting a `fn() -> i32 {bar}` to `u64` is bad style. - --> $DIR/types_fn_to_int.rs:17:14 - | -17 | let _y = bar as u64; - | ^^^^^^^^^^ help: if you need the address of the function, consider: `bar as usize` - | - = note: `-D clippy::fn-to-numeric-cast` implied by `-D warnings` - -error: casting a `fn(usize) -> Foo {Foo::A}` to `i128` is bad style. - --> $DIR/types_fn_to_int.rs:18:14 - | -18 | let _z = Foo::A as i128; - | ^^^^^^^^^^^^^^ help: if you need the address of the function, consider: `Foo::A as usize` - -error: casting a `fn(usize) -> Foo {Foo::A}` to `u128` is bad style. - --> $DIR/types_fn_to_int.rs:19:14 - | -19 | let _z = Foo::A as u128; - | ^^^^^^^^^^^^^^ help: if you need the address of the function, consider: `Foo::A as usize` - -error: casting a `fn() -> i32 {bar}` to `i128` is bad style. - --> $DIR/types_fn_to_int.rs:20:14 - | -20 | let _z = bar as i128; - | ^^^^^^^^^^^ help: if you need the address of the function, consider: `bar as usize` - -error: casting a `fn() -> i32 {bar}` to `u128` is bad style. - --> $DIR/types_fn_to_int.rs:21:14 - | -21 | let _z = bar as u128; - | ^^^^^^^^^^^ help: if you need the address of the function, consider: `bar as usize` - -error: aborting due to 10 previous errors -