feat(lint): impl lint about use first() instead of get(0)

Author: kyoto7250

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

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,
+                );
+            }
+        }
+    }
+}

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),

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,

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),

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`
 }
 

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", "<impl [T]>", "get"];
 pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "into_vec"];
 pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"];
 pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"];

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<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]);
+    let btreemap: BTreeMap<u8, char> = 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.
+}

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<u8, char> = HashMap::from_iter(vec![(0, 'a'), (1, 'b')]);
+    let btreemap: BTreeMap<u8, char> = 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.
+}

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
+