Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 0eeca6f

Browse files
lukaslueglukaslueg
committedAug 3, 2022
Add partialeq_to_none lint
Fixes #9275
1 parent 7177746 commit 0eeca6f

File tree

8 files changed

+271
-0
lines changed

8 files changed

+271
-0
lines changed
 

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3823,6 +3823,7 @@ Released 2018-09-13
38233823
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
38243824
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
38253825
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
3826+
[`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none
38263827
[`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
38273828
[`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
38283829
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma

‎clippy_lints/src/lib.register_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ store.register_lints(&[
454454
panic_unimplemented::UNIMPLEMENTED,
455455
panic_unimplemented::UNREACHABLE,
456456
partialeq_ne_impl::PARTIALEQ_NE_IMPL,
457+
partialeq_to_none::PARTIALEQ_TO_NONE,
457458
pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE,
458459
pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF,
459460
path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,

‎clippy_lints/src/lib.register_nursery.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
2222
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
2323
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
2424
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
25+
LintId::of(partialeq_to_none::PARTIALEQ_TO_NONE),
2526
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
2627
LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),
2728
LintId::of(regex::TRIVIAL_REGEX),

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ mod overflow_check_conditional;
332332
mod panic_in_result_fn;
333333
mod panic_unimplemented;
334334
mod partialeq_ne_impl;
335+
mod partialeq_to_none;
335336
mod pass_by_ref_or_value;
336337
mod path_buf_push_overwrite;
337338
mod pattern_type_mismatch;
@@ -931,6 +932,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
931932
store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
932933
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
933934
store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
935+
store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
934936
// add lints here, do not remove this comment, it's used in `new_lint`
935937
}
936938

‎clippy_lints/src/partialeq_to_none.rs‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
use clippy_utils::{
2+
diagnostics::span_lint_and_sugg, is_lang_ctor, source::snippet_with_applicability, ty::is_type_diagnostic_item,
3+
};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{BinOpKind, Expr, ExprKind, LangItem};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::sym;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
///
13+
/// Checks for binary comparisons to a literal `Option::None`.
14+
///
15+
/// ### Why is this bad?
16+
///
17+
/// A programmer checking if some `foo` is `None` via a comparison `foo == None`
18+
/// is usually inspired from other programming languages (e.g. `foo is None`
19+
/// in Python).
20+
/// Checking if a value of type `Option<T>` is (not) equal to `None` in that
21+
/// way relies on `T: PartialEq` to do the comparison, which is unneeded.
22+
///
23+
/// ### Example
24+
/// ```rust
25+
/// fn foo(f: Option<u32>) -> {
26+
/// if f != None {
27+
/// "yay"
28+
/// } else {
29+
/// "nay"
30+
/// }
31+
/// }
32+
/// ```
33+
/// Use instead:
34+
/// ```rust
35+
/// fn foo(f: Option<u32>) -> {
36+
/// if f.is_some() {
37+
/// "yay"
38+
/// } else {
39+
/// "nay"
40+
/// }
41+
/// }
42+
/// ```
43+
#[clippy::version = "1.64.0"]
44+
pub PARTIALEQ_TO_NONE,
45+
nursery,
46+
"default lint description"
47+
}
48+
declare_lint_pass!(PartialeqToNone => [PARTIALEQ_TO_NONE]);
49+
50+
impl<'tcx> LateLintPass<'tcx> for PartialeqToNone {
51+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
52+
// Skip expanded code, as we have no control over it anyway...
53+
if e.span.from_expansion() {
54+
return;
55+
}
56+
57+
// If the expression is of type `Option`
58+
let is_ty_option =
59+
|expr: &Expr<'_>| is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option);
60+
61+
// If the expression is a literal `Option::None`
62+
let is_none_ctor = |expr: &Expr<'_>| {
63+
matches!(&expr.kind,
64+
ExprKind::Path(p) if is_lang_ctor(cx, p, LangItem::OptionNone))
65+
};
66+
67+
let mut applicability = Applicability::MachineApplicable;
68+
69+
if let ExprKind::Binary(op, left_side, right_side) = e.kind {
70+
// All other comparisons (e.g. `>= None`) have special meaning wrt T
71+
let is_eq = match op.node {
72+
BinOpKind::Eq => true,
73+
BinOpKind::Ne => false,
74+
_ => return,
75+
};
76+
77+
// We are only interested in comparisons between `Option` and a literal `Option::None`
78+
let scrutinee = match (
79+
is_none_ctor(left_side) && is_ty_option(right_side),
80+
is_none_ctor(right_side) && is_ty_option(right_side),
81+
) {
82+
(true, false) => right_side,
83+
(false, true) => left_side,
84+
_ => return,
85+
};
86+
87+
span_lint_and_sugg(
88+
cx,
89+
PARTIALEQ_TO_NONE,
90+
e.span,
91+
"binary comparison to literal `Option::None`",
92+
if is_eq {
93+
"use `Option::is_none()` instead"
94+
} else {
95+
"use `Option::is_some()` instead"
96+
},
97+
format!(
98+
"{}.{}",
99+
snippet_with_applicability(cx, scrutinee.span, "..", &mut applicability),
100+
if is_eq { "is_none()" } else { "is_some()" }
101+
),
102+
applicability,
103+
);
104+
}
105+
}
106+
}

‎tests/ui/partialeq_to_none.fixed‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::partialeq_to_none)]
4+
5+
#[allow(dead_code)]
6+
fn foo(f: Option<u32>) -> &'static str {
7+
if f.is_some() { "yay" } else { "nay" }
8+
}
9+
10+
fn foobar() -> Option<()> {
11+
None
12+
}
13+
14+
fn bar() -> Result<(), ()> {
15+
Ok(())
16+
}
17+
18+
fn main() {
19+
let x = Some(0);
20+
21+
let _ = x.is_none();
22+
let _ = x.is_some();
23+
let _ = x.is_none();
24+
let _ = x.is_some();
25+
26+
if foobar().is_none() {}
27+
28+
if bar().ok().is_some() {}
29+
30+
let _ = Some(1 + 2).is_some();
31+
32+
let _ = { Some(0) }.is_none();
33+
34+
let _ = {
35+
/*
36+
This comment runs long
37+
*/
38+
Some(1)
39+
}.is_some();
40+
}

‎tests/ui/partialeq_to_none.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::partialeq_to_none)]
4+
5+
#[allow(dead_code)]
6+
fn foo(f: Option<u32>) -> &'static str {
7+
if f != None { "yay" } else { "nay" }
8+
}
9+
10+
fn foobar() -> Option<()> {
11+
None
12+
}
13+
14+
fn bar() -> Result<(), ()> {
15+
Ok(())
16+
}
17+
18+
fn main() {
19+
let x = Some(0);
20+
21+
let _ = x == None;
22+
let _ = x != None;
23+
let _ = None == x;
24+
let _ = None != x;
25+
26+
if foobar() == None {}
27+
28+
if bar().ok() != None {}
29+
30+
let _ = Some(1 + 2) != None;
31+
32+
let _ = { Some(0) } == None;
33+
34+
let _ = {
35+
/*
36+
This comment runs long
37+
*/
38+
Some(1)
39+
} != None;
40+
}

‎tests/ui/partialeq_to_none.stderr‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
error: binary comparison to literal `Option::None`
2+
--> $DIR/partialeq_to_none.rs:7:8
3+
|
4+
LL | if f != None { "yay" } else { "nay" }
5+
| ^^^^^^^^^ help: use `Option::is_some()` instead: `f.is_some()`
6+
|
7+
= note: `-D clippy::partialeq-to-none` implied by `-D warnings`
8+
9+
error: binary comparison to literal `Option::None`
10+
--> $DIR/partialeq_to_none.rs:21:13
11+
|
12+
LL | let _ = x == None;
13+
| ^^^^^^^^^ help: use `Option::is_none()` instead: `x.is_none()`
14+
15+
error: binary comparison to literal `Option::None`
16+
--> $DIR/partialeq_to_none.rs:22:13
17+
|
18+
LL | let _ = x != None;
19+
| ^^^^^^^^^ help: use `Option::is_some()` instead: `x.is_some()`
20+
21+
error: binary comparison to literal `Option::None`
22+
--> $DIR/partialeq_to_none.rs:23:13
23+
|
24+
LL | let _ = None == x;
25+
| ^^^^^^^^^ help: use `Option::is_none()` instead: `x.is_none()`
26+
27+
error: binary comparison to literal `Option::None`
28+
--> $DIR/partialeq_to_none.rs:24:13
29+
|
30+
LL | let _ = None != x;
31+
| ^^^^^^^^^ help: use `Option::is_some()` instead: `x.is_some()`
32+
33+
error: binary comparison to literal `Option::None`
34+
--> $DIR/partialeq_to_none.rs:26:8
35+
|
36+
LL | if foobar() == None {}
37+
| ^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `foobar().is_none()`
38+
39+
error: binary comparison to literal `Option::None`
40+
--> $DIR/partialeq_to_none.rs:28:8
41+
|
42+
LL | if bar().ok() != None {}
43+
| ^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `bar().ok().is_some()`
44+
45+
error: binary comparison to literal `Option::None`
46+
--> $DIR/partialeq_to_none.rs:30:13
47+
|
48+
LL | let _ = Some(1 + 2) != None;
49+
| ^^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `Some(1 + 2).is_some()`
50+
51+
error: binary comparison to literal `Option::None`
52+
--> $DIR/partialeq_to_none.rs:32:13
53+
|
54+
LL | let _ = { Some(0) } == None;
55+
| ^^^^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `{ Some(0) }.is_none()`
56+
57+
error: binary comparison to literal `Option::None`
58+
--> $DIR/partialeq_to_none.rs:34:13
59+
|
60+
LL | let _ = {
61+
| _____________^
62+
LL | | /*
63+
LL | | This comment runs long
64+
LL | | */
65+
LL | | Some(1)
66+
LL | | } != None;
67+
| |_____________^
68+
|
69+
help: use `Option::is_some()` instead
70+
|
71+
LL ~ let _ = {
72+
LL + /*
73+
LL + This comment runs long
74+
LL + */
75+
LL + Some(1)
76+
LL ~ }.is_some();
77+
|
78+
79+
error: aborting due to 10 previous errors
80+

0 commit comments

Comments
 (0)
Please sign in to comment.