From 1dd026698d4f6c5848fb88d48b2d4248846bb41c Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Tue, 24 May 2022 18:54:49 +0900 Subject: [PATCH] feat(lint): impl lint about use first() instead of get(0) --- CHANGELOG.md | 1 + clippy_lints/src/get_first.rs | 72 ++++++++++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_utils/src/paths.rs | 1 + tests/ui/get_first.fixed | 42 +++++++++++++++ tests/ui/get_first.rs | 42 +++++++++++++++ tests/ui/get_first.stderr | 22 ++++++++ 10 files changed, 185 insertions(+) create mode 100644 clippy_lints/src/get_first.rs create mode 100644 tests/ui/get_first.fixed create mode 100644 tests/ui/get_first.rs create mode 100644 tests/ui/get_first.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index f4a362e6c19..0f6295a45f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3435,6 +3435,7 @@ Released 2018-09-13 [`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into [`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10 [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send +[`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap [`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion diff --git a/clippy_lints/src/get_first.rs b/clippy_lints/src/get_first.rs new file mode 100644 index 00000000000..0f26c19c49e --- /dev/null +++ b/clippy_lints/src/get_first.rs @@ -0,0 +1,72 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::{is_slice_of_primitives, match_def_path, paths}; +use if_chain::if_chain; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Spanned; + +declare_clippy_lint! { + /// ### What it does + /// Checks for using `x.get(0)` instead of + /// `x.first()`. + /// + /// ### Why is this bad? + /// Using `x.first()` is easier to read and has the same + /// result. + /// + /// ### Example + /// ```rust + /// // Bad + /// let x = vec![2, 3, 5]; + /// let first_element = x.get(0); + /// ``` + /// Use instead: + /// ```rust + /// // Good + /// let x = vec![2, 3, 5]; + /// let first_element = x.first(); + /// ``` + #[clippy::version = "1.63.0"] + pub GET_FIRST, + style, + "Using `x.get(0)` when `x.first()` is simpler" +} +declare_lint_pass!(GetFirst => [GET_FIRST]); + +impl<'tcx> LateLintPass<'tcx> for GetFirst { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if_chain! { + if let hir::ExprKind::MethodCall(_, expr_args, _) = &expr.kind; + if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + if match_def_path(cx, expr_def_id, &paths::SLICE_GET) && expr_args.len() == 2; + + if let Some(struct_calling_on) = expr_args.get(0); + if let Some(_) = is_slice_of_primitives(cx, struct_calling_on); + + if let Some(method_arg) = expr_args.get(1); + if let hir::ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = method_arg.kind; + + then { + let mut applicability = Applicability::MachineApplicable; + let slice_name = snippet_with_applicability( + cx, + struct_calling_on.span, "..", + &mut applicability, + ); + span_lint_and_sugg( + cx, + GET_FIRST, + expr.span, + &format!("accessing first element with `{0}.get(0)`", slice_name), + "try", + format!("{}.first()", slice_name), + applicability, + ); + } + } + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 5aba6040b8c..a028b41db77 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -91,6 +91,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF), LintId::of(functions::RESULT_UNIT_ERR), LintId::of(functions::TOO_MANY_ARGUMENTS), + LintId::of(get_first::GET_FIRST), LintId::of(identity_op::IDENTITY_OP), LintId::of(if_let_mutex::IF_LET_MUTEX), LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index fea34fe66f4..570d736518b 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -183,6 +183,7 @@ store.register_lints(&[ functions::TOO_MANY_ARGUMENTS, functions::TOO_MANY_LINES, future_not_send::FUTURE_NOT_SEND, + get_first::GET_FIRST, identity_op::IDENTITY_OP, if_let_mutex::IF_LET_MUTEX, if_not_else::IF_NOT_ELSE, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 62f26d821a0..ea2e1082458 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -31,6 +31,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(functions::DOUBLE_MUST_USE), LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), + LintId::of(get_first::GET_FIRST), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS), LintId::of(len_zero::COMPARISON_TO_EMPTY), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2f8533723ef..5f636e5114b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -242,6 +242,7 @@ mod from_over_into; mod from_str_radix_10; mod functions; mod future_not_send; +mod get_first; mod identity_op; mod if_let_mutex; mod if_not_else; @@ -904,6 +905,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace)); store.register_late_pass(|| Box::new(rc_clone_in_vec_init::RcCloneInVecInit)); store.register_early_pass(|| Box::new(duplicate_mod::DuplicateMod::default())); + store.register_late_pass(|| Box::new(get_first::GetFirst)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 9b9cbff2d14..0064694ff92 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -141,6 +141,7 @@ pub const SERDE_DESERIALIZE: [&str; 3] = ["serde", "de", "Deserialize"]; pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"]; pub const SLICE_FROM_RAW_PARTS: [&str; 4] = ["core", "slice", "raw", "from_raw_parts"]; pub const SLICE_FROM_RAW_PARTS_MUT: [&str; 4] = ["core", "slice", "raw", "from_raw_parts_mut"]; +pub const SLICE_GET: [&str; 4] = ["core", "slice", "", "get"]; pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec"]; pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"]; pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; diff --git a/tests/ui/get_first.fixed b/tests/ui/get_first.fixed new file mode 100644 index 00000000000..def58afa4fb --- /dev/null +++ b/tests/ui/get_first.fixed @@ -0,0 +1,42 @@ +// run-rustfix +#![warn(clippy::get_first)] +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::VecDeque; + +struct Bar { + arr: [u32; 3], +} + +impl Bar { + fn get(&self, pos: usize) -> Option<&u32> { + self.arr.get(pos) + } +} + +fn main() { + let x = vec![2, 3, 5]; + let _ = x.first(); // Use x.first() + let _ = x.get(1); + let _ = x[0]; + + let y = [2, 3, 5]; + let _ = y.first(); // Use y.first() + let _ = y.get(1); + let _ = y[0]; + + let z = &[2, 3, 5]; + let _ = z.first(); // Use z.first() + let _ = z.get(1); + let _ = z[0]; + + let vecdeque: VecDeque<_> = x.iter().cloned().collect(); + let hashmap: HashMap = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]); + let btreemap: BTreeMap = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]); + let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice. + let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice. + let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice. + + let bar = Bar { arr: [0, 1, 2] }; + let _ = bar.get(0); // Do not lint, because Bar is struct. +} diff --git a/tests/ui/get_first.rs b/tests/ui/get_first.rs new file mode 100644 index 00000000000..85a381854cd --- /dev/null +++ b/tests/ui/get_first.rs @@ -0,0 +1,42 @@ +// run-rustfix +#![warn(clippy::get_first)] +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::VecDeque; + +struct Bar { + arr: [u32; 3], +} + +impl Bar { + fn get(&self, pos: usize) -> Option<&u32> { + self.arr.get(pos) + } +} + +fn main() { + let x = vec![2, 3, 5]; + let _ = x.get(0); // Use x.first() + let _ = x.get(1); + let _ = x[0]; + + let y = [2, 3, 5]; + let _ = y.get(0); // Use y.first() + let _ = y.get(1); + let _ = y[0]; + + let z = &[2, 3, 5]; + let _ = z.get(0); // Use z.first() + let _ = z.get(1); + let _ = z[0]; + + let vecdeque: VecDeque<_> = x.iter().cloned().collect(); + let hashmap: HashMap = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]); + let btreemap: BTreeMap = BTreeMap::from_iter(vec![(0, 'a'), (1, 'b')]); + let _ = vecdeque.get(0); // Do not lint, because VecDeque is not slice. + let _ = hashmap.get(&0); // Do not lint, because HashMap is not slice. + let _ = btreemap.get(&0); // Do not lint, because BTreeMap is not slice. + + let bar = Bar { arr: [0, 1, 2] }; + let _ = bar.get(0); // Do not lint, because Bar is struct. +} diff --git a/tests/ui/get_first.stderr b/tests/ui/get_first.stderr new file mode 100644 index 00000000000..466beff9c92 --- /dev/null +++ b/tests/ui/get_first.stderr @@ -0,0 +1,22 @@ +error: accessing first element with `x.get(0)` + --> $DIR/get_first.rs:19:13 + | +LL | let _ = x.get(0); // Use x.first() + | ^^^^^^^^ help: try: `x.first()` + | + = note: `-D clippy::get-first` implied by `-D warnings` + +error: accessing first element with `y.get(0)` + --> $DIR/get_first.rs:24:13 + | +LL | let _ = y.get(0); // Use y.first() + | ^^^^^^^^ help: try: `y.first()` + +error: accessing first element with `z.get(0)` + --> $DIR/get_first.rs:29:13 + | +LL | let _ = z.get(0); // Use z.first() + | ^^^^^^^^ help: try: `z.first()` + +error: aborting due to 3 previous errors +