Skip to content

Commit f0f07ac

Browse files
committed
move expect_used, filter_next, get_unwrap, ok_expect and unwrap_used to their own modules
1 parent 99afc6e commit f0f07ac

File tree

8 files changed

+244
-202
lines changed

8 files changed

+244
-202
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
use crate::utils::{is_type_diagnostic_item, span_lint_and_help};
2+
use rustc_hir as hir;
3+
use rustc_lint::LateContext;
4+
use rustc_span::sym;
5+
6+
use super::EXPECT_USED;
7+
8+
/// lint use of `expect()` for `Option`s and `Result`s
9+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
10+
let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();
11+
12+
let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
13+
Some((EXPECT_USED, "an Option", "None"))
14+
} else if is_type_diagnostic_item(cx, obj_ty, sym::result_type) {
15+
Some((EXPECT_USED, "a Result", "Err"))
16+
} else {
17+
None
18+
};
19+
20+
if let Some((lint, kind, none_value)) = mess {
21+
span_lint_and_help(
22+
cx,
23+
lint,
24+
expr.span,
25+
&format!("used `expect()` on `{}` value", kind,),
26+
None,
27+
&format!("if this value is an `{}`, it will panic", none_value,),
28+
);
29+
}
30+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
use crate::utils::{match_trait_method, paths, snippet, span_lint, span_lint_and_sugg};
2+
use rustc_errors::Applicability;
3+
use rustc_hir as hir;
4+
use rustc_lint::LateContext;
5+
6+
use super::FILTER_NEXT;
7+
8+
/// lint use of `filter().next()` for `Iterators`
9+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
10+
// lint if caller of `.filter().next()` is an Iterator
11+
if match_trait_method(cx, expr, &paths::ITERATOR) {
12+
let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
13+
`.find(..)` instead";
14+
let filter_snippet = snippet(cx, filter_args[1].span, "..");
15+
if filter_snippet.lines().count() <= 1 {
16+
let iter_snippet = snippet(cx, filter_args[0].span, "..");
17+
// add note if not multi-line
18+
span_lint_and_sugg(
19+
cx,
20+
FILTER_NEXT,
21+
expr.span,
22+
msg,
23+
"try this",
24+
format!("{}.find({})", iter_snippet, filter_snippet),
25+
Applicability::MachineApplicable,
26+
);
27+
} else {
28+
span_lint(cx, FILTER_NEXT, expr.span, msg);
29+
}
30+
}
31+
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
use crate::methods::derefs_to_slice;
2+
use crate::utils::{
3+
get_parent_expr, is_type_diagnostic_item, match_type, paths, snippet_with_applicability, span_lint_and_sugg,
4+
};
5+
use if_chain::if_chain;
6+
use rustc_errors::Applicability;
7+
use rustc_hir as hir;
8+
use rustc_lint::LateContext;
9+
use rustc_span::sym;
10+
11+
use super::GET_UNWRAP;
12+
13+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args: &'tcx [hir::Expr<'_>], is_mut: bool) {
14+
// Note: we don't want to lint `get_mut().unwrap` for `HashMap` or `BTreeMap`,
15+
// because they do not implement `IndexMut`
16+
let mut applicability = Applicability::MachineApplicable;
17+
let expr_ty = cx.typeck_results().expr_ty(&get_args[0]);
18+
let get_args_str = if get_args.len() > 1 {
19+
snippet_with_applicability(cx, get_args[1].span, "..", &mut applicability)
20+
} else {
21+
return; // not linting on a .get().unwrap() chain or variant
22+
};
23+
let mut needs_ref;
24+
let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
25+
needs_ref = get_args_str.parse::<usize>().is_ok();
26+
"slice"
27+
} else if is_type_diagnostic_item(cx, expr_ty, sym::vec_type) {
28+
needs_ref = get_args_str.parse::<usize>().is_ok();
29+
"Vec"
30+
} else if is_type_diagnostic_item(cx, expr_ty, sym!(vecdeque_type)) {
31+
needs_ref = get_args_str.parse::<usize>().is_ok();
32+
"VecDeque"
33+
} else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym!(hashmap_type)) {
34+
needs_ref = true;
35+
"HashMap"
36+
} else if !is_mut && match_type(cx, expr_ty, &paths::BTREEMAP) {
37+
needs_ref = true;
38+
"BTreeMap"
39+
} else {
40+
return; // caller is not a type that we want to lint
41+
};
42+
43+
let mut span = expr.span;
44+
45+
// Handle the case where the result is immediately dereferenced
46+
// by not requiring ref and pulling the dereference into the
47+
// suggestion.
48+
if_chain! {
49+
if needs_ref;
50+
if let Some(parent) = get_parent_expr(cx, expr);
51+
if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind;
52+
then {
53+
needs_ref = false;
54+
span = parent.span;
55+
}
56+
}
57+
58+
let mut_str = if is_mut { "_mut" } else { "" };
59+
let borrow_str = if !needs_ref {
60+
""
61+
} else if is_mut {
62+
"&mut "
63+
} else {
64+
"&"
65+
};
66+
67+
span_lint_and_sugg(
68+
cx,
69+
GET_UNWRAP,
70+
span,
71+
&format!(
72+
"called `.get{0}().unwrap()` on a {1}. Using `[]` is more clear and more concise",
73+
mut_str, caller_type
74+
),
75+
"try this",
76+
format!(
77+
"{}{}[{}]",
78+
borrow_str,
79+
snippet_with_applicability(cx, get_args[0].span, "..", &mut applicability),
80+
get_args_str
81+
),
82+
applicability,
83+
);
84+
}

0 commit comments

Comments
 (0)