Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up span_lint in methods/mod.rs #5084

Merged
merged 3 commits into from
Jan 26, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 52 additions & 52 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2133,14 +2133,12 @@ fn lint_iter_nth<'a, 'tcx>(
return; // caller is not a type that we want to lint
};

span_lint(
span_help_and_lint(
cx,
ITER_NTH,
expr.span,
&format!(
"called `.iter{0}().nth()` on a {1}. Calling `.get{0}()` is both faster and more readable",
mut_str, caller_type
),
&format!("called `.iter{0}().nth()` on a {1}", mut_str, caller_type),
&format!("Calling `.get{}()` is both faster and more readable", mut_str),
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
);
}

Expand Down Expand Up @@ -2244,11 +2242,12 @@ fn lint_get_unwrap<'a, 'tcx>(
fn lint_iter_skip_next(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) {
// lint if caller of skip is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
span_lint(
span_help_and_lint(
cx,
ITER_SKIP_NEXT,
expr.span,
"called `skip(x).next()` on an iterator. This is more succinctly expressed by calling `nth(x)`",
"called `skip(x).next()` on an iterator",
"This is more succinctly expressed by calling `nth(x)`.",
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
Expand Down Expand Up @@ -2304,15 +2303,15 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, unwrap_args: &[hi
};

if let Some((lint, kind, none_value)) = mess {
span_lint(
span_help_and_lint(
cx,
lint,
expr.span,
&format!("used `unwrap()` on `{}` value", kind,),
&format!(
"used `unwrap()` on `{}` value. If you don't want to handle the `{}` case gracefully, consider \
using `expect()` to provide a better panic \
message",
kind, none_value
"If you don't want to handle the `{}` case gracefully, consider \
using `expect()` to provide a better panic message.",
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
none_value,
),
);
}
Expand All @@ -2331,14 +2330,12 @@ fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, expect_args: &[hi
};

if let Some((lint, kind, none_value)) = mess {
span_lint(
span_help_and_lint(
cx,
lint,
expr.span,
&format!(
"used `expect()` on `{}` value. If this value is an `{}` it will panic",
kind, none_value
),
&format!("used `expect()` on `{}` value", kind,),
&format!("If this value is an `{}`, it will panic.", none_value,),
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
Expand All @@ -2353,11 +2350,12 @@ fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, ok_args: &[hir
if has_debug_impl(error_type, cx);

then {
span_lint(
span_help_and_lint(
cx,
OK_EXPECT,
expr.span,
"called `ok().expect()` on a `Result` value. You can call `expect()` directly on the `Result`",
"called `ok().expect()` on a `Result` value",
"You can call `expect()` directly on the `Result`",
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
Expand All @@ -2372,14 +2370,15 @@ fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<
let self_snippet = snippet(cx, map_args[0].span, "..");
let func_snippet = snippet(cx, map_args[1].span, "..");
let hint = format!("{0}.flat_map({1})", self_snippet, func_snippet);
span_lint_and_then(cx, MAP_FLATTEN, expr.span, msg, |db| {
db.span_suggestion(
expr.span,
"try using `flat_map` instead",
hint,
Applicability::MachineApplicable,
);
});
span_lint_and_sugg(
cx,
MAP_FLATTEN,
expr.span,
msg,
"try using `flat_map` instead",
hint,
Applicability::MachineApplicable,
);
}
}

Expand Down Expand Up @@ -2474,14 +2473,15 @@ fn lint_map_or_none<'a, 'tcx>(
let map_or_self_snippet = snippet(cx, map_or_args[0].span, "..");
let map_or_func_snippet = snippet(cx, map_or_args[2].span, "..");
let hint = format!("{0}.and_then({1})", map_or_self_snippet, map_or_func_snippet);
span_lint_and_then(cx, OPTION_MAP_OR_NONE, expr.span, msg, |db| {
db.span_suggestion(
expr.span,
"try using `and_then` instead",
hint,
Applicability::MachineApplicable, // snippet
);
});
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
span_lint_and_sugg(
cx,
OPTION_MAP_OR_NONE,
expr.span,
msg,
"try using `and_then` instead",
hint,
Applicability::MachineApplicable,
);
}
}
}
Expand Down Expand Up @@ -2607,9 +2607,9 @@ fn lint_filter_map<'a, 'tcx>(
) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(p).map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.filter_map(..)` instead.";
span_lint(cx, FILTER_MAP, expr.span, msg);
let msg = "called `filter(p).map(q)` on an `Iterator`";
let hint = "This is more succinctly expressed by calling `.filter_map(..)` instead.";
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
}
}

Expand Down Expand Up @@ -2647,9 +2647,9 @@ fn lint_find_map<'a, 'tcx>(
) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
let msg = "called `find(p).map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.find_map(..)` instead.";
span_lint(cx, FIND_MAP, expr.span, msg);
let msg = "called `find(p).map(q)` on an `Iterator`";
let hint = "This is more succinctly expressed by calling `.find_map(..)` instead.";
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
span_help_and_lint(cx, FIND_MAP, expr.span, msg, hint);
}
}

Expand All @@ -2662,9 +2662,9 @@ fn lint_filter_map_map<'a, 'tcx>(
) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(p).map(q)` on an `Iterator`. \
This is more succinctly expressed by only calling `.filter_map(..)` instead.";
span_lint(cx, FILTER_MAP, expr.span, msg);
let msg = "called `filter_map(p).map(q)` on an `Iterator`";
let hint = "This is more succinctly expressed by only calling `.filter_map(..)` instead.";
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
}
}

Expand All @@ -2677,10 +2677,10 @@ fn lint_filter_flat_map<'a, 'tcx>(
) {
// lint if caller of `.filter().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(p).flat_map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
span_lint(cx, FILTER_MAP, expr.span, msg);
let msg = "called `filter(p).flat_map(q)` on an `Iterator`";
let hint = "This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
}
}

Expand All @@ -2693,10 +2693,10 @@ fn lint_filter_map_flat_map<'a, 'tcx>(
) {
// lint if caller of `.filter_map().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
span_lint(cx, FILTER_MAP, expr.span, msg);
let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`";
let hint = "This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
JohnTitor marked this conversation as resolved.
Show resolved Hide resolved
span_help_and_lint(cx, FILTER_MAP, expr.span, msg, hint);
}
}

Expand Down
6 changes: 4 additions & 2 deletions tests/ui/expect.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
error: used `expect()` on `an Option` value. If this value is an `None` it will panic
error: used `expect()` on `an Option` value
--> $DIR/expect.rs:5:13
|
LL | let _ = opt.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::option-expect-used` implied by `-D warnings`
= help: If this value is an `None`, it will panic.

error: used `expect()` on `a Result` value. If this value is an `Err` it will panic
error: used `expect()` on `a Result` value
--> $DIR/expect.rs:10:13
|
LL | let _ = res.expect("");
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::result-expect-used` implied by `-D warnings`
= help: If this value is an `Err`, it will panic.

error: aborting due to 2 previous errors

15 changes: 11 additions & 4 deletions tests/ui/filter_methods.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
error: called `filter(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.filter_map(..)` instead.
error: called `filter(p).map(q)` on an `Iterator`
--> $DIR/filter_methods.rs:5:21
|
LL | let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::filter-map` implied by `-D warnings`
= help: This is more succinctly expressed by calling `.filter_map(..)` instead.

error: called `filter(p).flat_map(q)` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.
error: called `filter(p).flat_map(q)` on an `Iterator`
--> $DIR/filter_methods.rs:7:21
|
LL | let _: Vec<_> = vec![5_i8; 6]
Expand All @@ -15,8 +16,10 @@ LL | | .into_iter()
LL | | .filter(|&x| x == 0)
LL | | .flat_map(|x| x.checked_mul(2))
| |_______________________________________^
|
= help: This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.

error: called `filter_map(p).flat_map(q)` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.
error: called `filter_map(p).flat_map(q)` on an `Iterator`
--> $DIR/filter_methods.rs:13:21
|
LL | let _: Vec<_> = vec![5_i8; 6]
Expand All @@ -25,8 +28,10 @@ LL | | .into_iter()
LL | | .filter_map(|x| x.checked_mul(2))
LL | | .flat_map(|x| x.checked_mul(2))
| |_______________________________________^
|
= help: This is more succinctly expressed by calling `.flat_map(..)` and filtering by returning an empty Iterator.

error: called `filter_map(p).map(q)` on an `Iterator`. This is more succinctly expressed by only calling `.filter_map(..)` instead.
error: called `filter_map(p).map(q)` on an `Iterator`
--> $DIR/filter_methods.rs:19:21
|
LL | let _: Vec<_> = vec![5_i8; 6]
Expand All @@ -35,6 +40,8 @@ LL | | .into_iter()
LL | | .filter_map(|x| x.checked_mul(2))
LL | | .map(|x| x.checked_mul(2))
| |__________________________________^
|
= help: This is more succinctly expressed by only calling `.filter_map(..)` instead.

error: aborting due to 4 previous errors

7 changes: 5 additions & 2 deletions tests/ui/find_map.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
error: called `find(p).map(q)` on an `Iterator`
--> $DIR/find_map.rs:20:26
|
LL | let _: Option<i32> = a.iter().find(|s| s.parse::<i32>().is_ok()).map(|s| s.parse().unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::find-map` implied by `-D warnings`
= help: This is more succinctly expressed by calling `.find_map(..)` instead.

error: called `find(p).map(q)` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead.
error: called `find(p).map(q)` on an `Iterator`
--> $DIR/find_map.rs:22:29
|
LL | let _: Option<Flavor> = desserts_of_the_week
Expand All @@ -18,6 +19,8 @@ LL | | Dessert::Cake(_) => true,
LL | | _ => unreachable!(),
LL | | });
| |__________^
|
= help: This is more succinctly expressed by calling `.find_map(..)` instead.

error: aborting due to 2 previous errors

27 changes: 20 additions & 7 deletions tests/ui/iter_nth.stderr
Original file line number Diff line number Diff line change
@@ -1,46 +1,59 @@
error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable
error: called `.iter().nth()` on a Vec
--> $DIR/iter_nth.rs:33:23
|
LL | let bad_vec = some_vec.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::iter-nth` implied by `-D warnings`
= help: Calling `.get()` is both faster and more readable

error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
error: called `.iter().nth()` on a slice
--> $DIR/iter_nth.rs:34:26
|
LL | let bad_slice = &some_vec[..].iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get()` is both faster and more readable

error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
error: called `.iter().nth()` on a slice
--> $DIR/iter_nth.rs:35:31
|
LL | let bad_boxed_slice = boxed_slice.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get()` is both faster and more readable

error: called `.iter().nth()` on a VecDeque. Calling `.get()` is both faster and more readable
error: called `.iter().nth()` on a VecDeque
--> $DIR/iter_nth.rs:36:29
|
LL | let bad_vec_deque = some_vec_deque.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get()` is both faster and more readable

error: called `.iter_mut().nth()` on a Vec. Calling `.get_mut()` is both faster and more readable
error: called `.iter_mut().nth()` on a Vec
--> $DIR/iter_nth.rs:41:23
|
LL | let bad_vec = some_vec.iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get_mut()` is both faster and more readable

error: called `.iter_mut().nth()` on a slice. Calling `.get_mut()` is both faster and more readable
error: called `.iter_mut().nth()` on a slice
--> $DIR/iter_nth.rs:44:26
|
LL | let bad_slice = &some_vec[..].iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get_mut()` is both faster and more readable

error: called `.iter_mut().nth()` on a VecDeque. Calling `.get_mut()` is both faster and more readable
error: called `.iter_mut().nth()` on a VecDeque
--> $DIR/iter_nth.rs:47:29
|
LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Calling `.get_mut()` is both faster and more readable

error: aborting due to 7 previous errors

Loading