cmp_owned refactor
This commit is contained in:
parent
c9718fa589
commit
352863065c
1 changed files with 14 additions and 31 deletions
|
@ -21,7 +21,7 @@ use crate::utils::{get_item_name, get_parent_expr, implements_trait, in_constant
|
||||||
iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
|
iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
|
||||||
span_lint_and_then, walk_ptrs_ty, SpanlessEq};
|
span_lint_and_then, walk_ptrs_ty, SpanlessEq};
|
||||||
use crate::utils::sugg::Sugg;
|
use crate::utils::sugg::Sugg;
|
||||||
use crate::syntax::ast::{LitKind, CRATE_NODE_ID};
|
use crate::syntax::ast::LitKind;
|
||||||
use crate::consts::{constant, Constant};
|
use crate::consts::{constant, Constant};
|
||||||
use crate::rustc_errors::Applicability;
|
use crate::rustc_errors::Applicability;
|
||||||
|
|
||||||
|
@ -540,18 +540,10 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) {
|
||||||
_ => false,
|
_ => false,
|
||||||
};
|
};
|
||||||
|
|
||||||
let (lint_span, try_hint) = if deref_arg_impl_partial_eq_other {
|
let lint_span = if other_gets_derefed {
|
||||||
// suggest deref on the left
|
expr.span.to(other.span)
|
||||||
(expr.span, format!("*{}", snip))
|
|
||||||
} else if other_gets_derefed {
|
|
||||||
// suggest dropping the to_owned on the left and the deref on the right
|
|
||||||
let other_snippet = snippet(cx, other.span, "..").into_owned();
|
|
||||||
let other_without_deref = other_snippet.replacen('*', "", 1);
|
|
||||||
|
|
||||||
(expr.span.to(other.span), format!("{} == {}", snip.to_string(), other_without_deref))
|
|
||||||
} else {
|
} else {
|
||||||
// suggest dropping the to_owned on the left
|
expr.span
|
||||||
(expr.span, snip.to_string())
|
|
||||||
};
|
};
|
||||||
|
|
||||||
span_lint_and_then(
|
span_lint_and_then(
|
||||||
|
@ -560,29 +552,20 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) {
|
||||||
lint_span,
|
lint_span,
|
||||||
"this creates an owned instance just for comparison",
|
"this creates an owned instance just for comparison",
|
||||||
|db| {
|
|db| {
|
||||||
// this is as good as our recursion check can get, we can't prove that the
|
// this also catches PartialEq implementations that call to_owned
|
||||||
// current function is
|
|
||||||
// called by
|
|
||||||
// PartialEq::eq, but we can at least ensure that this code is not part of it
|
|
||||||
let parent_fn = cx.tcx.hir.get_parent(expr.id);
|
|
||||||
let parent_impl = cx.tcx.hir.get_parent(parent_fn);
|
|
||||||
if parent_impl != CRATE_NODE_ID {
|
|
||||||
if let Node::Item(item) = cx.tcx.hir.get(parent_impl) {
|
|
||||||
if let ItemKind::Impl(.., Some(ref trait_ref), _, _) = item.node {
|
|
||||||
if trait_ref.path.def.def_id() == partial_eq_trait_id {
|
|
||||||
// we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise
|
|
||||||
// we go into
|
|
||||||
// recursion
|
|
||||||
db.span_label(lint_span, "try implementing the comparison without allocating");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if other_gets_derefed {
|
if other_gets_derefed {
|
||||||
db.span_label(lint_span, "try implementing the comparison without allocating");
|
db.span_label(lint_span, "try implementing the comparison without allocating");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let try_hint = if deref_arg_impl_partial_eq_other {
|
||||||
|
// suggest deref on the left
|
||||||
|
format!("*{}", snip)
|
||||||
|
} else {
|
||||||
|
// suggest dropping the to_owned on the left
|
||||||
|
snip.to_string()
|
||||||
|
};
|
||||||
|
|
||||||
db.span_suggestion_with_applicability(
|
db.span_suggestion_with_applicability(
|
||||||
lint_span,
|
lint_span,
|
||||||
"try",
|
"try",
|
||||||
|
|
Loading…
Reference in a new issue