Skip to content

Commit

Permalink
Initial impl of unnecessary_first_then_check
Browse files Browse the repository at this point in the history
  • Loading branch information
lukaslueg committed Sep 19, 2024
1 parent 903293b commit 290a01e
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6021,6 +6021,7 @@ Released 2018-09-13
[`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map
[`unnecessary_first_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_first_then_check
[`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold
[`unnecessary_get_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check
[`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 33 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
},
_ => {},
}
}
Expand Down
56 changes: 56 additions & 0 deletions clippy_lints/src/methods/unnecessary_first_then_check.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
22 changes: 22 additions & 0 deletions tests/ui/unnecessary_first_then_check.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![warn(clippy::unnecessary_first_then_check)]
#![allow(clippy::useless_vec, clippy::const_is_empty)]

fn main() {
let s = [1, 2, 3];
let _: bool = !s.is_empty();
let _: bool = s.is_empty();

let v = vec![1, 2, 3];
let _: bool = !v.is_empty();

let n = [[1, 2, 3], [4, 5, 6]];
let _: bool = !n[0].is_empty();
let _: bool = n[0].is_empty();

struct Foo {
bar: &'static [i32],
}
let f = [Foo { bar: &[] }];
let _: bool = !f[0].bar.is_empty();
let _: bool = f[0].bar.is_empty();
}
22 changes: 22 additions & 0 deletions tests/ui/unnecessary_first_then_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![warn(clippy::unnecessary_first_then_check)]
#![allow(clippy::useless_vec, clippy::const_is_empty)]

fn main() {
let s = [1, 2, 3];
let _: bool = s.first().is_some();
let _: bool = s.first().is_none();

let v = vec![1, 2, 3];
let _: bool = v.first().is_some();

let n = [[1, 2, 3], [4, 5, 6]];
let _: bool = n[0].first().is_some();
let _: bool = n[0].first().is_none();

struct Foo {
bar: &'static [i32],
}
let f = [Foo { bar: &[] }];
let _: bool = f[0].bar.first().is_some();
let _: bool = f[0].bar.first().is_none();
}
47 changes: 47 additions & 0 deletions tests/ui/unnecessary_first_then_check.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: unnecessary use of `first().is_some()` to check if slice is not empty
--> tests/ui/unnecessary_first_then_check.rs:6:19
|
LL | let _: bool = s.first().is_some();
| ^^^^^^^^^^^^^^^^^^^ help: replace this with: `!s.is_empty()`
|
= note: `-D clippy::unnecessary-first-then-check` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_first_then_check)]`

error: unnecessary use of `first().is_none()` to check if slice is empty
--> tests/ui/unnecessary_first_then_check.rs:7:21
|
LL | let _: bool = s.first().is_none();
| ^^^^^^^^^^^^^^^^^ help: replace this with: `is_empty()`

error: unnecessary use of `first().is_some()` to check if slice is not empty
--> tests/ui/unnecessary_first_then_check.rs:10:19
|
LL | let _: bool = v.first().is_some();
| ^^^^^^^^^^^^^^^^^^^ help: replace this with: `!v.is_empty()`

error: unnecessary use of `first().is_some()` to check if slice is not empty
--> tests/ui/unnecessary_first_then_check.rs:13:19
|
LL | let _: bool = n[0].first().is_some();
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `!n[0].is_empty()`

error: unnecessary use of `first().is_none()` to check if slice is empty
--> tests/ui/unnecessary_first_then_check.rs:14:24
|
LL | let _: bool = n[0].first().is_none();
| ^^^^^^^^^^^^^^^^^ help: replace this with: `is_empty()`

error: unnecessary use of `first().is_some()` to check if slice is not empty
--> tests/ui/unnecessary_first_then_check.rs:20:19
|
LL | let _: bool = f[0].bar.first().is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace this with: `!f[0].bar.is_empty()`

error: unnecessary use of `first().is_none()` to check if slice is empty
--> tests/ui/unnecessary_first_then_check.rs:21:28
|
LL | let _: bool = f[0].bar.first().is_none();
| ^^^^^^^^^^^^^^^^^ help: replace this with: `is_empty()`

error: aborting due to 7 previous errors

0 comments on commit 290a01e

Please sign in to comment.