Skip to content

Commit

Permalink
added lint to check for full range of vector and suggest append
Browse files Browse the repository at this point in the history
  • Loading branch information
Valentine-Mario committed Jun 9, 2021
1 parent e4a1e85 commit be79701
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2295,6 +2295,7 @@ Released 2018-09-13
<!-- begin autogenerated links to lint list -->
[`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
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1735,6 +1737,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),
Expand Down Expand Up @@ -2066,7 +2069,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv));
store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison);
store.register_late_pass(|| box unused_async::UnusedAsync);

}

#[rustfmt::skip]
Expand Down
41 changes: 41 additions & 0 deletions clippy_lints/src/methods/append_instead_of_extend.rs
Original file line number Diff line number Diff line change
@@ -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<'_>) {
if_chain! {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
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";
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
let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs();
if is_type_lang_item(cx, src_ty_range, LangItem::RangeFull);
let mut applicability = Applicability::MachineApplicable;
then {
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,
);
}
}
}
33 changes: 31 additions & 2 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod append_instead_of_extend;
mod bind_instead_of_map;
mod bytes_nth;
mod chars_cmp;
Expand Down Expand Up @@ -1031,6 +1032,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`.
Expand Down Expand Up @@ -1760,7 +1785,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.
Expand Down Expand Up @@ -2022,7 +2048,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);
Expand Down
55 changes: 55 additions & 0 deletions tests/ui/append_instead_of_extend.fixed
Original file line number Diff line number Diff line change
@@ -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<u8> = Vec::new();

vec2.append(&mut vec1);

let mut vec3 = vec![0u8; 1024];
let mut vec4: std::vec::Vec<u8> = Vec::new();

vec4.append(&mut vec3);

let mut vec11: std::vec::Vec<u8> = 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<u8> = Vec::new();

test2.extend(test1.drain(4..10));

let mut vec3 = vec![0u8; 104];
let mut vec7: std::vec::Vec<u8> = Vec::new();

vec3.append(&mut vec7);

let mut vec5 = vec![0u8; 1024];
let mut vec6: std::vec::Vec<u8> = Vec::new();

vec5.extend(vec6.drain(..4));

let mut vec9: std::vec::Vec<u8> = 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<u8> {
let mut new_vector = vec![];

for i in 1..10 {
new_vector.push(i)
}

new_vector
}
55 changes: 55 additions & 0 deletions tests/ui/append_instead_of_extend.rs
Original file line number Diff line number Diff line change
@@ -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<u8> = Vec::new();

vec2.extend(vec1.drain(..));

let mut vec3 = vec![0u8; 1024];
let mut vec4: std::vec::Vec<u8> = Vec::new();

vec4.extend(vec3.drain(..));

let mut vec11: std::vec::Vec<u8> = 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<u8> = Vec::new();

test2.extend(test1.drain(4..10));

let mut vec3 = vec![0u8; 104];
let mut vec7: std::vec::Vec<u8> = Vec::new();

vec3.append(&mut vec7);

let mut vec5 = vec![0u8; 1024];
let mut vec6: std::vec::Vec<u8> = Vec::new();

vec5.extend(vec6.drain(..4));

let mut vec9: std::vec::Vec<u8> = 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<u8> {
let mut new_vector = vec![];

for i in 1..10 {
new_vector.push(i)
}

new_vector
}
22 changes: 22 additions & 0 deletions tests/ui/append_instead_of_extend.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit be79701

Please sign in to comment.