Skip to content

Commit 917890b

Browse files
committed
Auto merge of #8201 - smoelius:master, r=camsteffen
Change `unnecessary_to_owned` `into_iter` suggestions to `MaybeIncorrect` I am having a hard time finding a good solution for #8148, so I am wondering if is enough to just change the suggestion's applicability to `MaybeIncorrect`? I apologize, as I realize this is a bit of a cop out. changelog: none
2 parents be7cf76 + 366234a commit 917890b

File tree

3 files changed

+30
-5
lines changed

3 files changed

+30
-5
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,6 +1895,11 @@ declare_clippy_lint! {
18951895
/// ### Why is this bad?
18961896
/// The unnecessary calls result in useless allocations.
18971897
///
1898+
/// ### Known problems
1899+
/// `unnecessary_to_owned` can falsely trigger if `IntoIterator::into_iter` is applied to an
1900+
/// owned copy of a resource and the resource is later used mutably. See
1901+
/// [#8148](https://github.com/rust-lang/rust-clippy/issues/8148).
1902+
///
18981903
/// ### Example
18991904
/// ```rust
19001905
/// let path = std::path::Path::new("x");

clippy_lints/src/methods/unnecessary_iter_cloned.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol
1818
if let Some(callee_def_id) = fn_def_id(cx, parent);
1919
if is_into_iter(cx, callee_def_id);
2020
then {
21-
check_for_loop_iter(cx, parent, method_name, receiver)
21+
check_for_loop_iter(cx, parent, method_name, receiver, false)
2222
} else {
2323
false
2424
}
@@ -34,6 +34,7 @@ pub fn check_for_loop_iter(
3434
expr: &'tcx Expr<'tcx>,
3535
method_name: Symbol,
3636
receiver: &'tcx Expr<'tcx>,
37+
cloned_before_iter: bool,
3738
) -> bool {
3839
if_chain! {
3940
if let Some(grandparent) = get_parent_expr(cx, expr).and_then(|parent| get_parent_expr(cx, parent));
@@ -70,12 +71,22 @@ pub fn check_for_loop_iter(
7071
expr.span,
7172
&format!("unnecessary use of `{}`", method_name),
7273
|diag| {
73-
diag.span_suggestion(expr.span, "use", snippet, Applicability::MachineApplicable);
74+
// If `check_into_iter_call_arg` called `check_for_loop_iter` because a call to
75+
// a `to_owned`-like function was removed, then the next suggestion may be
76+
// incorrect. This is because the iterator that results from the call's removal
77+
// could hold a reference to a resource that is used mutably. See
78+
// https://github.com/rust-lang/rust-clippy/issues/8148.
79+
let applicability = if cloned_before_iter {
80+
Applicability::MaybeIncorrect
81+
} else {
82+
Applicability::MachineApplicable
83+
};
84+
diag.span_suggestion(expr.span, "use", snippet, applicability);
7485
for addr_of_expr in addr_of_exprs {
7586
match addr_of_expr.kind {
7687
ExprKind::AddrOf(_, _, referent) => {
7788
let span = addr_of_expr.span.with_hi(referent.span.lo());
78-
diag.span_suggestion(span, "remove this `&`", String::new(), Applicability::MachineApplicable);
89+
diag.span_suggestion(span, "remove this `&`", String::new(), applicability);
7990
}
8091
_ => unreachable!(),
8192
}

clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,22 +187,31 @@ fn check_into_iter_call_arg(
187187
if let Some(item_ty) = get_iterator_item_ty(cx, parent_ty);
188188
if let Some(receiver_snippet) = snippet_opt(cx, receiver.span);
189189
then {
190-
if unnecessary_iter_cloned::check_for_loop_iter(cx, parent, method_name, receiver) {
190+
if unnecessary_iter_cloned::check_for_loop_iter(
191+
cx,
192+
parent,
193+
method_name,
194+
receiver,
195+
true,
196+
) {
191197
return true;
192198
}
193199
let cloned_or_copied = if is_copy(cx, item_ty) {
194200
"copied"
195201
} else {
196202
"cloned"
197203
};
204+
// The next suggestion may be incorrect because the removal of the `to_owned`-like
205+
// function could cause the iterator to hold a reference to a resource that is used
206+
// mutably. See https://github.com/rust-lang/rust-clippy/issues/8148.
198207
span_lint_and_sugg(
199208
cx,
200209
UNNECESSARY_TO_OWNED,
201210
parent.span,
202211
&format!("unnecessary use of `{}`", method_name),
203212
"use",
204213
format!("{}.iter().{}()", receiver_snippet, cloned_or_copied),
205-
Applicability::MachineApplicable,
214+
Applicability::MaybeIncorrect,
206215
);
207216
return true;
208217
}

0 commit comments

Comments
 (0)