From 98585a03241f7b849b43f4ed9ede1b3ec42890d7 Mon Sep 17 00:00:00 2001
From: Michael Wright <mikerite@lavabit.com>
Date: Sun, 2 Jun 2019 08:47:06 +0200
Subject: [PATCH 1/2] Fix .map(..).unwrap_or_else(..) bad suggestion
Closes #4144
---
clippy_lints/src/methods/mod.rs | 14 ++++++++++++++
tests/ui/methods.rs | 15 +++++++++++++++
tests/ui/methods.stderr | 18 +++++++++---------
3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 4e57b46207d5..ae3f72e98e4e 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -20,6 +20,7 @@ use syntax::symbol::LocalInternedString;
use crate::utils::paths;
use crate::utils::sugg;
+use crate::utils::usage::mutated_variables;
use crate::utils::{
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
is_ctor_function, is_expn_of, is_self, is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path,
@@ -1880,7 +1881,20 @@ fn lint_map_unwrap_or_else<'a, 'tcx>(
// lint if the caller of `map()` is an `Option`
let is_option = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION);
let is_result = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::RESULT);
+
if is_option || is_result {
+ // Don't make a suggestion that may fail to compile due to mutably borrowing
+ // they same variable twice.
+ let map_mutated_vars = mutated_variables(&map_args[0], cx);
+ let unwrap_mutated_vars = mutated_variables(&unwrap_args[1], cx);
+ if let (Some(map_mutated_vars), Some(unwrap_mutated_vars)) = (map_mutated_vars, unwrap_mutated_vars) {
+ if map_mutated_vars.intersection(&unwrap_mutated_vars).next().is_some() {
+ return;
+ }
+ } else {
+ return;
+ }
+
// lint message
let msg = if is_option {
"called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling \
diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs
index 3a8aedcd6598..1b50c94ac03a 100644
--- a/tests/ui/methods.rs
+++ b/tests/ui/methods.rs
@@ -203,6 +203,21 @@ fn option_methods() {
// Macro case.
// Should not lint.
let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0);
+
+ // Issue #4144
+ {
+ let mut frequencies = HashMap::new();
+ let word = "foo";
+
+ frequencies
+ .get_mut(word)
+ .map(|count| {
+ *count += 1;
+ })
+ .unwrap_or_else(|| {
+ frequencies.insert(word.to_owned(), 1);
+ });
+ }
}
/// Checks implementation of `FILTER_NEXT` lint.
diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr
index 4bf53acf01ef..8d28c3282ae7 100644
--- a/tests/ui/methods.stderr
+++ b/tests/ui/methods.stderr
@@ -130,7 +130,7 @@ LL | | );
| |_________________^
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
- --> $DIR/methods.rs:214:13
+ --> $DIR/methods.rs:229:13
|
LL | let _ = v.iter().filter(|&x| *x < 0).next();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -139,7 +139,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next();
= note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)`
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
- --> $DIR/methods.rs:217:13
+ --> $DIR/methods.rs:232:13
|
LL | let _ = v.iter().filter(|&x| {
| _____________^
@@ -149,7 +149,7 @@ LL | | ).next();
| |___________________________^
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:233:13
+ --> $DIR/methods.rs:248:13
|
LL | let _ = v.iter().find(|&x| *x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -158,7 +158,7 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some();
= note: replace `find(|&x| *x < 0).is_some()` with `any(|x| *x < 0)`
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:236:13
+ --> $DIR/methods.rs:251:13
|
LL | let _ = v.iter().find(|&x| {
| _____________^
@@ -168,7 +168,7 @@ LL | | ).is_some();
| |______________________________^
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:242:13
+ --> $DIR/methods.rs:257:13
|
LL | let _ = v.iter().position(|&x| x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -176,7 +176,7 @@ LL | let _ = v.iter().position(|&x| x < 0).is_some();
= note: replace `position(|&x| x < 0).is_some()` with `any(|&x| x < 0)`
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:245:13
+ --> $DIR/methods.rs:260:13
|
LL | let _ = v.iter().position(|&x| {
| _____________^
@@ -186,7 +186,7 @@ LL | | ).is_some();
| |______________________________^
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:251:13
+ --> $DIR/methods.rs:266:13
|
LL | let _ = v.iter().rposition(|&x| x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -194,7 +194,7 @@ LL | let _ = v.iter().rposition(|&x| x < 0).is_some();
= note: replace `rposition(|&x| x < 0).is_some()` with `any(|&x| x < 0)`
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
- --> $DIR/methods.rs:254:13
+ --> $DIR/methods.rs:269:13
|
LL | let _ = v.iter().rposition(|&x| {
| _____________^
@@ -204,7 +204,7 @@ LL | | ).is_some();
| |______________________________^
error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
- --> $DIR/methods.rs:269:13
+ --> $DIR/methods.rs:284:13
|
LL | let _ = opt.unwrap();
| ^^^^^^^^^^^^
From 3b7d6eeb4fc4112504fc77dee6fc35a16c80f05c Mon Sep 17 00:00:00 2001
From: mikerite <33983332+mikerite@users.noreply.github.com>
Date: Tue, 4 Jun 2019 08:27:31 +0200
Subject: [PATCH 2/2] Fix comment grammar
Co-Authored-By: Philipp Krones <hello@philkrones.com>
---
clippy_lints/src/methods/mod.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index ae3f72e98e4e..1e7ff4907ae6 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1884,7 +1884,7 @@ fn lint_map_unwrap_or_else<'a, 'tcx>(
if is_option || is_result {
// Don't make a suggestion that may fail to compile due to mutably borrowing
- // they same variable twice.
+ // the same variable twice.
let map_mutated_vars = mutated_variables(&map_args[0], cx);
let unwrap_mutated_vars = mutated_variables(&unwrap_args[1], cx);
if let (Some(map_mutated_vars), Some(unwrap_mutated_vars)) = (map_mutated_vars, unwrap_mutated_vars) {