about summary refs log tree commit diff
path: root/clippy_lints/src
diff options
context:
space:
mode:
authorLukas Lueg <lukas.lueg@gmail.com>2022-08-03 22:13:06 +0200
committerLukas Lueg <lukas.lueg@gmail.com>2022-08-08 20:17:13 +0200
commit657b0da912f0281aa11fe61df7fa273bf9be1d5b (patch)
tree3ef3632f1ffdc71b0a494b1c5292ab39f1613ba0 /clippy_lints/src
parent71777465cc71b365ca08db8afd5c25eb99bd3f08 (diff)
Add partialeq_to_none lint
Fixes #9275
Diffstat (limited to 'clippy_lints/src')
-rw-r--r--clippy_lints/src/lib.register_all.rs1
-rw-r--r--clippy_lints/src/lib.register_lints.rs1
-rw-r--r--clippy_lints/src/lib.register_style.rs1
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/partialeq_to_none.rs104
5 files changed, 109 insertions, 0 deletions
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index b9cc9fc1e85..5b10061052d 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -267,6 +267,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
     LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
+    LintId::of(partialeq_to_none::PARTIALEQ_TO_NONE),
     LintId::of(precedence::PRECEDENCE),
     LintId::of(ptr::CMP_NULL),
     LintId::of(ptr::INVALID_NULL_PTR_USAGE),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index c2bc4badc43..7468fc8b47b 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -454,6 +454,7 @@ store.register_lints(&[
     panic_unimplemented::UNIMPLEMENTED,
     panic_unimplemented::UNREACHABLE,
     partialeq_ne_impl::PARTIALEQ_NE_IMPL,
+    partialeq_to_none::PARTIALEQ_TO_NONE,
     pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE,
     pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF,
     path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,
diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
index ebc0933e31d..bfa654238f1 100644
--- a/clippy_lints/src/lib.register_style.rs
+++ b/clippy_lints/src/lib.register_style.rs
@@ -100,6 +100,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(operators::ASSIGN_OP_PATTERN),
     LintId::of(operators::OP_REF),
     LintId::of(operators::PTR_EQ),
+    LintId::of(partialeq_to_none::PARTIALEQ_TO_NONE),
     LintId::of(ptr::CMP_NULL),
     LintId::of(ptr::PTR_ARG),
     LintId::of(question_mark::QUESTION_MARK),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 88c1a727f8d..e6a405f8170 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -332,6 +332,7 @@ mod overflow_check_conditional;
 mod panic_in_result_fn;
 mod panic_unimplemented;
 mod partialeq_ne_impl;
+mod partialeq_to_none;
 mod pass_by_ref_or_value;
 mod path_buf_push_overwrite;
 mod pattern_type_mismatch;
@@ -931,6 +932,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
     store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
+    store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/partialeq_to_none.rs b/clippy_lints/src/partialeq_to_none.rs
new file mode 100644
index 00000000000..eee7642068d
--- /dev/null
+++ b/clippy_lints/src/partialeq_to_none.rs
@@ -0,0 +1,104 @@
+use clippy_utils::{
+    diagnostics::span_lint_and_sugg, is_lang_ctor, peel_hir_expr_refs, peel_ref_operators, sugg,
+    ty::is_type_diagnostic_item,
+};
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind, LangItem};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for binary comparisons to a literal `Option::None`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// A programmer checking if some `foo` is `None` via a comparison `foo == None`
+    /// is usually inspired from other programming languages (e.g. `foo is None`
+    /// in Python).
+    /// Checking if a value of type `Option<T>` is (not) equal to `None` in that
+    /// way relies on `T: PartialEq` to do the comparison, which is unneeded.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn foo(f: Option<u32>) -> &'static str {
+    ///     if f != None { "yay" } else { "nay" }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn foo(f: Option<u32>) -> &'static str {
+    ///     if f.is_some() { "yay" } else { "nay" }
+    /// }
+    /// ```
+    #[clippy::version = "1.64.0"]
+    pub PARTIALEQ_TO_NONE,
+    style,
+    "Binary comparison to `Option<T>::None` relies on `T: PartialEq`, which is unneeded"
+}
+declare_lint_pass!(PartialeqToNone => [PARTIALEQ_TO_NONE]);
+
+impl<'tcx> LateLintPass<'tcx> for PartialeqToNone {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
+        // Skip expanded code, as we have no control over it anyway...
+        if e.span.from_expansion() {
+            return;
+        }
+
+        // If the expression is of type `Option`
+        let is_ty_option =
+            |expr: &Expr<'_>| is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr).peel_refs(), sym::Option);
+
+        // If the expression is a literal `Option::None`
+        let is_none_ctor = |expr: &Expr<'_>| {
+            matches!(&peel_hir_expr_refs(expr).0.kind,
+            ExprKind::Path(p) if is_lang_ctor(cx, p, LangItem::OptionNone))
+        };
+
+        let mut applicability = Applicability::MachineApplicable;
+
+        if let ExprKind::Binary(op, left_side, right_side) = e.kind {
+            // All other comparisons (e.g. `>= None`) have special meaning wrt T
+            let is_eq = match op.node {
+                BinOpKind::Eq => true,
+                BinOpKind::Ne => false,
+                _ => return,
+            };
+
+            // We are only interested in comparisons between `Option` and a literal `Option::None`
+            let scrutinee = match (
+                is_none_ctor(left_side) && is_ty_option(right_side),
+                is_none_ctor(right_side) && is_ty_option(left_side),
+            ) {
+                (true, false) => right_side,
+                (false, true) => left_side,
+                _ => return,
+            };
+
+            // Peel away refs/derefs (as long as we don't cross manual deref impls), as
+            // autoref/autoderef will take care of those
+            let sugg = format!(
+                "{}.{}",
+                sugg::Sugg::hir_with_applicability(cx, peel_ref_operators(cx, scrutinee), "..", &mut applicability)
+                    .maybe_par(),
+                if is_eq { "is_none()" } else { "is_some()" }
+            );
+
+            span_lint_and_sugg(
+                cx,
+                PARTIALEQ_TO_NONE,
+                e.span,
+                "binary comparison to literal `Option::None`",
+                if is_eq {
+                    "use `Option::is_none()` instead"
+                } else {
+                    "use `Option::is_some()` instead"
+                },
+                sugg,
+                applicability,
+            );
+        }
+    }
+}