Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit b97d29a

Browse files
borsbors
committed
Auto merge of rust-lang#9689 - koka831:fix/or_fun_call_map_or, r=flip1995
fix: support `map_or` for `or_fun_call` lint fixes rust-lang/rust-clippy#8993 The methods defined in `KNOW_TYPES`, except for `map_or`, accepts only one argument, so the matching `if let [arg] = args` works only for these methods. This PR adds `rest_arg` argument to `check_general_case` method and handling of cases with two arguments to support `map_or`. changelog: `or_fun_call` support `map_or` * add `rest_arg` to pass second argument from `map_or(U, F)` * extract some procedures into closure
2 parents 3f4287c + a41cb7a commit b97d29a

File tree

7 files changed

+92
-21
lines changed

7 files changed

+92
-21
lines changed

‎clippy_lints/src/methods/or_fun_call.rs‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ pub(super) fn check<'tcx>(
8383
method_span: Span,
8484
self_expr: &hir::Expr<'_>,
8585
arg: &'tcx hir::Expr<'_>,
86+
// `Some` if fn has second argument
87+
second_arg: Option<&hir::Expr<'_>>,
8688
span: Span,
8789
// None if lambda is required
8890
fun_span: Option<Span>,
@@ -109,30 +111,40 @@ pub(super) fn check<'tcx>(
109111
if poss.contains(&name);
110112

111113
then {
112-
let macro_expanded_snipped;
113-
let sugg: Cow<'_, str> = {
114+
let sugg = {
114115
let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
115116
(false, Some(fun_span)) => (fun_span, false),
116117
_ => (arg.span, true),
117118
};
118-
let snippet = {
119-
let not_macro_argument_snippet = snippet_with_macro_callsite(cx, snippet_span, "..");
120-
if not_macro_argument_snippet == "vec![]" {
121-
macro_expanded_snipped = snippet(cx, snippet_span, "..");
119+
120+
let format_span = |span: Span| {
121+
let not_macro_argument_snippet = snippet_with_macro_callsite(cx, span, "..");
122+
let snip = if not_macro_argument_snippet == "vec![]" {
123+
let macro_expanded_snipped = snippet(cx, snippet_span, "..");
122124
match macro_expanded_snipped.strip_prefix("$crate::vec::") {
123-
Some(stripped) => Cow::from(stripped),
125+
Some(stripped) => Cow::Owned(stripped.to_owned()),
124126
None => macro_expanded_snipped,
125127
}
126128
} else {
127129
not_macro_argument_snippet
128-
}
130+
};
131+
132+
snip.to_string()
129133
};
130134

131-
if use_lambda {
135+
let snip = format_span(snippet_span);
136+
let snip = if use_lambda {
132137
let l_arg = if fn_has_arguments { "_" } else { "" };
133-
format!("|{l_arg}| {snippet}").into()
138+
format!("|{l_arg}| {snip}")
134139
} else {
135-
snippet
140+
snip
141+
};
142+
143+
if let Some(f) = second_arg {
144+
let f = format_span(f.span);
145+
format!("{snip}, {f}")
146+
} else {
147+
snip
136148
}
137149
};
138150
let span_replace_word = method_span.with_hi(span.hi());
@@ -149,8 +161,8 @@ pub(super) fn check<'tcx>(
149161
}
150162
}
151163

152-
if let [arg] = args {
153-
let inner_arg = if let hir::ExprKind::Block(
164+
let extract_inner_arg = |arg: &'tcx hir::Expr<'_>| {
165+
if let hir::ExprKind::Block(
154166
hir::Block {
155167
stmts: [],
156168
expr: Some(expr),
@@ -162,19 +174,32 @@ pub(super) fn check<'tcx>(
162174
expr
163175
} else {
164176
arg
165-
};
177+
}
178+
};
179+
180+
if let [arg] = args {
181+
let inner_arg = extract_inner_arg(arg);
166182
match inner_arg.kind {
167183
hir::ExprKind::Call(fun, or_args) => {
168184
let or_has_args = !or_args.is_empty();
169185
if !check_unwrap_or_default(cx, name, fun, arg, or_has_args, expr.span, method_span) {
170186
let fun_span = if or_has_args { None } else { Some(fun.span) };
171-
check_general_case(cx, name, method_span, receiver, arg, expr.span, fun_span);
187+
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, fun_span);
172188
}
173189
},
174190
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
175-
check_general_case(cx, name, method_span, receiver, arg, expr.span, None);
191+
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, None);
176192
},
177193
_ => (),
178194
}
179195
}
196+
197+
// `map_or` takes two arguments
198+
if let [arg, lambda] = args {
199+
let inner_arg = extract_inner_arg(arg);
200+
if let hir::ExprKind::Call(fun, or_args) = inner_arg.kind {
201+
let fun_span = if or_args.is_empty() { Some(fun.span) } else { None };
202+
check_general_case(cx, name, method_span, receiver, arg, Some(lambda), expr.span, fun_span);
203+
}
204+
}
180205
}

