Auto merge of rust-lang#8882 - kyoto7250:get_first, r=llogiq

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

close rust-lang#8851

This PR adds new lint about considering replacing .get(0) with .first().

Thank you in advance.

changelog: adds new lint [`get_first`] to consider replacing .get(0) with .first()

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,69 @@
+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(_, [struct_calling_on, method_arg], _) = &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);
+
+            if let Some(_) = is_slice_of_primitives(cx, struct_calling_on);
+            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_lints/src/methods/iter_next_slice.rs

@@ -34,13 +34,18 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, cal
             if let ast::LitKind::Int(start_idx, _) = start_lit.node;
             then {
                 let mut applicability = Applicability::MachineApplicable;
+                let suggest = if start_idx == 0 {
+                    format!("{}.first()", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability))
+                } else {
+                    format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx)
+                };
                 span_lint_and_sugg(
                     cx,
                     ITER_NEXT_SLICE,
                     expr.span,
                     "using `.iter().next()` on a Slice without end index",
                     "try calling",
-                    format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx),
+                    suggest,
                     applicability,
                 );
             }
@@ -55,7 +60,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, cal
             "using `.iter().next()` on an array",
             "try calling",
             format!(
-                "{}.get(0)",
+                "{}.first()",
                 snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability)
             ),
             applicability,

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-toml/unwrap_used/unwrap_used.rs

@@ -1,6 +1,6 @@
 // compile-flags: --test
 
-#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
+#![allow(unused_mut, clippy::get_first, clippy::from_iter_instead_of_collect)]
 #![warn(clippy::unwrap_used)]
 #![deny(clippy::get_unwrap)]
 

tests/ui/debug_assert_with_mut_call.rs

@@ -1,7 +1,7 @@
 #![feature(custom_inner_attributes)]
 #![rustfmt::skip]
 #![warn(clippy::debug_assert_with_mut_call)]
-#![allow(clippy::redundant_closure_call)]
+#![allow(clippy::redundant_closure_call, clippy::get_first)]
 
 
 struct S;

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
+

tests/ui/get_unwrap.fixed

@@ -1,6 +1,6 @@
 // run-rustfix
 
-#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
+#![allow(unused_mut, clippy::from_iter_instead_of_collect, clippy::get_first)]
 #![warn(clippy::unwrap_used)]
 #![deny(clippy::get_unwrap)]
 

tests/ui/get_unwrap.rs

@@ -1,6 +1,6 @@
 // run-rustfix
 
-#![allow(unused_mut, clippy::from_iter_instead_of_collect)]
+#![allow(unused_mut, clippy::from_iter_instead_of_collect, clippy::get_first)]
 #![warn(clippy::unwrap_used)]
 #![deny(clippy::get_unwrap)]
 

tests/ui/iter_next_slice.fixed

@@ -6,17 +6,17 @@ fn main() {
     let s = [1, 2, 3];
     let v = vec![1, 2, 3];
 
-    let _ = s.get(0);
-    // Should be replaced by s.get(0)
+    let _ = s.first();
+    // Should be replaced by s.first()
 
     let _ = s.get(2);
     // Should be replaced by s.get(2)
 
     let _ = v.get(5);
     // Should be replaced by v.get(5)
 
-    let _ = v.get(0);
-    // Should be replaced by v.get(0)
+    let _ = v.first();
+    // Should be replaced by v.first()
 
     let o = Some(5);
     o.iter().next();

tests/ui/iter_next_slice.rs

@@ -7,7 +7,7 @@ fn main() {
     let v = vec![1, 2, 3];
 
     let _ = s.iter().next();
-    // Should be replaced by s.get(0)
+    // Should be replaced by s.first()
 
     let _ = s[2..].iter().next();
     // Should be replaced by s.get(2)
@@ -16,7 +16,7 @@ fn main() {
     // Should be replaced by v.get(5)
 
     let _ = v.iter().next();
-    // Should be replaced by v.get(0)
+    // Should be replaced by v.first()
 
     let o = Some(5);
     o.iter().next();

tests/ui/iter_next_slice.stderr

@@ -2,7 +2,7 @@ error: using `.iter().next()` on an array
   --> $DIR/iter_next_slice.rs:9:13
    |
 LL |     let _ = s.iter().next();
-   |             ^^^^^^^^^^^^^^^ help: try calling: `s.get(0)`
+   |             ^^^^^^^^^^^^^^^ help: try calling: `s.first()`
    |
    = note: `-D clippy::iter-next-slice` implied by `-D warnings`
 
@@ -22,7 +22,7 @@ error: using `.iter().next()` on an array
   --> $DIR/iter_next_slice.rs:18:13
    |
 LL |     let _ = v.iter().next();
-   |             ^^^^^^^^^^^^^^^ help: try calling: `v.get(0)`
+   |             ^^^^^^^^^^^^^^^ help: try calling: `v.first()`
 
 error: aborting due to 4 previous errors
 

tests/ui/needless_lifetimes.rs

@@ -4,7 +4,8 @@
     clippy::boxed_local,
     clippy::needless_pass_by_value,
     clippy::unnecessary_wraps,
-    dyn_drop
+    dyn_drop,
+    clippy::get_first
 )]
 
 fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {}