Skip to content

Commit 6ed6f1e

Browse files
committed
Auto merge of #6826 - TaKO8Ki:refactor-methods-mod, r=phansch
Refactor: arrange lints in `methods` module This PR arranges methods lints so that they can be accessed more easily. Basically, I refactored them following the instruction described in #6680. changelog: Move lints in methods module into their own modules.
2 parents 534a13d + 83a9553 commit 6ed6f1e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+2502
-2143
lines changed

clippy_lints/src/methods/bind_instead_of_map.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ pub(crate) trait BindInsteadOfMap {
158158
}
159159

160160
/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
161-
fn lint(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
161+
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
162162
if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) {
163163
return false;
164164
}

clippy_lints/src/methods/bytes_nth.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_span::sym;
77

88
use super::BYTES_NTH;
99

10-
pub(super) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
10+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
1111
if_chain! {
1212
if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
1313
let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs();
+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
use crate::utils::{is_copy, span_lint_and_then, sugg};
2+
use rustc_errors::Applicability;
3+
use rustc_hir as hir;
4+
use rustc_lint::LateContext;
5+
use rustc_middle::ty::{self, Ty};
6+
use std::iter;
7+
8+
use super::CLONE_DOUBLE_REF;
9+
use super::CLONE_ON_COPY;
10+
11+
/// Checks for the `CLONE_ON_COPY` lint.
12+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'_>) {
13+
let ty = cx.typeck_results().expr_ty(expr);
14+
if let ty::Ref(_, inner, _) = arg_ty.kind() {
15+
if let ty::Ref(_, innermost, _) = inner.kind() {
16+
span_lint_and_then(
17+
cx,
18+
CLONE_DOUBLE_REF,
19+
expr.span,
20+
&format!(
21+
"using `clone` on a double-reference; \
22+
this will copy the reference of type `{}` instead of cloning the inner type",
23+
ty
24+
),
25+
|diag| {
26+
if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) {
27+
let mut ty = innermost;
28+
let mut n = 0;
29+
while let ty::Ref(_, inner, _) = ty.kind() {
30+
ty = inner;
31+
n += 1;
32+
}
33+
let refs: String = iter::repeat('&').take(n + 1).collect();
34+
let derefs: String = iter::repeat('*').take(n).collect();
35+
let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
36+
diag.span_suggestion(
37+
expr.span,
38+
"try dereferencing it",
39+
format!("{}({}{}).clone()", refs, derefs, snip.deref()),
40+
Applicability::MaybeIncorrect,
41+
);
42+
diag.span_suggestion(
43+
expr.span,
44+
"or try being explicit if you are sure, that you want to clone a reference",
45+
explicit,
46+
Applicability::MaybeIncorrect,
47+
);
48+
}
49+
},
50+
);
51+
return; // don't report clone_on_copy
52+
}
53+
}
54+
55+
if is_copy(cx, ty) {
56+
let snip;
57+
if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) {
58+
let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
59+
match &cx.tcx.hir().get(parent) {
60+
hir::Node::Expr(parent) => match parent.kind {
61+
// &*x is a nop, &x.clone() is not
62+
hir::ExprKind::AddrOf(..) => return,
63+
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
64+
hir::ExprKind::MethodCall(_, _, parent_args, _) if expr.hir_id == parent_args[0].hir_id => {
65+
return;
66+
},
67+
68+
_ => {},
69+
},
70+
hir::Node::Stmt(stmt) => {
71+
if let hir::StmtKind::Local(ref loc) = stmt.kind {
72+
if let hir::PatKind::Ref(..) = loc.pat.kind {
73+
// let ref y = *x borrows x, let ref y = x.clone() does not
74+
return;
75+
}
76+
}
77+
},
78+
_ => {},
79+
}
80+
81+
// x.clone() might have dereferenced x, possibly through Deref impls
82+
if cx.typeck_results().expr_ty(arg) == ty {
83+
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
84+
} else {
85+
let deref_count = cx
86+
.typeck_results()
87+
.expr_adjustments(arg)
88+
.iter()
89+
.filter(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_)))
90+
.count();
91+
let derefs: String = iter::repeat('*').take(deref_count).collect();
92+
snip = Some(("try dereferencing it", format!("{}{}", derefs, snippet)));
93+
}
94+
} else {
95+
snip = None;
96+
}
97+
span_lint_and_then(
98+
cx,
99+
CLONE_ON_COPY,
100+
expr.span,
101+
&format!("using `clone` on type `{}` which implements the `Copy` trait", ty),
102+
|diag| {
103+
if let Some((text, snip)) = snip {
104+
diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable);
105+
}
106+
},
107+
);
108+
}
109+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
use crate::utils::{is_type_diagnostic_item, match_type, paths, snippet_with_macro_callsite, span_lint_and_sugg};
2+
use rustc_errors::Applicability;
3+
use rustc_hir as hir;
4+
use rustc_lint::LateContext;
5+
use rustc_middle::ty;
6+
use rustc_span::symbol::sym;
7+
8+
use super::CLONE_ON_REF_PTR;
9+
10+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
11+
let obj_ty = cx.typeck_results().expr_ty(arg).peel_refs();
12+
13+
if let ty::Adt(_, subst) = obj_ty.kind() {
14+
let caller_type = if is_type_diagnostic_item(cx, obj_ty, sym::Rc) {
15+
"Rc"
16+
} else if is_type_diagnostic_item(cx, obj_ty, sym::Arc) {
17+
"Arc"
18+
} else if match_type(cx, obj_ty, &paths::WEAK_RC) || match_type(cx, obj_ty, &paths::WEAK_ARC) {
19+
"Weak"
20+
} else {
21+
return;
22+
};
23+
24+
let snippet = snippet_with_macro_callsite(cx, arg.span, "..");
25+
26+
span_lint_and_sugg(
27+
cx,
28+
CLONE_ON_REF_PTR,
29+
expr.span,
30+
"using `.clone()` on a ref-counted pointer",
31+
"try this",
32+
format!("{}::<{}>::clone(&{})", caller_type, subst.type_at(0), snippet),
33+
Applicability::Unspecified, // Sometimes unnecessary ::<_> after Rc/Arc/Weak
34+
);
35+
}
36+
}
+199
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
use crate::utils::{is_expn_of, is_type_diagnostic_item, snippet, snippet_with_applicability, span_lint_and_sugg};
2+
use if_chain::if_chain;
3+
use rustc_errors::Applicability;
4+
use rustc_hir as hir;
5+
use rustc_lint::LateContext;
6+
use rustc_middle::ty;
7+
use rustc_span::source_map::Span;
8+
use rustc_span::symbol::sym;
9+
use std::borrow::Cow;
10+
11+
use super::EXPECT_FUN_CALL;
12+
13+
/// Checks for the `EXPECT_FUN_CALL` lint.
14+
#[allow(clippy::too_many_lines)]
15+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Span, name: &str, args: &[hir::Expr<'_>]) {
16+
// Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or
17+
// `&str`
18+
fn get_arg_root<'a>(cx: &LateContext<'_>, arg: &'a hir::Expr<'a>) -> &'a hir::Expr<'a> {
19+
let mut arg_root = arg;
20+
loop {
21+
arg_root = match &arg_root.kind {
22+
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => expr,
23+
hir::ExprKind::MethodCall(method_name, _, call_args, _) => {
24+
if call_args.len() == 1
25+
&& (method_name.ident.name == sym::as_str || method_name.ident.name == sym!(as_ref))
26+
&& {
27+
let arg_type = cx.typeck_results().expr_ty(&call_args[0]);
28+
let base_type = arg_type.peel_refs();
29+
*base_type.kind() == ty::Str || is_type_diagnostic_item(cx, base_type, sym::string_type)
30+
}
31+
{
32+
&call_args[0]
33+
} else {
34+
break;
35+
}
36+
},
37+
_ => break,
38+
};
39+
}
40+
arg_root
41+
}
42+
43+
// Only `&'static str` or `String` can be used directly in the `panic!`. Other types should be
44+
// converted to string.
45+
fn requires_to_string(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
46+
let arg_ty = cx.typeck_results().expr_ty(arg);
47+
if is_type_diagnostic_item(cx, arg_ty, sym::string_type) {
48+
return false;
49+
}
50+
if let ty::Ref(_, ty, ..) = arg_ty.kind() {
51+
if *ty.kind() == ty::Str && can_be_static_str(cx, arg) {
52+
return false;
53+
}
54+
};
55+
true
56+
}
57+
58+
// Check if an expression could have type `&'static str`, knowing that it
59+
// has type `&str` for some lifetime.
60+
fn can_be_static_str(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
61+
match arg.kind {
62+
hir::ExprKind::Lit(_) => true,
63+
hir::ExprKind::Call(fun, _) => {
64+
if let hir::ExprKind::Path(ref p) = fun.kind {
65+
match cx.qpath_res(p, fun.hir_id) {
66+
hir::def::Res::Def(hir::def::DefKind::Fn | hir::def::DefKind::AssocFn, def_id) => matches!(
67+
cx.tcx.fn_sig(def_id).output().skip_binder().kind(),
68+
ty::Ref(ty::ReStatic, ..)
69+
),
70+
_ => false,
71+
}
72+
} else {
73+
false
74+
}
75+
},
76+
hir::ExprKind::MethodCall(..) => {
77+
cx.typeck_results()
78+
.type_dependent_def_id(arg.hir_id)
79+
.map_or(false, |method_id| {
80+
matches!(
81+
cx.tcx.fn_sig(method_id).output().skip_binder().kind(),
82+
ty::Ref(ty::ReStatic, ..)
83+
)
84+
})
85+
},
86+
hir::ExprKind::Path(ref p) => matches!(
87+
cx.qpath_res(p, arg.hir_id),
88+
hir::def::Res::Def(hir::def::DefKind::Const | hir::def::DefKind::Static, _)
89+
),
90+
_ => false,
91+
}
92+
}
93+
94+
fn generate_format_arg_snippet(
95+
cx: &LateContext<'_>,
96+
a: &hir::Expr<'_>,
97+
applicability: &mut Applicability,
98+
) -> Vec<String> {
99+
if_chain! {
100+
if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, ref format_arg) = a.kind;
101+
if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.kind;
102+
if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.kind;
103+
104+
then {
105+
format_arg_expr_tup
106+
.iter()
107+
.map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
108+
.collect()
109+
} else {
110+
unreachable!()
111+
}
112+
}
113+
}
114+
115+
fn is_call(node: &hir::ExprKind<'_>) -> bool {
116+
match node {
117+
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => {
118+
is_call(&expr.kind)
119+
},
120+
hir::ExprKind::Call(..)
121+
| hir::ExprKind::MethodCall(..)
122+
// These variants are debatable or require further examination
123+
| hir::ExprKind::If(..)
124+
| hir::ExprKind::Match(..)
125+
| hir::ExprKind::Block{ .. } => true,
126+
_ => false,
127+
}
128+
}
129+
130+
if args.len() != 2 || name != "expect" || !is_call(&args[1].kind) {
131+
return;
132+
}
133+
134+
let receiver_type = cx.typeck_results().expr_ty_adjusted(&args[0]);
135+
let closure_args = if is_type_diagnostic_item(cx, receiver_type, sym::option_type) {
136+
"||"
137+
} else if is_type_diagnostic_item(cx, receiver_type, sym::result_type) {
138+
"|_|"
139+
} else {
140+
return;
141+
};
142+
143+
let arg_root = get_arg_root(cx, &args[1]);
144+
145+
let span_replace_word = method_span.with_hi(expr.span.hi());
146+
147+
let mut applicability = Applicability::MachineApplicable;
148+
149+
//Special handling for `format!` as arg_root
150+
if_chain! {
151+
if let hir::ExprKind::Block(block, None) = &arg_root.kind;
152+
if block.stmts.len() == 1;
153+
if let hir::StmtKind::Local(local) = &block.stmts[0].kind;
154+
if let Some(arg_root) = &local.init;
155+
if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = arg_root.kind;
156+
if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1;
157+
if let hir::ExprKind::Call(_, format_args) = &inner_args[0].kind;
158+
then {
159+
let fmt_spec = &format_args[0];
160+
let fmt_args = &format_args[1];
161+
162+
let mut args = vec![snippet(cx, fmt_spec.span, "..").into_owned()];
163+
164+
args.extend(generate_format_arg_snippet(cx, fmt_args, &mut applicability));
165+
166+
let sugg = args.join(", ");
167+
168+
span_lint_and_sugg(
169+
cx,
170+
EXPECT_FUN_CALL,
171+
span_replace_word,
172+
&format!("use of `{}` followed by a function call", name),
173+
"try this",
174+
format!("unwrap_or_else({} panic!({}))", closure_args, sugg),
175+
applicability,
176+
);
177+
178+
return;
179+
}
180+
}
181+
182+
let mut arg_root_snippet: Cow<'_, _> = snippet_with_applicability(cx, arg_root.span, "..", &mut applicability);
183+
if requires_to_string(cx, arg_root) {
184+
arg_root_snippet.to_mut().push_str(".to_string()");
185+
}
186+
187+
span_lint_and_sugg(
188+
cx,
189+
EXPECT_FUN_CALL,
190+
span_replace_word,
191+
&format!("use of `{}` followed by a function call", name),
192+
"try this",
193+
format!(
194+
"unwrap_or_else({} {{ panic!(\"{{}}\", {}) }})",
195+
closure_args, arg_root_snippet
196+
),
197+
applicability,
198+
);
199+
}
+30
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+
}

0 commit comments

Comments
 (0)