Skip to content

Commit 5317b7b

Browse files
ada4aada4a
committed
clean-up
1 parent 3e0590c commit 5317b7b

File tree

5 files changed

+223
-227
lines changed

5 files changed

+223
-227
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4460,7 +4460,7 @@ declare_clippy_lint! {
44604460
/// Checks for calls to `Read::bytes` on types which don't implement `BufRead`.
44614461
///
44624462
/// ### Why is this bad?
4463-
/// The default implementation calls `read` for each byte, which can be very inefficient for data that’s not in memory, such as `File`.
4463+
/// The default implementation calls `read` for each byte, which can be very inefficient for data that's not in memory, such as `File`.
44644464
///
44654465
/// ### Example
44664466
/// ```no_run

‎clippy_lints/src/methods/or_fun_call.rs‎

Lines changed: 177 additions & 179 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use {rustc_ast as ast, rustc_hir as hir};
1818
use super::{OR_FUN_CALL, UNWRAP_OR_DEFAULT};
1919

2020
/// Checks for the `OR_FUN_CALL` lint.
21-
#[expect(clippy::too_many_lines)]
2221
pub(super) fn check<'tcx>(
2322
cx: &LateContext<'tcx>,
2423
expr: &hir::Expr<'_>,
@@ -27,184 +26,6 @@ pub(super) fn check<'tcx>(
2726
receiver: &'tcx hir::Expr<'_>,
2827
args: &'tcx [hir::Expr<'_>],
2928
) {
30-
/// Checks for `unwrap_or(T::new())`, `unwrap_or(T::default())`,
31-
/// `or_insert(T::new())` or `or_insert(T::default())`.
32-
/// Similarly checks for `unwrap_or_else(T::new)`, `unwrap_or_else(T::default)`,
33-
/// `or_insert_with(T::new)` or `or_insert_with(T::default)`.
34-
fn check_unwrap_or_default(
35-
cx: &LateContext<'_>,
36-
name: Symbol,
37-
receiver: &hir::Expr<'_>,
38-
fun: &hir::Expr<'_>,
39-
call_expr: Option<&hir::Expr<'_>>,
40-
span: Span,
41-
method_span: Span,
42-
) -> bool {
43-
if !expr_type_is_certain(cx, receiver) {
44-
return false;
45-
}
46-
47-
let is_new = |fun: &hir::Expr<'_>| {
48-
if let hir::ExprKind::Path(ref qpath) = fun.kind {
49-
let path = last_path_segment(qpath).ident.name;
50-
matches!(path, sym::new)
51-
} else {
52-
false
53-
}
54-
};
55-
56-
let output_type_implements_default = |fun| {
57-
let fun_ty = cx.typeck_results().expr_ty(fun);
58-
if let ty::FnDef(def_id, args) = fun_ty.kind() {
59-
let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, args).skip_binder().output();
60-
cx.tcx
61-
.get_diagnostic_item(sym::Default)
62-
.is_some_and(|default_trait_id| implements_trait(cx, output_ty, default_trait_id, &[]))
63-
} else {
64-
false
65-
}
66-
};
67-
68-
let sugg = match (name, call_expr.is_some()) {
69-
(sym::unwrap_or, true) | (sym::unwrap_or_else, false) => sym::unwrap_or_default,
70-
(sym::or_insert, true) | (sym::or_insert_with, false) => sym::or_default,
71-
_ => return false,
72-
};
73-
74-
let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs();
75-
let Some(suggested_method_def_id) = receiver_ty.ty_adt_def().and_then(|adt_def| {
76-
cx.tcx
77-
.inherent_impls(adt_def.did())
78-
.iter()
79-
.flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg))
80-
.find_map(|assoc| {
81-
if assoc.is_method() && cx.tcx.fn_sig(assoc.def_id).skip_binder().inputs().skip_binder().len() == 1
82-
{
83-
Some(assoc.def_id)
84-
} else {
85-
None
86-
}
87-
})
88-
}) else {
89-
return false;
90-
};
91-
let in_sugg_method_implementation = {
92-
matches!(
93-
suggested_method_def_id.as_local(),
94-
Some(local_def_id) if local_def_id == cx.tcx.hir_get_parent_item(receiver.hir_id).def_id
95-
)
96-
};
97-
if in_sugg_method_implementation {
98-
return false;
99-
}
100-
101-
// `.unwrap_or(vec![])` is as readable as `.unwrap_or_default()`. And if the expression is a
102-
// non-empty `Vec`, then it will not be a default value anyway. Bail out in all cases.
103-
if call_expr.and_then(|call_expr| VecArgs::hir(cx, call_expr)).is_some() {
104-
return false;
105-
}
106-
107-
// needs to target Default::default in particular or be *::new and have a Default impl
108-
// available
109-
if (is_new(fun) && output_type_implements_default(fun))
110-
|| match call_expr {
111-
Some(call_expr) => is_default_equivalent(cx, call_expr),
112-
None => is_default_equivalent_call(cx, fun, None) || closure_body_returns_empty_to_string(cx, fun),
113-
}
114-
{
115-
span_lint_and_sugg(
116-
cx,
117-
UNWRAP_OR_DEFAULT,
118-
method_span.with_hi(span.hi()),
119-
format!("use of `{name}` to construct default value"),
120-
"try",
121-
format!("{sugg}()"),
122-
Applicability::MachineApplicable,
123-
);
124-
125-
true
126-
} else {
127-
false
128-
}
129-
}
130-
131-
/// Checks for `*or(foo())`.
132-
#[expect(clippy::too_many_arguments)]
133-
fn check_or_fn_call<'tcx>(
134-
cx: &LateContext<'tcx>,
135-
name: Symbol,
136-
method_span: Span,
137-
self_expr: &hir::Expr<'_>,
138-
arg: &'tcx hir::Expr<'_>,
139-
// `Some` if fn has second argument
140-
second_arg: Option<&hir::Expr<'_>>,
141-
span: Span,
142-
// None if lambda is required
143-
fun_span: Option<Span>,
144-
) -> bool {
145-
// (path, fn_has_argument, methods, suffix)
146-
const KNOW_TYPES: [(Symbol, bool, &[Symbol], &str); 7] = [
147-
(sym::BTreeEntry, false, &[sym::or_insert], "with"),
148-
(sym::HashMapEntry, false, &[sym::or_insert], "with"),
149-
(
150-
sym::Option,
151-
false,
152-
&[sym::map_or, sym::ok_or, sym::or, sym::unwrap_or],
153-
"else",
154-
),
155-
(sym::Option, false, &[sym::get_or_insert], "with"),
156-
(sym::Option, true, &[sym::and], "then"),
157-
(sym::Result, true, &[sym::map_or, sym::or, sym::unwrap_or], "else"),
158-
(sym::Result, true, &[sym::and], "then"),
159-
];
160-
161-
if KNOW_TYPES.iter().any(|k| k.2.contains(&name))
162-
&& switch_to_lazy_eval(cx, arg)
163-
&& !contains_return(arg)
164-
&& let self_ty = cx.typeck_results().expr_ty(self_expr)
165-
&& let Some(&(_, fn_has_arguments, _, suffix)) = KNOW_TYPES
166-
.iter()
167-
.find(|&&i| is_type_diagnostic_item(cx, self_ty, i.0) && i.2.contains(&name))
168-
{
169-
let ctxt = span.ctxt();
170-
let mut app = Applicability::HasPlaceholders;
171-
let sugg = {
172-
let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
173-
(false, Some(fun_span)) => (fun_span, false),
174-
_ => (arg.span, true),
175-
};
176-
177-
let snip = snippet_with_context(cx, snippet_span, ctxt, "..", &mut app).0;
178-
let snip = if use_lambda {
179-
let l_arg = if fn_has_arguments { "_" } else { "" };
180-
format!("|{l_arg}| {snip}")
181-
} else {
182-
snip.into_owned()
183-
};
184-
185-
if let Some(f) = second_arg {
186-
let f = snippet_with_context(cx, f.span, ctxt, "..", &mut app).0;
187-
format!("{snip}, {f}")
188-
} else {
189-
snip
190-
}
191-
};
192-
let span_replace_word = method_span.with_hi(span.hi());
193-
span_lint_and_sugg(
194-
cx,
195-
OR_FUN_CALL,
196-
span_replace_word,
197-
format!("function call inside of `{name}`"),
198-
"try",
199-
format!("{name}_{suffix}({sugg})"),
200-
app,
201-
);
202-
true
203-
} else {
204-
false
205-
}
206-
}
207-
20829
if let [arg] = args {
20930
let inner_arg = peel_blocks(arg);
21031
for_each_expr(cx, inner_arg, |ex| {
@@ -272,6 +93,183 @@ pub(super) fn check<'tcx>(
27293
}
27394
}
27495

96+
/// Checks for `unwrap_or(T::new())`, `unwrap_or(T::default())`,
97+
/// `or_insert(T::new())` or `or_insert(T::default())`.
98+
/// Similarly checks for `unwrap_or_else(T::new)`, `unwrap_or_else(T::default)`,
99+
/// `or_insert_with(T::new)` or `or_insert_with(T::default)`.
100+
fn check_unwrap_or_default(
101+
cx: &LateContext<'_>,
102+
name: Symbol,
103+
receiver: &hir::Expr<'_>,
104+
fun: &hir::Expr<'_>,
105+
call_expr: Option<&hir::Expr<'_>>,
106+
span: Span,
107+
method_span: Span,
108+
) -> bool {
109+
if !expr_type_is_certain(cx, receiver) {
110+
return false;
111+
}
112+
113+
let is_new = |fun: &hir::Expr<'_>| {
114+
if let hir::ExprKind::Path(ref qpath) = fun.kind {
115+
let path = last_path_segment(qpath).ident.name;
116+
matches!(path, sym::new)
117+
} else {
118+
false
119+
}
120+
};
121+
122+
let output_type_implements_default = |fun| {
123+
let fun_ty = cx.typeck_results().expr_ty(fun);
124+
if let ty::FnDef(def_id, args) = fun_ty.kind() {
125+
let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, args).skip_binder().output();
126+
cx.tcx
127+
.get_diagnostic_item(sym::Default)
128+
.is_some_and(|default_trait_id| implements_trait(cx, output_ty, default_trait_id, &[]))
129+
} else {
130+
false
131+
}
132+
};
133+
134+
let sugg = match (name, call_expr.is_some()) {
135+
(sym::unwrap_or, true) | (sym::unwrap_or_else, false) => sym::unwrap_or_default,
136+
(sym::or_insert, true) | (sym::or_insert_with, false) => sym::or_default,
137+
_ => return false,
138+
};
139+
140+
let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs();
141+
let Some(suggested_method_def_id) = receiver_ty.ty_adt_def().and_then(|adt_def| {
142+
cx.tcx
143+
.inherent_impls(adt_def.did())
144+
.iter()
145+
.flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg))
146+
.find_map(|assoc| {
147+
if assoc.is_method() && cx.tcx.fn_sig(assoc.def_id).skip_binder().inputs().skip_binder().len() == 1 {
148+
Some(assoc.def_id)
149+
} else {
150+
None
151+
}
152+
})
153+
}) else {
154+
return false;
155+
};
156+
let in_sugg_method_implementation = {
157+
matches!(
158+
suggested_method_def_id.as_local(),
159+
Some(local_def_id) if local_def_id == cx.tcx.hir_get_parent_item(receiver.hir_id).def_id
160+
)
161+
};
162+
if in_sugg_method_implementation {
163+
return false;
164+
}
165+
166+
// `.unwrap_or(vec![])` is as readable as `.unwrap_or_default()`. And if the expression is a
167+
// non-empty `Vec`, then it will not be a default value anyway. Bail out in all cases.
168+
if call_expr.and_then(|call_expr| VecArgs::hir(cx, call_expr)).is_some() {
169+
return false;
170+
}
171+
172+
// needs to target Default::default in particular or be *::new and have a Default impl
173+
// available
174+
if (is_new(fun) && output_type_implements_default(fun))
175+
|| match call_expr {
176+
Some(call_expr) => is_default_equivalent(cx, call_expr),
177+
None => is_default_equivalent_call(cx, fun, None) || closure_body_returns_empty_to_string(cx, fun),
178+
}
179+
{
180+
span_lint_and_sugg(
181+
cx,
182+
UNWRAP_OR_DEFAULT,
183+
method_span.with_hi(span.hi()),
184+
format!("use of `{name}` to construct default value"),
185+
"try",
186+
format!("{sugg}()"),
187+
Applicability::MachineApplicable,
188+
);
189+
190+
true
191+
} else {
192+
false
193+
}
194+
}
195+
196+
/// Checks for `*or(foo())`.
197+
#[expect(clippy::too_many_arguments)]
198+
fn check_or_fn_call<'tcx>(
199+
cx: &LateContext<'tcx>,
200+
name: Symbol,
201+
method_span: Span,
202+
self_expr: &hir::Expr<'_>,
203+
arg: &'tcx hir::Expr<'_>,
204+
// `Some` if fn has second argument
205+
second_arg: Option<&hir::Expr<'_>>,
206+
span: Span,
207+
// None if lambda is required
208+
fun_span: Option<Span>,
209+
) -> bool {
210+
// (path, fn_has_argument, methods, suffix)
211+
const KNOW_TYPES: [(Symbol, bool, &[Symbol], &str); 7] = [
212+
(sym::BTreeEntry, false, &[sym::or_insert], "with"),
213+
(sym::HashMapEntry, false, &[sym::or_insert], "with"),
214+
(
215+
sym::Option,
216+
false,
217+
&[sym::map_or, sym::ok_or, sym::or, sym::unwrap_or],
218+
"else",
219+
),
220+
(sym::Option, false, &[sym::get_or_insert], "with"),
221+
(sym::Option, true, &[sym::and], "then"),
222+
(sym::Result, true, &[sym::map_or, sym::or, sym::unwrap_or], "else"),
223+
(sym::Result, true, &[sym::and], "then"),
224+
];
225+
226+
if KNOW_TYPES.iter().any(|k| k.2.contains(&name))
227+
&& switch_to_lazy_eval(cx, arg)
228+
&& !contains_return(arg)
229+
&& let self_ty = cx.typeck_results().expr_ty(self_expr)
230+
&& let Some(&(_, fn_has_arguments, _, suffix)) = KNOW_TYPES
231+
.iter()
232+
.find(|&&i| is_type_diagnostic_item(cx, self_ty, i.0) && i.2.contains(&name))
233+
{
234+
let ctxt = span.ctxt();
235+
let mut app = Applicability::HasPlaceholders;
236+
let sugg = {
237+
let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
238+
(false, Some(fun_span)) => (fun_span, false),
239+
_ => (arg.span, true),
240+
};
241+
242+
let snip = snippet_with_context(cx, snippet_span, ctxt, "..", &mut app).0;
243+
let snip = if use_lambda {
244+
let l_arg = if fn_has_arguments { "_" } else { "" };
245+
format!("|{l_arg}| {snip}")
246+
} else {
247+
snip.into_owned()
248+
};
249+
250+
if let Some(f) = second_arg {
251+
let f = snippet_with_context(cx, f.span, ctxt, "..", &mut app).0;
252+
format!("{snip}, {f}")
253+
} else {
254+
snip
255+
}
256+
};
257+
let span_replace_word = method_span.with_hi(span.hi());
258+
span_lint_and_sugg(
259+
cx,
260+
OR_FUN_CALL,
261+
span_replace_word,
262+
format!("function call inside of `{name}`"),
263+
"try",
264+
format!("{name}_{suffix}({sugg})"),
265+
app,
266+
);
267+
true
268+
} else {
269+
false
270+
}
271+
}
272+
275273
fn closure_body_returns_empty_to_string(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
276274
if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = e.kind {
277275
let body = cx.tcx.hir_body(body);

‎tests/ui/or_fun_call.fixed‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::or_fun_call)]
2-
#![allow(dead_code)]
32
#![allow(
43
clippy::borrow_as_ptr,
54
clippy::uninlined_format_args,

‎tests/ui/or_fun_call.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::or_fun_call)]
2-
#![allow(dead_code)]
32
#![allow(
43
clippy::borrow_as_ptr,
54
clippy::uninlined_format_args,

0 commit comments

Comments
 (0)