Skip to content

Commit

Permalink
Auto merge of #11860 - GuillaumeGomez:improve-error-messages, r=flip1995
Browse files Browse the repository at this point in the history
Improve error messages format

Following review in #11845, since there is already suggestions, no need to add the second part of the sentence every time.

r? `@flip1995`

changelog: Improve some error messages
  • Loading branch information
bors committed Nov 23, 2023
2 parents 840e227 + 6c84b96 commit c3243f9
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 61 deletions.
6 changes: 2 additions & 4 deletions clippy_lints/src/methods/map_unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,9 @@ pub(super) fn check<'tcx>(

// 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 \
`map_or_else(<g>, <f>)` instead"
"called `map(<f>).unwrap_or_else(<g>)` on an `Option` value"
} else {
"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"
"called `map(<f>).unwrap_or_else(<g>)` on a `Result` value"
};
// get snippets for args to map() and unwrap_or_else()
let map_snippet = snippet(cx, map_arg.span, "..");
Expand Down
5 changes: 1 addition & 4 deletions clippy_lints/src/methods/option_as_ref_deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,7 @@ pub(super) fn check(
let hint = format!("{}.{method_hint}()", snippet(cx, as_ref_recv.span, ".."));
let suggestion = format!("try using {method_hint} instead");

let msg = format!(
"called `{current_method}` on an Option value. This can be done more directly \
by calling `{hint}` instead"
);
let msg = format!("called `{current_method}` on an `Option` value");
span_lint_and_sugg(
cx,
OPTION_AS_REF_DEREF,
Expand Down
9 changes: 3 additions & 6 deletions clippy_lints/src/methods/option_map_or_none.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ pub(super) fn check<'tcx>(
&& Some(id) == cx.tcx.lang_items().option_some_variant()
{
let func_snippet = snippet(cx, arg_char.span, "..");
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
`map(..)` instead";
let msg = "called `map_or(None, ..)` on an `Option` value";
return span_lint_and_sugg(
cx,
OPTION_MAP_OR_NONE,
Expand All @@ -80,8 +79,7 @@ pub(super) fn check<'tcx>(
}

let func_snippet = snippet(cx, map_arg.span, "..");
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
`and_then(..)` instead";
let msg = "called `map_or(None, ..)` on an `Option` value";
span_lint_and_sugg(
cx,
OPTION_MAP_OR_NONE,
Expand All @@ -92,8 +90,7 @@ pub(super) fn check<'tcx>(
Applicability::MachineApplicable,
);
} else if f_arg_is_some {
let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
`ok()` instead";
let msg = "called `map_or(None, Some)` on a `Result` value";
let self_snippet = snippet(cx, recv.span, "..");
span_lint_and_sugg(
cx,
Expand Down
5 changes: 1 addition & 4 deletions clippy_lints/src/methods/option_map_unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,7 @@ pub(super) fn check<'tcx>(
} else {
"map_or(<a>, <f>)"
};
let msg = &format!(
"called `map(<f>).unwrap_or({arg})` on an `Option` value. \
This can be done more directly by calling `{suggest}` instead"
);
let msg = &format!("called `map(<f>).unwrap_or({arg})` on an `Option` value");

span_lint_and_then(cx, MAP_UNWRAP_OR, expr.span, msg, |diag| {
let map_arg_span = map_arg.span;
Expand Down
30 changes: 15 additions & 15 deletions tests/ui/map_unwrap_or.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:17:13
|
LL | let _ = opt.map(|x| x + 1)
Expand All @@ -15,7 +15,7 @@ LL - let _ = opt.map(|x| x + 1)
LL + let _ = opt.map_or(0, |x| x + 1);
|

error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:21:13
|
LL | let _ = opt.map(|x| {
Expand All @@ -33,7 +33,7 @@ LL | }
LL ~ );
|

error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:25:13
|
LL | let _ = opt.map(|x| x + 1)
Expand All @@ -50,7 +50,7 @@ LL + 0
LL ~ }, |x| x + 1);
|

error: called `map(<f>).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(<f>)` instead
error: called `map(<f>).unwrap_or(None)` on an `Option` value
--> $DIR/map_unwrap_or.rs:30:13
|
LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
Expand All @@ -62,7 +62,7 @@ LL - let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
LL + let _ = opt.and_then(|x| Some(x + 1));
|

error: called `map(<f>).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(<f>)` instead
error: called `map(<f>).unwrap_or(None)` on an `Option` value
--> $DIR/map_unwrap_or.rs:32:13
|
LL | let _ = opt.map(|x| {
Expand All @@ -80,7 +80,7 @@ LL | }
LL ~ );
|

error: called `map(<f>).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(<f>)` instead
error: called `map(<f>).unwrap_or(None)` on an `Option` value
--> $DIR/map_unwrap_or.rs:36:13
|
LL | let _ = opt
Expand All @@ -95,7 +95,7 @@ LL - .map(|x| Some(x + 1))
LL + .and_then(|x| Some(x + 1));
|

error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:47:13
|
LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
Expand All @@ -107,7 +107,7 @@ LL - let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
LL + let _ = Some("prefix").map_or(id, |p| format!("{}.", p));
|

error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value. This can be done more directly by calling `map_or_else(<g>, <f>)` instead
error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:51:13
|
LL | let _ = opt.map(|x| {
Expand All @@ -117,7 +117,7 @@ LL | | }
LL | | ).unwrap_or_else(|| 0);
| |__________________________^

error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value. This can be done more directly by calling `map_or_else(<g>, <f>)` instead
error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:55:13
|
LL | let _ = opt.map(|x| x + 1)
Expand All @@ -127,7 +127,7 @@ LL | | 0
LL | | );
| |_________^

error: called `map(<f>).unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and(<f>)` instead
error: called `map(<f>).unwrap_or(false)` on an `Option` value
--> $DIR/map_unwrap_or.rs:61:13
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
Expand All @@ -139,7 +139,7 @@ LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
LL + let _ = opt.is_some_and(|x| x > 5);
|

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
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value
--> $DIR/map_unwrap_or.rs:71:13
|
LL | let _ = res.map(|x| {
Expand All @@ -149,7 +149,7 @@ LL | | }
LL | | ).unwrap_or_else(|_e| 0);
| |____________________________^

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
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value
--> $DIR/map_unwrap_or.rs:75:13
|
LL | let _ = res.map(|x| x + 1)
Expand All @@ -159,13 +159,13 @@ LL | | 0
LL | | });
| |__________^

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
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value
--> $DIR/map_unwrap_or.rs:99:13
|
LL | let _ = res.map(|x| x + 1).unwrap_or_else(|_e| 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or_else(|_e| 0, |x| x + 1)`

error: called `map(<f>).unwrap_or(<a>)` on an `Option` value. This can be done more directly by calling `map_or(<a>, <f>)` instead
error: called `map(<f>).unwrap_or(<a>)` on an `Option` value
--> $DIR/map_unwrap_or.rs:106:13
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
Expand All @@ -177,7 +177,7 @@ LL - let _ = opt.map(|x| x > 5).unwrap_or(false);
LL + let _ = opt.map_or(false, |x| x > 5);
|

error: called `map(<f>).unwrap_or(false)` on an `Option` value. This can be done more directly by calling `is_some_and(<f>)` instead
error: called `map(<f>).unwrap_or(false)` on an `Option` value
--> $DIR/map_unwrap_or.rs:113:13
|
LL | let _ = opt.map(|x| x > 5).unwrap_or(false);
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/map_unwrap_or_fixable.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value. This can be done more directly by calling `map_or_else(<g>, <f>)` instead
error: called `map(<f>).unwrap_or_else(<g>)` on an `Option` value
--> $DIR/map_unwrap_or_fixable.rs:16:13
|
LL | let _ = opt.map(|x| x + 1)
Expand All @@ -10,7 +10,7 @@ LL | | .unwrap_or_else(|| 0);
= note: `-D clippy::map-unwrap-or` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::map_unwrap_or)]`

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
error: called `map(<f>).unwrap_or_else(<g>)` on a `Result` value
--> $DIR/map_unwrap_or_fixable.rs:46:13
|
LL | let _ = res.map(|x| x + 1)
Expand Down
36 changes: 18 additions & 18 deletions tests/ui/option_as_ref_deref.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
error: called `.as_ref().map(Deref::deref)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:11:13
|
LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len);
Expand All @@ -7,7 +7,7 @@ LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len);
= note: `-D clippy::option-as-ref-deref` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::option_as_ref_deref)]`

error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
error: called `.as_ref().map(Deref::deref)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:14:13
|
LL | let _ = opt.clone()
Expand All @@ -17,97 +17,97 @@ LL | | Deref::deref
LL | | )
| |_________^ help: try using as_deref instead: `opt.clone().as_deref()`

error: called `.as_mut().map(DerefMut::deref_mut)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(DerefMut::deref_mut)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:20:13
|
LL | let _ = opt.as_mut().map(DerefMut::deref_mut);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`

error: called `.as_ref().map(String::as_str)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(String::as_str)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:22:13
|
LL | let _ = opt.as_ref().map(String::as_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`

error: called `.as_ref().map(|x| x.as_str())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(|x| x.as_str())` on an `Option` value
--> $DIR/option_as_ref_deref.rs:23:13
|
LL | let _ = opt.as_ref().map(|x| x.as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`

error: called `.as_mut().map(String::as_mut_str)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(String::as_mut_str)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:24:13
|
LL | let _ = opt.as_mut().map(String::as_mut_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`

error: called `.as_mut().map(|x| x.as_mut_str())` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(|x| x.as_mut_str())` on an `Option` value
--> $DIR/option_as_ref_deref.rs:25:13
|
LL | let _ = opt.as_mut().map(|x| x.as_mut_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`

error: called `.as_ref().map(CString::as_c_str)` on an Option value. This can be done more directly by calling `Some(CString::new(vec![]).unwrap()).as_deref()` instead
error: called `.as_ref().map(CString::as_c_str)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:26:13
|
LL | let _ = Some(CString::new(vec![]).unwrap()).as_ref().map(CString::as_c_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(CString::new(vec![]).unwrap()).as_deref()`

error: called `.as_ref().map(OsString::as_os_str)` on an Option value. This can be done more directly by calling `Some(OsString::new()).as_deref()` instead
error: called `.as_ref().map(OsString::as_os_str)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:27:13
|
LL | let _ = Some(OsString::new()).as_ref().map(OsString::as_os_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(OsString::new()).as_deref()`

error: called `.as_ref().map(PathBuf::as_path)` on an Option value. This can be done more directly by calling `Some(PathBuf::new()).as_deref()` instead
error: called `.as_ref().map(PathBuf::as_path)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:28:13
|
LL | let _ = Some(PathBuf::new()).as_ref().map(PathBuf::as_path);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(PathBuf::new()).as_deref()`

error: called `.as_ref().map(Vec::as_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref()` instead
error: called `.as_ref().map(Vec::as_slice)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:29:13
|
LL | let _ = Some(Vec::<()>::new()).as_ref().map(Vec::as_slice);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(Vec::<()>::new()).as_deref()`

error: called `.as_mut().map(Vec::as_mut_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref_mut()` instead
error: called `.as_mut().map(Vec::as_mut_slice)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:30:13
|
LL | let _ = Some(Vec::<()>::new()).as_mut().map(Vec::as_mut_slice);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `Some(Vec::<()>::new()).as_deref_mut()`

error: called `.as_ref().map(|x| x.deref())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(|x| x.deref())` on an `Option` value
--> $DIR/option_as_ref_deref.rs:32:13
|
LL | let _ = opt.as_ref().map(|x| x.deref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`

error: called `.as_mut().map(|x| x.deref_mut())` on an Option value. This can be done more directly by calling `opt.clone().as_deref_mut()` instead
error: called `.as_mut().map(|x| x.deref_mut())` on an `Option` value
--> $DIR/option_as_ref_deref.rs:33:13
|
LL | let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()`

error: called `.as_ref().map(|x| &**x)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(|x| &**x)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:40:13
|
LL | let _ = opt.as_ref().map(|x| &**x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`

error: called `.as_mut().map(|x| &mut **x)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(|x| &mut **x)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:41:13
|
LL | let _ = opt.as_mut().map(|x| &mut **x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`

error: called `.as_ref().map(std::ops::Deref::deref)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(std::ops::Deref::deref)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:44:13
|
LL | let _ = opt.as_ref().map(std::ops::Deref::deref);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`

error: called `.as_ref().map(String::as_str)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(String::as_str)` on an `Option` value
--> $DIR/option_as_ref_deref.rs:56:13
|
LL | let _ = opt.as_ref().map(String::as_str);
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/option_map_or_none.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
error: called `map_or(None, ..)` on an `Option` value
--> $DIR/option_map_or_none.rs:10:26
|
LL | let _: Option<i32> = opt.map_or(None, |x| Some(x + 1));
Expand All @@ -7,7 +7,7 @@ LL | let _: Option<i32> = opt.map_or(None, |x| Some(x + 1));
= note: `-D clippy::option-map-or-none` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::option_map_or_none)]`

error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
error: called `map_or(None, ..)` on an `Option` value
--> $DIR/option_map_or_none.rs:13:26
|
LL | let _: Option<i32> = opt.map_or(None, |x| {
Expand All @@ -16,13 +16,13 @@ LL | | Some(x + 1)
LL | | });
| |_________________________^ help: try using `map` instead: `opt.map(|x| x + 1)`

error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
error: called `map_or(None, ..)` on an `Option` value
--> $DIR/option_map_or_none.rs:17:26
|
LL | let _: Option<i32> = opt.map_or(None, bar);
| ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `opt.and_then(bar)`

error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
error: called `map_or(None, ..)` on an `Option` value
--> $DIR/option_map_or_none.rs:18:26
|
LL | let _: Option<i32> = opt.map_or(None, |x| {
Expand All @@ -42,7 +42,7 @@ LL + Some(offset + height)
LL ~ });
|

error: called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling `ok()` instead
error: called `map_or(None, Some)` on a `Result` value
--> $DIR/option_map_or_none.rs:25:26
|
LL | let _: Option<i32> = r.map_or(None, Some);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/result_map_or_into_option.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
fn main() {
let opt: Result<u32, &str> = Ok(1);
let _ = opt.ok();
//~^ ERROR: called `map_or(None, Some)` on a `Result` value.
//~^ ERROR: called `map_or(None, Some)` on a `Result` value
let _ = opt.ok();
//~^ ERROR: called `map_or_else(|_| None, Some)` on a `Result` value
#[rustfmt::skip]
Expand Down

0 comments on commit c3243f9

Please sign in to comment.