Check inner caller for clone and judge whether they are mutable
if immutbale -> lint delete redudant clone if mutable -> lint check whether clone is needed
This commit is contained in:
parent
befb659145
commit
a8c35cbbda
4 changed files with 117 additions and 4 deletions
|
|
@ -3,10 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_then;
|
|||
use clippy_utils::higher::ForLoop;
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::ty::{get_iterator_item_ty, implements_trait};
|
||||
use clippy_utils::{fn_def_id, get_parent_expr};
|
||||
use clippy_utils::visitors::for_each_expr;
|
||||
use clippy_utils::{can_mut_borrow_both, fn_def_id, get_parent_expr, path_to_local};
|
||||
use core::ops::ControlFlow;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_hir::{Expr, ExprKind};
|
||||
use rustc_hir::{BindingMode, Expr, ExprKind, Node, PatKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_span::{sym, Symbol};
|
||||
|
||||
|
|
@ -40,6 +42,53 @@ pub fn check_for_loop_iter(
|
|||
&& !clone_or_copy_needed
|
||||
&& let Some(receiver_snippet) = snippet_opt(cx, receiver.span)
|
||||
{
|
||||
// Issue 12098
|
||||
// https://github.com/rust-lang/rust-clippy/issues/12098
|
||||
// if the assignee have `mut borrow` conflict with the iteratee
|
||||
// the lint should not execute, former didn't consider the mut case
|
||||
|
||||
// check whether `expr` is mutable
|
||||
fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
if let Some(hir_id) = path_to_local(expr)
|
||||
&& let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
|
||||
{
|
||||
matches!(pat.kind, PatKind::Binding(BindingMode::MUT, ..))
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
fn is_caller_or_fields_change(cx: &LateContext<'_>, body: &Expr<'_>, caller: &Expr<'_>) -> bool {
|
||||
let mut change = false;
|
||||
if let ExprKind::Block(block, ..) = body.kind {
|
||||
for_each_expr(block, |e| {
|
||||
match e.kind {
|
||||
ExprKind::Assign(assignee, _, _) | ExprKind::AssignOp(_, assignee, _) => {
|
||||
change |= !can_mut_borrow_both(cx, caller, assignee);
|
||||
},
|
||||
_ => {},
|
||||
}
|
||||
// the return value has no effect but the function need one return value
|
||||
ControlFlow::<()>::Continue(())
|
||||
});
|
||||
}
|
||||
change
|
||||
}
|
||||
|
||||
if let ExprKind::Call(_, [child, ..]) = expr.kind {
|
||||
// filter first layer of iterator
|
||||
let mut child = child;
|
||||
// get inner real caller requests for clone
|
||||
while let ExprKind::MethodCall(_, caller, _, _) = child.kind {
|
||||
child = caller;
|
||||
}
|
||||
if is_mutable(cx, child) && is_caller_or_fields_change(cx, body, child) {
|
||||
// skip lint
|
||||
return true;
|
||||
}
|
||||
};
|
||||
|
||||
// the lint should not be executed if no violation happens
|
||||
let snippet = if let ExprKind::MethodCall(maybe_iter_method_name, collection, [], _) = receiver.kind
|
||||
&& maybe_iter_method_name.ident.name == sym::iter
|
||||
&& let Some(iterator_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator)
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ fn main() {
|
|||
let _ = check_files_ref_mut(&[(FileType::Account, path)]);
|
||||
let _ = check_files_self_and_arg(&[(FileType::Account, path)]);
|
||||
let _ = check_files_mut_path_buf(&[(FileType::Account, std::path::PathBuf::new())]);
|
||||
|
||||
check_mut_iteratee_and_modify_inner_variable();
|
||||
}
|
||||
|
||||
// `check_files` and its variants are based on:
|
||||
|
|
@ -138,3 +140,33 @@ fn check_files_mut_path_buf(files: &[(FileType, std::path::PathBuf)]) -> bool {
|
|||
fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::Error> {
|
||||
Ok(std::path::PathBuf::new())
|
||||
}
|
||||
|
||||
// Issue 12098
|
||||
// https://github.com/rust-lang/rust-clippy/issues/12098
|
||||
// no message emits
|
||||
fn check_mut_iteratee_and_modify_inner_variable() {
|
||||
struct Test {
|
||||
list: Vec<String>,
|
||||
mut_this: bool,
|
||||
}
|
||||
|
||||
impl Test {
|
||||
fn list(&self) -> &[String] {
|
||||
&self.list
|
||||
}
|
||||
}
|
||||
|
||||
let mut test = Test {
|
||||
list: vec![String::from("foo"), String::from("bar")],
|
||||
mut_this: false,
|
||||
};
|
||||
|
||||
for _item in test.list().to_vec() {
|
||||
println!("{}", _item);
|
||||
|
||||
test.mut_this = true;
|
||||
{
|
||||
test.mut_this = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ fn main() {
|
|||
let _ = check_files_ref_mut(&[(FileType::Account, path)]);
|
||||
let _ = check_files_self_and_arg(&[(FileType::Account, path)]);
|
||||
let _ = check_files_mut_path_buf(&[(FileType::Account, std::path::PathBuf::new())]);
|
||||
|
||||
check_mut_iteratee_and_modify_inner_variable();
|
||||
}
|
||||
|
||||
// `check_files` and its variants are based on:
|
||||
|
|
@ -138,3 +140,33 @@ fn check_files_mut_path_buf(files: &[(FileType, std::path::PathBuf)]) -> bool {
|
|||
fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::Error> {
|
||||
Ok(std::path::PathBuf::new())
|
||||
}
|
||||
|
||||
// Issue 12098
|
||||
// https://github.com/rust-lang/rust-clippy/issues/12098
|
||||
// no message emits
|
||||
fn check_mut_iteratee_and_modify_inner_variable() {
|
||||
struct Test {
|
||||
list: Vec<String>,
|
||||
mut_this: bool,
|
||||
}
|
||||
|
||||
impl Test {
|
||||
fn list(&self) -> &[String] {
|
||||
&self.list
|
||||
}
|
||||
}
|
||||
|
||||
let mut test = Test {
|
||||
list: vec![String::from("foo"), String::from("bar")],
|
||||
mut_this: false,
|
||||
};
|
||||
|
||||
for _item in test.list().to_vec() {
|
||||
println!("{}", _item);
|
||||
|
||||
test.mut_this = true;
|
||||
{
|
||||
test.mut_this = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: unnecessary use of `copied`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:29:22
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:31:22
|
||||
|
|
||||
LL | for (t, path) in files.iter().copied() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -17,7 +17,7 @@ LL + let other = match get_file_path(t) {
|
|||
|
|
||||
|
||||
error: unnecessary use of `copied`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:44:22
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:46:22
|
||||
|
|
||||
LL | for (t, path) in files.iter().copied() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue