Skip to content

Commit f49a2c3

Browse files
committed
feat: add use_unwrap_or
1 parent 65e5cd0 commit f49a2c3

File tree

8 files changed

+157
-0
lines changed

8 files changed

+157
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,6 +3538,7 @@ Released 2018-09-13
35383538
[`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
35393539
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
35403540
[`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
3541+
[`use_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_unwrap_or
35413542
[`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
35423543
[`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref
35433544
[`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute

clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
310310
LintId::of(unwrap::PANICKING_UNWRAP),
311311
LintId::of(unwrap::UNNECESSARY_UNWRAP),
312312
LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS),
313+
LintId::of(use_unwrap_or::USE_UNWRAP_OR),
313314
LintId::of(useless_conversion::USELESS_CONVERSION),
314315
LintId::of(vec::USELESS_VEC),
315316
LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH),

clippy_lints/src/lib.register_complexity.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
9494
LintId::of(unit_types::UNIT_ARG),
9595
LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY),
9696
LintId::of(unwrap::UNNECESSARY_UNWRAP),
97+
LintId::of(use_unwrap_or::USE_UNWRAP_OR),
9798
LintId::of(useless_conversion::USELESS_CONVERSION),
9899
LintId::of(zero_div_zero::ZERO_DIVIDED_BY_ZERO),
99100
])

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ store.register_lints(&[
528528
unwrap_in_result::UNWRAP_IN_RESULT,
529529
upper_case_acronyms::UPPER_CASE_ACRONYMS,
530530
use_self::USE_SELF,
531+
use_unwrap_or::USE_UNWRAP_OR,
531532
useless_conversion::USELESS_CONVERSION,
532533
vec::USELESS_VEC,
533534
vec_init_then_push::VEC_INIT_THEN_PUSH,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ mod unwrap;
394394
mod unwrap_in_result;
395395
mod upper_case_acronyms;
396396
mod use_self;
397+
mod use_unwrap_or;
397398
mod useless_conversion;
398399
mod vec;
399400
mod vec_init_then_push;
@@ -866,6 +867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
866867
ignore_publish: cargo_ignore_publish,
867868
})
868869
});
870+
store.register_late_pass(|| Box::new(use_unwrap_or::UseUnwrapOr));
869871
// add lints here, do not remove this comment, it's used in `new_lint`
870872
}
871873

clippy_lints/src/use_unwrap_or.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
use if_chain::if_chain;
2+
use rustc_hir::*;
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
use rustc_span::{Span, sym};
6+
use clippy_utils::diagnostics::span_lint_and_help;
7+
use clippy_utils::ty::is_type_diagnostic_item;
8+
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for `.or(…).unwrap()` calls to Options and Results.
13+
///
14+
/// ### Why is this bad?
15+
/// You should use `.unwrap_or(…)` instead for clarity.
16+
///
17+
/// ### Example
18+
/// ```rust
19+
/// // Result
20+
/// let port = result.or::<Error>(Ok(fallback)).unwrap();
21+
///
22+
/// // Option
23+
/// let value = option.or(Some(fallback)).unwrap();
24+
/// ```
25+
/// Use instead:
26+
/// ```rust
27+
/// // Result
28+
/// let port = result.unwrap_or(fallback);
29+
///
30+
/// // Option
31+
/// let value = option.unwrap_or(fallback);
32+
/// ```
33+
#[clippy::version = "1.61.0"]
34+
pub USE_UNWRAP_OR,
35+
complexity,
36+
"checks for `.or(…).unwrap()` calls to Options and Results."
37+
}
38+
declare_lint_pass!(UseUnwrapOr => [USE_UNWRAP_OR]);
39+
40+
impl<'tcx> LateLintPass<'tcx> for UseUnwrapOr {
41+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
42+
43+
// look for x.or().unwrap()
44+
if_chain! {
45+
if let ExprKind::MethodCall(path, args, unwrap_span) = expr.kind;
46+
if path.ident.name.as_str() == "unwrap";
47+
if let Some(caller) = args.first();
48+
if let ExprKind::MethodCall(caller_path, caller_args, or_span) = caller.kind;
49+
if caller_path.ident.name.as_str() == "or";
50+
then {
51+
let ty = cx.typeck_results().expr_ty(&caller_args[0]); // get type of x (we later check if it's Option or Result)
52+
let title;
53+
let arg = &caller_args[1]; // the argument or(xyz) is called with
54+
55+
if is_type_diagnostic_item(&cx, ty, sym::Option) {
56+
title = ".or(Some(…)).unwrap() found";
57+
if !is(arg, "Some") {
58+
return;
59+
}
60+
61+
} else if is_type_diagnostic_item(&cx, ty, sym::Result) {
62+
title = ".or(Ok(…)).unwrap() found";
63+
if !is(arg, "Ok") {
64+
return;
65+
}
66+
} else {
67+
// Someone has implemented a struct with .or(...).unwrap() chaining,
68+
// but it's not an Option or a Result, so bail
69+
return;
70+
}
71+
72+
// span = or_span + unwrap_span
73+
let span = Span::new(or_span.lo(), unwrap_span.hi(), or_span.ctxt(), or_span.parent());
74+
75+
span_lint_and_help(
76+
cx,
77+
USE_UNWRAP_OR,
78+
span,
79+
title,
80+
None,
81+
"use `unwrap_or()` instead"
82+
);
83+
}
84+
}
85+
}
86+
}
87+
88+
/// is expr a Call to name?
89+
/// name might be "Some", "Ok", "Err", etc.
90+
fn is<'a>(expr: &Expr<'a>, name: &str) -> bool {
91+
if_chain! {
92+
if let ExprKind::Call(some_expr, _some_args) = expr.kind;
93+
if let ExprKind::Path(path) = &some_expr.kind;
94+
if let QPath::Resolved(_, path) = path;
95+
if let Some(path_segment) = path.segments.first();
96+
if path_segment.ident.name.as_str() == name;
97+
then {
98+
return true;
99+
}
100+
else {
101+
return false;
102+
}
103+
}
104+
}
105+

tests/ui/use_unwrap_or.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![warn(clippy::use_unwrap_or)]
2+
3+
struct SomeStruct {}
4+
impl SomeStruct {
5+
fn or(self, _: Option<Self>) -> Self { self }
6+
fn unwrap(&self){}
7+
}
8+
9+
fn main() {
10+
let option: Option<&str> = None;
11+
let _ = option.or(Some("fallback")).unwrap(); // should trigger lint
12+
13+
let result: Result<&str, &str> = Err("Error");
14+
let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint
15+
16+
// Not Option/Result
17+
let instance = SomeStruct {};
18+
let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint
19+
20+
// None in or
21+
let option: Option<&str> = None;
22+
let _ = option.or(None).unwrap(); // should not trigger lint
23+
24+
// Not Err in or
25+
let result: Result<&str, &str> = Err("Error");
26+
let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint
27+
}

tests/ui/use_unwrap_or.stderr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: .or(Some(…)).unwrap() found
2+
--> $DIR/use_unwrap_or.rs:11:20
3+
|
4+
LL | let _ = option.or(Some("fallback")).unwrap(); // should trigger lint
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::use-unwrap-or` implied by `-D warnings`
8+
= help: use `unwrap_or()` instead
9+
10+
error: .or(Ok(…)).unwrap() found
11+
--> $DIR/use_unwrap_or.rs:14:20
12+
|
13+
LL | let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: use `unwrap_or()` instead
17+
18+
error: aborting due to 2 previous errors
19+

0 commit comments

Comments
 (0)