diff options
| author | Lukas Lueg <lukas.lueg@gmail.com> | 2022-08-03 22:13:06 +0200 |
|---|---|---|
| committer | Lukas Lueg <lukas.lueg@gmail.com> | 2022-08-08 20:17:13 +0200 |
| commit | 657b0da912f0281aa11fe61df7fa273bf9be1d5b (patch) | |
| tree | 3ef3632f1ffdc71b0a494b1c5292ab39f1613ba0 /clippy_lints/src | |
| parent | 71777465cc71b365ca08db8afd5c25eb99bd3f08 (diff) | |
Add partialeq_to_none lint
Fixes #9275
Diffstat (limited to 'clippy_lints/src')
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_style.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/partialeq_to_none.rs | 104 |
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, + ); + } + } +} |
