Skip to content

make use of Result::map_or #4848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> {
fn is_large_box(&self, ty: Ty<'tcx>) -> bool {
// Large types need to be boxed to avoid stack overflows.
if ty.is_box() {
self.cx.layout_of(ty.boxed_ty()).ok().map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
self.cx.layout_of(ty.boxed_ty()).map_or(0, |l| l.size.bytes()) > self.too_large_for_stack
} else {
false
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![feature(crate_visibility_modifier)]
#![feature(concat_idents)]
#![feature(result_map_or)]

// FIXME: switch to something more ergonomic here, once available.
// (Currently there is no way to opt into sysroot crates without `extern crate`.)
Expand Down
12 changes: 5 additions & 7 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ declare_clippy_lint! {
/// **What it does:** Checks for usage of `result.map(_).unwrap_or_else(_)`.
///
/// **Why is this bad?** Readability, this can be written more concisely as
/// `result.ok().map_or_else(_, _)`.
/// `result.map_or_else(_, _)`.
///
/// **Known problems:** None.
///
Expand All @@ -303,7 +303,7 @@ declare_clippy_lint! {
/// ```
pub RESULT_MAP_UNWRAP_OR_ELSE,
pedantic,
"using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.ok().map_or_else(g, f)`"
"using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.map_or_else(g, f)`"
}

declare_clippy_lint! {
Expand Down Expand Up @@ -2217,7 +2217,7 @@ fn lint_map_unwrap_or_else<'a, 'tcx>(
`map_or_else(g, f)` instead"
} else {
"called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling \
`ok().map_or_else(g, f)` instead"
`.map_or_else(g, f)` instead"
};
// get snippets for args to map() and unwrap_or_else()
let map_snippet = snippet(cx, map_args[1].span, "..");
Expand All @@ -2238,10 +2238,8 @@ fn lint_map_unwrap_or_else<'a, 'tcx>(
msg,
expr.span,
&format!(
"replace `map({0}).unwrap_or_else({1})` with `{2}map_or_else({1}, {0})`",
map_snippet,
unwrap_snippet,
if is_result { "ok()." } else { "" }
"replace `map({0}).unwrap_or_else({1})` with `map_or_else({1}, {0})`",
map_snippet, unwrap_snippet,
),
);
} else if same_span && multiline {
Expand Down
3 changes: 2 additions & 1 deletion src/driver.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![feature(result_map_or)]
#![feature(rustc_private)]

// FIXME: switch to something more ergonomic here, once available.
Expand Down Expand Up @@ -319,7 +320,7 @@ pub fn main() {
// this check ensures that dependencies are built but not linted and the final
// crate is
// linted but not built
let clippy_enabled = env::var("CLIPPY_TESTS").ok().map_or(false, |val| val == "true")
let clippy_enabled = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true")
|| arg_value(&orig_args, "--emit", |val| val.split(',').any(|e| e == "metadata")).is_some();

if clippy_enabled {
Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ pub const ALL_LINTS: [Lint; 337] = [
Lint {
name: "result_map_unwrap_or_else",
group: "pedantic",
desc: "using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.ok().map_or_else(g, f)`",
desc: "using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.map_or_else(g, f)`",
deprecation: None,
module: "methods",
},
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/result_map_unwrap_or_else.stderr
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `ok().map_or_else(g, f)` instead
error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `.map_or_else(g, f)` instead
--> $DIR/result_map_unwrap_or_else.rs:15:13
|
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); // should lint even though this call is on a separate line
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::result-map-unwrap-or-else` implied by `-D warnings`
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `ok().map_or_else(|e| 0, |x| x + 1)`
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)`

error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `ok().map_or_else(g, f)` instead
error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `.map_or_else(g, f)` instead
--> $DIR/result_map_unwrap_or_else.rs:17:13
|
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `ok().map_or_else(|e| 0, |x| x + 1)`
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)`

error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `ok().map_or_else(g, f)` instead
error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done more directly by calling `.map_or_else(g, f)` instead
--> $DIR/result_map_unwrap_or_else.rs:18:13
|
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `ok().map_or_else(|e| 0, |x| x + 1)`
= note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)`

error: aborting due to 3 previous errors