Auto merge of #13421 - lukaslueg:issue11212, r=Manishearth

Initial impl of `unnecessary_first_then_check`

Fixes #11212

Checks for `{slice/vec/Box<[]>}.first().is_some()` and suggests replacing the unnecessary `Option`-construct with a direct `{slice/...}.is_empty()`. Other lints guide constructs like `if let Some(_) = v.get(0)` into this, which end up as `!v.is_empty()`.

changelog: [`unnecessary_first_then_check`]: Initial implementation

3 files changed, 90 insertions, 0 deletions

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 16c64830e70..824f57cece9 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -467,6 +467,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::UNNECESSARY_FALLIBLE_CONVERSIONS_INFO,
     crate::methods::UNNECESSARY_FILTER_MAP_INFO,
     crate::methods::UNNECESSARY_FIND_MAP_INFO,
+    crate::methods::UNNECESSARY_FIRST_THEN_CHECK_INFO,
     crate::methods::UNNECESSARY_FOLD_INFO,
     crate::methods::UNNECESSARY_GET_THEN_CHECK_INFO,
     crate::methods::UNNECESSARY_JOIN_INFO,
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 9b7cba9dafb..d8812dfd90c 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -111,6 +111,7 @@ mod uninit_assumed_init;
 mod unit_hash;
 mod unnecessary_fallible_conversions;
 mod unnecessary_filter_map;
+mod unnecessary_first_then_check;
 mod unnecessary_fold;
 mod unnecessary_get_then_check;
 mod unnecessary_iter_cloned;
@@ -4137,6 +4138,34 @@ declare_clippy_lint! {
     "use of `map` returning the original item"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks the usage of `.first().is_some()` or `.first().is_none()` to check if a slice is
+    /// empty.
+    ///
+    /// ### Why is this bad?
+    /// Using `.is_empty()` is shorter and better communicates the intention.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let v = vec![1, 2, 3];
+    /// if v.first().is_none() {
+    ///     // The vector is empty...
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let v = vec![1, 2, 3];
+    /// if v.is_empty() {
+    ///     // The vector is empty...
+    /// }
+    /// ```
+    #[clippy::version = "1.83.0"]
+    pub UNNECESSARY_FIRST_THEN_CHECK,
+    complexity,
+    "calling `.first().is_some()` or `.first().is_none()` instead of `.is_empty()`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4294,6 +4323,7 @@ impl_lint_pass!(Methods => [
     UNNECESSARY_RESULT_MAP_OR_ELSE,
     MANUAL_C_STR_LITERALS,
     UNNECESSARY_GET_THEN_CHECK,
+    UNNECESSARY_FIRST_THEN_CHECK,
     NEEDLESS_CHARACTER_ITERATION,
     MANUAL_INSPECT,
     UNNECESSARY_MIN_OR_MAX,
@@ -5066,6 +5096,9 @@ fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>,
         Some(("get", f_recv, [arg], _, _)) => {
             unnecessary_get_then_check::check(cx, call_span, recv, f_recv, arg, is_some);
         },
+        Some(("first", f_recv, [], _, _)) => {
+            unnecessary_first_then_check::check(cx, call_span, recv, f_recv, is_some);
+        },
         _ => {},
     }
 }
diff --git a/clippy_lints/src/methods/unnecessary_first_then_check.rs b/clippy_lints/src/methods/unnecessary_first_then_check.rs
new file mode 100644
index 00000000000..7ae1bb54e60
--- /dev/null
+++ b/clippy_lints/src/methods/unnecessary_first_then_check.rs
@@ -0,0 +1,56 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::SpanRangeExt;
+
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::Span;
+
+use super::UNNECESSARY_FIRST_THEN_CHECK;
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    call_span: Span,
+    first_call: &Expr<'_>,
+    first_caller: &Expr<'_>,
+    is_some: bool,
+) {
+    if !cx
+        .typeck_results()
+        .expr_ty_adjusted(first_caller)
+        .peel_refs()
+        .is_slice()
+    {
+        return;
+    }
+
+    let ExprKind::MethodCall(_, _, _, first_call_span) = first_call.kind else {
+        return;
+    };
+
+    let both_calls_span = first_call_span.with_hi(call_span.hi());
+    if let Some(both_calls_snippet) = both_calls_span.get_source_text(cx)
+        && let Some(first_caller_snippet) = first_caller.span.get_source_text(cx)
+    {
+        let (sugg_span, suggestion) = if is_some {
+            (
+                first_caller.span.with_hi(call_span.hi()),
+                format!("!{first_caller_snippet}.is_empty()"),
+            )
+        } else {
+            (both_calls_span, "is_empty()".to_owned())
+        };
+        span_lint_and_sugg(
+            cx,
+            UNNECESSARY_FIRST_THEN_CHECK,
+            sugg_span,
+            format!(
+                "unnecessary use of `{both_calls_snippet}` to check if slice {}",
+                if is_some { "is not empty" } else { "is empty" }
+            ),
+            "replace this with",
+            suggestion,
+            Applicability::MaybeIncorrect,
+        );
+    }
+}