Skip to content

Commit

Permalink
Auto merge of #13528 - zvavybir:master, r=Alexendoo
Browse files Browse the repository at this point in the history
Improved wording of or_fun_call lint

The current wording (e.g. ``use of `ok_or` followed by a function call``) is potentially confusing (at least it confused me) by suggesting that the function that follows the (in this case) `ok_or` is the problem and not the function that is an argument to it.

The code in my program that triggered the confusing message is the following:
```rust
let file_id = buf
    .lines()
    .next()
    .ok_or((
        InternalError::ProblemReadingFromInbox,
        anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
    ))
    .html_context(stream, lang)?;
```
I thought that `html_context` was the problem and that I should do something along the following lines:
```rust
let file_id = buf
    .lines()
    .next()
    .ok_or_else(
        (
            InternalError::ProblemReadingFromInbox,
            anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
        ),
        html_context(stream, lang),
    )?
```
This is of course wrong.  My confusion was only cleared up through the help message indicating what I should try instead.

If someone has a better idea of a replacement wording (currently e.g. ``` function call inside of `ok_or` ```), I'm all ears.

changelog: none
  • Loading branch information
bors committed Oct 11, 2024
2 parents 6f1def7 + 8c46c49 commit b85f632
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 54 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/methods/expect_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub(super) fn check<'tcx>(
cx,
EXPECT_FUN_CALL,
span_replace_word,
format!("use of `{name}` followed by a function call"),
format!("function call inside of `{name}`"),
"try",
format!("unwrap_or_else({closure_args} panic!({sugg}))"),
applicability,
Expand All @@ -161,7 +161,7 @@ pub(super) fn check<'tcx>(
cx,
EXPECT_FUN_CALL,
span_replace_word,
format!("use of `{name}` followed by a function call"),
format!("function call inside of `{name}`"),
"try",
format!("unwrap_or_else({closure_args} {{ panic!(\"{{}}\", {arg_root_snippet}) }})"),
applicability,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ pub(super) fn check<'tcx>(
cx,
OR_FUN_CALL,
span_replace_word,
format!("use of `{name}` followed by a function call"),
format!("function call inside of `{name}`"),
"try",
format!("{name}_{suffix}({sugg})"),
app,
Expand Down
30 changes: 15 additions & 15 deletions tests/ui/expect_fun_call.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:35:26
|
LL | with_none_and_format.expect(&format!("Error {}: fake error", error_code));
Expand All @@ -7,85 +7,85 @@ LL | with_none_and_format.expect(&format!("Error {}: fake error", error_code
= note: `-D clippy::expect-fun-call` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::expect_fun_call)]`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:38:26
|
LL | with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:41:37
|
LL | with_none_and_format_with_macro.expect(format!("Error {}: fake error", one!()).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("Error {}: fake error", one!()))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:51:25
|
LL | with_err_and_format.expect(&format!("Error {}: fake error", error_code));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:54:25
|
LL | with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:66:17
|
LL | Some("foo").expect(format!("{} {}", 1, 2).as_ref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("{} {}", 1, 2))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:87:21
|
LL | Some("foo").expect(&get_string());
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| { panic!("{}", get_string()) })`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:88:21
|
LL | Some("foo").expect(get_string().as_ref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| { panic!("{}", get_string()) })`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:89:21
|
LL | Some("foo").expect(get_string().as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| { panic!("{}", get_string()) })`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:91:21
|
LL | Some("foo").expect(get_static_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| { panic!("{}", get_static_str()) })`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:92:21
|
LL | Some("foo").expect(get_non_static_str(&0));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| { panic!("{}", get_non_static_str(&0).to_string()) })`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:96:16
|
LL | Some(true).expect(&format!("key {}, {}", 1, 2));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("key {}, {}", 1, 2))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:102:17
|
LL | opt_ref.expect(&format!("{:?}", opt_ref));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("{:?}", opt_ref))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:106:20
|
LL | format_capture.expect(&format!("{error_code}"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("{error_code}"))`

error: use of `expect` followed by a function call
error: function call inside of `expect`
--> tests/ui/expect_fun_call.rs:109:30
|
LL | format_capture_and_value.expect(&format!("{error_code}, {}", 1));
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/or_fun_call.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -341,18 +341,18 @@ fn fn_call_in_nested_expr() {
}
let opt: Option<i32> = Some(1);

//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or_else(f); // suggest `.unwrap_or_else(f)`
//
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or_else(|| f() + 1); // suggest `.unwrap_or_else(|| f() + 1)`
//
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or_else(|| {
let x = f();
x + 1
});
//~v ERROR: use of `map_or` followed by a function call
//~v ERROR: function call inside of `map_or`
let _ = opt.map_or_else(|| f() + 1, |v| v); // suggest `.map_or_else(|| f() + 1, |v| v)`
//
//~v ERROR: use of `unwrap_or` to construct default value
Expand All @@ -361,7 +361,7 @@ fn fn_call_in_nested_expr() {
let opt_foo = Some(Foo {
val: String::from("123"),
});
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt_foo.unwrap_or_else(|| Foo { val: String::default() });
}

Expand Down
10 changes: 5 additions & 5 deletions tests/ui/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,18 +341,18 @@ fn fn_call_in_nested_expr() {
}
let opt: Option<i32> = Some(1);

//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or({ f() }); // suggest `.unwrap_or_else(f)`
//
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or(f() + 1); // suggest `.unwrap_or_else(|| f() + 1)`
//
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt.unwrap_or({
let x = f();
x + 1
});
//~v ERROR: use of `map_or` followed by a function call
//~v ERROR: function call inside of `map_or`
let _ = opt.map_or(f() + 1, |v| v); // suggest `.map_or_else(|| f() + 1, |v| v)`
//
//~v ERROR: use of `unwrap_or` to construct default value
Expand All @@ -361,7 +361,7 @@ fn fn_call_in_nested_expr() {
let opt_foo = Some(Foo {
val: String::from("123"),
});
//~v ERROR: use of `unwrap_or` followed by a function call
//~v ERROR: function call inside of `unwrap_or`
let _ = opt_foo.unwrap_or(Foo { val: String::default() });
}

Expand Down
40 changes: 20 additions & 20 deletions tests/ui/or_fun_call.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:52:22
|
LL | with_constructor.unwrap_or(make());
Expand All @@ -16,19 +16,19 @@ LL | with_new.unwrap_or(Vec::new());
= note: `-D clippy::unwrap-or-default` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unwrap_or_default)]`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:58:21
|
LL | with_const_args.unwrap_or(Vec::with_capacity(12));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Vec::with_capacity(12))`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:61:14
|
LL | with_err.unwrap_or(make());
| ^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|_| make())`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:64:19
|
LL | with_err_args.unwrap_or(Vec::with_capacity(12));
Expand All @@ -46,7 +46,7 @@ error: use of `unwrap_or` to construct default value
LL | with_default_type.unwrap_or(u64::default());
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:73:18
|
LL | self_default.unwrap_or(<FakeDefault>::default());
Expand All @@ -64,7 +64,7 @@ error: use of `unwrap_or` to construct default value
LL | with_vec.unwrap_or(vec![]);
| ^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:82:21
|
LL | without_default.unwrap_or(Foo::new());
Expand Down Expand Up @@ -100,55 +100,55 @@ error: use of `unwrap_or` to construct default value
LL | let _ = stringy.unwrap_or(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`

error: use of `ok_or` followed by a function call
error: function call inside of `ok_or`
--> tests/ui/or_fun_call.rs:101:17
|
LL | let _ = opt.ok_or(format!("{} world.", hello));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ok_or_else(|| format!("{} world.", hello))`

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

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

error: use of `or` followed by a function call
error: function call inside of `or`
--> tests/ui/or_fun_call.rs:131:35
|
LL | let _ = Some("a".to_string()).or(Some("b".to_string()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_else(|| Some("b".to_string()))`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:170:14
|
LL | None.unwrap_or(ptr_to_ref(s));
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| ptr_to_ref(s))`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:176:14
|
LL | None.unwrap_or(unsafe { ptr_to_ref(s) });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:178:14
|
LL | None.unwrap_or( unsafe { ptr_to_ref(s) } );
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`

error: use of `map_or` followed by a function call
error: function call inside of `map_or`
--> tests/ui/or_fun_call.rs:253:25
|
LL | let _ = Some(4).map_or(g(), |v| v);
| ^^^^^^^^^^^^^^^^^^ help: try: `map_or_else(g, |v| v)`

error: use of `map_or` followed by a function call
error: function call inside of `map_or`
--> tests/ui/or_fun_call.rs:254:25
|
LL | let _ = Some(4).map_or(g(), f);
Expand Down Expand Up @@ -196,19 +196,19 @@ error: use of `unwrap_or_else` to construct default value
LL | let _ = stringy.unwrap_or_else(String::new);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:345:17
|
LL | let _ = opt.unwrap_or({ f() }); // suggest `.unwrap_or_else(f)`
| ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(f)`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:348:17
|
LL | let _ = opt.unwrap_or(f() + 1); // suggest `.unwrap_or_else(|| f() + 1)`
| ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| f() + 1)`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:351:17
|
LL | let _ = opt.unwrap_or({
Expand All @@ -226,7 +226,7 @@ LL + x + 1
LL ~ });
|

error: use of `map_or` followed by a function call
error: function call inside of `map_or`
--> tests/ui/or_fun_call.rs:356:17
|
LL | let _ = opt.map_or(f() + 1, |v| v); // suggest `.map_or_else(|| f() + 1, |v| v)`
Expand All @@ -238,7 +238,7 @@ error: use of `unwrap_or` to construct default value
LL | let _ = opt.unwrap_or({ i32::default() });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/or_fun_call.rs:365:21
|
LL | let _ = opt_foo.unwrap_or(Foo { val: String::default() });
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/unwrap_or.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

fn main() {
let s = Some(String::from("test string")).unwrap_or_else(|| "Fail".to_string()).len();
//~^ ERROR: use of `unwrap_or` followed by a function call
//~^ ERROR: function call inside of `unwrap_or`
//~| NOTE: `-D clippy::or-fun-call` implied by `-D warnings`
}

fn new_lines() {
let s = Some(String::from("test string")).unwrap_or_else(|| "Fail".to_string()).len();
//~^ ERROR: use of `unwrap_or` followed by a function call
//~^ ERROR: function call inside of `unwrap_or`
}
4 changes: 2 additions & 2 deletions tests/ui/unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

fn main() {
let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len();
//~^ ERROR: use of `unwrap_or` followed by a function call
//~^ ERROR: function call inside of `unwrap_or`
//~| NOTE: `-D clippy::or-fun-call` implied by `-D warnings`
}

fn new_lines() {
let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len();
//~^ ERROR: use of `unwrap_or` followed by a function call
//~^ ERROR: function call inside of `unwrap_or`
}
4 changes: 2 additions & 2 deletions tests/ui/unwrap_or.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/unwrap_or.rs:5:47
|
LL | let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len();
Expand All @@ -7,7 +7,7 @@ LL | let s = Some(String::from("test string")).unwrap_or("Fail".to_string())
= note: `-D clippy::or-fun-call` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::or_fun_call)]`

error: use of `unwrap_or` followed by a function call
error: function call inside of `unwrap_or`
--> tests/ui/unwrap_or.rs:11:47
|
LL | let s = Some(String::from("test string")).unwrap_or("Fail".to_string()).len();
Expand Down

0 comments on commit b85f632

Please sign in to comment.