‎tests/ui/manual_ok_or.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// run-rustfix
22
#![warn(clippy::manual_ok_or)]
3+
#![allow(clippy::or_fun_call)]
34
#![allow(clippy::disallowed_names)]
45
#![allow(clippy::redundant_closure)]
56
#![allow(dead_code)]

‎tests/ui/manual_ok_or.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// run-rustfix
22
#![warn(clippy::manual_ok_or)]
3+
#![allow(clippy::or_fun_call)]
34
#![allow(clippy::disallowed_names)]
45
#![allow(clippy::redundant_closure)]
56
#![allow(dead_code)]

‎tests/ui/manual_ok_or.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
error: this pattern reimplements `Option::ok_or`
2-
--> $DIR/manual_ok_or.rs:11:5
2+
--> $DIR/manual_ok_or.rs:12:5
33
|
44
LL | foo.map_or(Err("error"), |v| Ok(v));
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
66
|
77
= note: `-D clippy::manual-ok-or` implied by `-D warnings`
88

99
error: this pattern reimplements `Option::ok_or`
10-
--> $DIR/manual_ok_or.rs:14:5
10+
--> $DIR/manual_ok_or.rs:15:5
1111
|
1212
LL | foo.map_or(Err("error"), Ok);
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
1414

1515
error: this pattern reimplements `Option::ok_or`
16-
--> $DIR/manual_ok_or.rs:17:5
16+
--> $DIR/manual_ok_or.rs:18:5
1717
|
1818
LL | None::<i32>.map_or(Err("error"), |v| Ok(v));
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `None::<i32>.ok_or("error")`
2020

2121
error: this pattern reimplements `Option::ok_or`
22-
--> $DIR/manual_ok_or.rs:21:5
22+
--> $DIR/manual_ok_or.rs:22:5
2323
|
2424
LL | / foo.map_or(Err::<i32, &str>(
2525
LL | | &format!(

‎tests/ui/or_fun_call.fixed‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,20 @@ mod issue9608 {
236236
}
237237
}
238238

239+
mod issue8993 {
240+
fn g() -> i32 {
241+
3
242+
}
243+
244+
fn f(n: i32) -> i32 {
245+
n
246+
}
247+
248+
fn test_map_or() {
249+
let _ = Some(4).map_or_else(g, |v| v);
250+
let _ = Some(4).map_or_else(g, f);
251+
let _ = Some(4).map_or(0, f);
252+
}
253+
}
254+
239255
fn main() {}

‎tests/ui/or_fun_call.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,20 @@ mod issue9608 {
236236
}
237237
}
238238

239+
mod issue8993 {
240+
fn g() -> i32 {
241+
3
242+
}
243+
244+
fn f(n: i32) -> i32 {
245+
n
246+
}
247+
248+
fn test_map_or() {
249+
let _ = Some(4).map_or(g(), |v| v);
250+
let _ = Some(4).map_or(g(), f);
251+
let _ = Some(4).map_or(0, f);
252+
}
253+
}
254+
239255
fn main() {}

‎tests/ui/or_fun_call.stderr‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,5 +156,17 @@ error: use of `unwrap_or` followed by a call to `new`
156156
LL | .unwrap_or(String::new());
157157
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
158158

159-
error: aborting due to 26 previous errors
159+
error: use of `map_or` followed by a function call
160+
--> $DIR/or_fun_call.rs:249:25
161+
|
162+
LL | let _ = Some(4).map_or(g(), |v| v);
163+
| ^^^^^^^^^^^^^^^^^^ help: try this: `map_or_else(g, |v| v)`
164+
165+
error: use of `map_or` followed by a function call
166+
--> $DIR/or_fun_call.rs:250:25
167+
|
168+
LL | let _ = Some(4).map_or(g(), f);
169+
| ^^^^^^^^^^^^^^ help: try this: `map_or_else(g, f)`
170+
171+
error: aborting due to 28 previous errors
160172

0 commit comments

Comments
 (0)