Skip to content

Commit

Permalink
or_fun_call: fix suggestion for or_insert(vec![])
Browse files Browse the repository at this point in the history
Applies for `std::collections::hash_map::Entry` and `std::collections::btree_map::Entry`

Example:
Previously, for the following code:
`let _ = hash_map.entry("test".to_owned()).or_insert(vec![]);`
clippy would suggest to use:
`or_insert_with(vec![])`, which causes a compiler error (E0277).

Now clippy suggests:
`or_insert_with(Vec::new)`
  • Loading branch information
mgacek8 committed Feb 25, 2021
1 parent 877be18 commit 2f0e9f7
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 8 deletions.
16 changes: 15 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2005,12 +2005,26 @@ fn lint_or_fun_call<'tcx>(
if poss.contains(&name);

then {
let macro_expanded_snipped;
let sugg: Cow<'_, str> = {
let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
(false, Some(fun_span)) => (fun_span, false),
_ => (arg.span, true),
};
let snippet = snippet_with_macro_callsite(cx, snippet_span, "..");
let snippet = {
let not_macro_argument_snippet = snippet_with_macro_callsite(cx, snippet_span, "..");
if not_macro_argument_snippet == "vec![]" {
macro_expanded_snipped = snippet(cx, snippet_span, "..");
match macro_expanded_snipped.strip_prefix("$crate::vec::") {
Some(stripped) => Cow::from(stripped),
None => macro_expanded_snipped
}
}
else {
not_macro_argument_snippet
}
};

if use_lambda {
let l_arg = if fn_has_arguments { "_" } else { "" };
format!("|{}| {}", l_arg, snippet).into()
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/or_fun_call.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,15 @@ fn or_fun_call() {
let mut map = HashMap::<u64, String>::new();
map.entry(42).or_insert_with(String::new);

let mut map_vec = HashMap::<u64, Vec<i32>>::new();
map_vec.entry(42).or_insert_with(Vec::new);

let mut btree = BTreeMap::<u64, String>::new();
btree.entry(42).or_insert_with(String::new);

let mut btree_vec = BTreeMap::<u64, Vec<i32>>::new();
btree_vec.entry(42).or_insert_with(Vec::new);

let stringy = Some(String::from(""));
let _ = stringy.unwrap_or_else(|| "".to_owned());

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,15 @@ fn or_fun_call() {
let mut map = HashMap::<u64, String>::new();
map.entry(42).or_insert(String::new());

let mut map_vec = HashMap::<u64, Vec<i32>>::new();
map_vec.entry(42).or_insert(vec![]);

let mut btree = BTreeMap::<u64, String>::new();
btree.entry(42).or_insert(String::new());

let mut btree_vec = BTreeMap::<u64, Vec<i32>>::new();
btree_vec.entry(42).or_insert(vec![]);

let stringy = Some(String::from(""));
let _ = stringy.unwrap_or("".to_owned());

Expand Down
26 changes: 19 additions & 7 deletions tests/ui/or_fun_call.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -67,40 +67,52 @@ LL | map.entry(42).or_insert(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`

error: use of `or_insert` followed by a function call
--> $DIR/or_fun_call.rs:66:21
--> $DIR/or_fun_call.rs:66:23
|
LL | map_vec.entry(42).or_insert(vec![]);
| ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)`

error: use of `or_insert` followed by a function call
--> $DIR/or_fun_call.rs:69:21
|
LL | btree.entry(42).or_insert(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`

error: use of `or_insert` followed by a function call
--> $DIR/or_fun_call.rs:72:25
|
LL | btree_vec.entry(42).or_insert(vec![]);
| ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)`

error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:69:21
--> $DIR/or_fun_call.rs:75:21
|
LL | let _ = stringy.unwrap_or("".to_owned());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`

error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:77:21
--> $DIR/or_fun_call.rs:83:21
|
LL | let _ = Some(1).unwrap_or(map[&1]);
| ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])`

error: use of `unwrap_or` followed by a function call
--> $DIR/or_fun_call.rs:79:21
--> $DIR/or_fun_call.rs:85:21
|
LL | let _ = Some(1).unwrap_or(map[&1]);
| ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])`

error: use of `or` followed by a function call
--> $DIR/or_fun_call.rs:103:35
--> $DIR/or_fun_call.rs:109:35
|
LL | let _ = Some("a".to_string()).or(Some("b".to_string()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))`

error: use of `or` followed by a function call
--> $DIR/or_fun_call.rs:107:10
--> $DIR/or_fun_call.rs:113:10
|
LL | .or(Some(Bar(b, Duration::from_secs(2))));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))`

error: aborting due to 17 previous errors
error: aborting due to 19 previous errors

0 comments on commit 2f0e9f7

Please sign in to comment.