diff --git a/CHANGELOG.md b/CHANGELOG.md index 13f0fc8f609d..52d0e8c7cdeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2295,6 +2295,7 @@ Released 2018-09-13 [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped +[`append_instead_of_extend`]: https://rust-lang.github.io/rust-clippy/master/index.html#append_instead_of_extend [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 591bf68f927a..5ab333f8aa10 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -730,6 +730,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: mem_replace::MEM_REPLACE_OPTION_WITH_NONE, mem_replace::MEM_REPLACE_WITH_DEFAULT, mem_replace::MEM_REPLACE_WITH_UNINIT, + methods::APPEND_INSTEAD_OF_EXTEND, methods::BIND_INSTEAD_OF_MAP, methods::BYTES_NTH, methods::CHARS_LAST_CMP, @@ -1276,6 +1277,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE), LintId::of(mem_replace::MEM_REPLACE_WITH_DEFAULT), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), + LintId::of(methods::APPEND_INSTEAD_OF_EXTEND), LintId::of(methods::BIND_INSTEAD_OF_MAP), LintId::of(methods::BYTES_NTH), LintId::of(methods::CHARS_LAST_CMP), @@ -1736,6 +1738,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(loops::MANUAL_MEMCPY), LintId::of(loops::NEEDLESS_COLLECT), + LintId::of(methods::APPEND_INSTEAD_OF_EXTEND), LintId::of(methods::EXPECT_FUN_CALL), LintId::of(methods::ITER_NTH), LintId::of(methods::MANUAL_STR_REPEAT), diff --git a/clippy_lints/src/methods/append_instead_of_extend.rs b/clippy_lints/src/methods/append_instead_of_extend.rs new file mode 100644 index 000000000000..e39a5a1efd1e --- /dev/null +++ b/clippy_lints/src/methods/append_instead_of_extend.rs @@ -0,0 +1,41 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::APPEND_INSTEAD_OF_EXTEND; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + if_chain! { + if is_type_diagnostic_item(cx, ty, sym::vec_type); + //check source object + if let ExprKind::MethodCall(src_method, _, [drain_vec, drain_arg], _) = &arg.kind; + if src_method.ident.as_str() == "drain"; + if let src_ty = cx.typeck_results().expr_ty(drain_vec).peel_refs(); + if is_type_diagnostic_item(cx, src_ty, sym::vec_type); + //check drain range + if let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs(); + if is_type_lang_item(cx, src_ty_range, LangItem::RangeFull); + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + APPEND_INSTEAD_OF_EXTEND, + expr.span, + "use of `extend` instead of `append` for adding the full range of a second vector", + "try this", + format!( + "{}.append(&mut {})", + snippet_with_applicability(cx, recv.span, "..", &mut applicability), + snippet_with_applicability(cx, drain_vec.span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d4f8cef4f375..b556dcb4dfe8 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,3 +1,4 @@ +mod append_instead_of_extend; mod bind_instead_of_map; mod bytes_nth; mod chars_cmp; @@ -1032,6 +1033,30 @@ declare_clippy_lint! { "using `.get().unwrap()` or `.get_mut().unwrap()` when using `[]` would work instead" } +declare_clippy_lint! { + /// **What it does:** Checks for occurrences where one vector gets extended instead of append + /// + /// **Why is this bad?** Using `append` instead of `extend` is more concise and faster + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let mut a = vec![1, 2, 3]; + /// let mut b = vec![4, 5, 6]; + /// + /// // Bad + /// a.extend(b.drain(..)); + /// + /// // Good + /// a.append(&mut b); + /// ``` + pub APPEND_INSTEAD_OF_EXTEND, + perf, + "using vec.append(&mut vec) to move the full range of a vecor to another" +} + declare_clippy_lint! { /// **What it does:** Checks for the use of `.extend(s.chars())` where s is a /// `&str` or `String`. @@ -1785,7 +1810,8 @@ impl_lint_pass!(Methods => [ INSPECT_FOR_EACH, IMPLICIT_CLONE, SUSPICIOUS_SPLITN, - MANUAL_STR_REPEAT + MANUAL_STR_REPEAT, + APPEND_INSTEAD_OF_EXTEND ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2047,7 +2073,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv), _ => expect_used::check(cx, expr, recv), }, - ("extend", [arg]) => string_extend_chars::check(cx, expr, recv, arg), + ("extend", [arg]) => { + string_extend_chars::check(cx, expr, recv, arg); + append_instead_of_extend::check(cx, expr, recv, arg); + }, ("filter_map", [arg]) => { unnecessary_filter_map::check(cx, expr, arg); filter_map_identity::check(cx, expr, arg, span); diff --git a/tests/ui/append_instead_of_extend.fixed b/tests/ui/append_instead_of_extend.fixed new file mode 100644 index 000000000000..283358333cdf --- /dev/null +++ b/tests/ui/append_instead_of_extend.fixed @@ -0,0 +1,55 @@ +// run-rustfix +#![warn(clippy::append_instead_of_extend)] +use std::collections::BinaryHeap; +fn main() { + //gets linted + let mut vec1 = vec![0u8; 1024]; + let mut vec2: std::vec::Vec = Vec::new(); + + vec2.append(&mut vec1); + + let mut vec3 = vec![0u8; 1024]; + let mut vec4: std::vec::Vec = Vec::new(); + + vec4.append(&mut vec3); + + let mut vec11: std::vec::Vec = Vec::new(); + + vec11.append(&mut return_vector()); + + //won't get linted it dosen't move the entire content of a vec into another + let mut test1 = vec![0u8, 10]; + let mut test2: std::vec::Vec = Vec::new(); + + test2.extend(test1.drain(4..10)); + + let mut vec3 = vec![0u8; 104]; + let mut vec7: std::vec::Vec = Vec::new(); + + vec3.append(&mut vec7); + + let mut vec5 = vec![0u8; 1024]; + let mut vec6: std::vec::Vec = Vec::new(); + + vec5.extend(vec6.drain(..4)); + + let mut vec9: std::vec::Vec = Vec::new(); + + return_vector().append(&mut vec9); + + //won't get linted because it is not a vec + + let mut heap = BinaryHeap::from(vec![1, 3]); + let mut heap2 = BinaryHeap::from(vec![]); + heap2.extend(heap.drain()) +} + +fn return_vector() -> Vec { + let mut new_vector = vec![]; + + for i in 1..10 { + new_vector.push(i) + } + + new_vector +} diff --git a/tests/ui/append_instead_of_extend.rs b/tests/ui/append_instead_of_extend.rs new file mode 100644 index 000000000000..abde5cdac5cf --- /dev/null +++ b/tests/ui/append_instead_of_extend.rs @@ -0,0 +1,55 @@ +// run-rustfix +#![warn(clippy::append_instead_of_extend)] +use std::collections::BinaryHeap; +fn main() { + //gets linted + let mut vec1 = vec![0u8; 1024]; + let mut vec2: std::vec::Vec = Vec::new(); + + vec2.extend(vec1.drain(..)); + + let mut vec3 = vec![0u8; 1024]; + let mut vec4: std::vec::Vec = Vec::new(); + + vec4.extend(vec3.drain(..)); + + let mut vec11: std::vec::Vec = Vec::new(); + + vec11.extend(return_vector().drain(..)); + + //won't get linted it dosen't move the entire content of a vec into another + let mut test1 = vec![0u8, 10]; + let mut test2: std::vec::Vec = Vec::new(); + + test2.extend(test1.drain(4..10)); + + let mut vec3 = vec![0u8; 104]; + let mut vec7: std::vec::Vec = Vec::new(); + + vec3.append(&mut vec7); + + let mut vec5 = vec![0u8; 1024]; + let mut vec6: std::vec::Vec = Vec::new(); + + vec5.extend(vec6.drain(..4)); + + let mut vec9: std::vec::Vec = Vec::new(); + + return_vector().append(&mut vec9); + + //won't get linted because it is not a vec + + let mut heap = BinaryHeap::from(vec![1, 3]); + let mut heap2 = BinaryHeap::from(vec![]); + heap2.extend(heap.drain()) +} + +fn return_vector() -> Vec { + let mut new_vector = vec![]; + + for i in 1..10 { + new_vector.push(i) + } + + new_vector +} diff --git a/tests/ui/append_instead_of_extend.stderr b/tests/ui/append_instead_of_extend.stderr new file mode 100644 index 000000000000..9d309d981def --- /dev/null +++ b/tests/ui/append_instead_of_extend.stderr @@ -0,0 +1,22 @@ +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:9:5 + | +LL | vec2.extend(vec1.drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec2.append(&mut vec1)` + | + = note: `-D clippy::append-instead-of-extend` implied by `-D warnings` + +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:14:5 + | +LL | vec4.extend(vec3.drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec4.append(&mut vec3)` + +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:18:5 + | +LL | vec11.extend(return_vector().drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec11.append(&mut return_vector())` + +error: aborting due to 3 previous errors +