Skip to content

Commit

Permalink
refactor: aggregate the separated functions
Browse files Browse the repository at this point in the history
  • Loading branch information
@chansuke
chansuke committed Jan 22, 2023
1 parent d985787 commit c348dd1
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 23 deletions.
93 changes: 75 additions & 18 deletions clippy_lints/src/swap.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::macros::HirNode;
use clippy_utils::source::{first_line_of_span, snippet_with_applicability};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{can_mut_borrow_both, eq_expr_value, in_constant, std_or_core, SpanlessEq};
use clippy_utils::{can_mut_borrow_both, eq_expr_value, in_constant, std_or_core};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Local, PatKind, QPath, Stmt, StmtKind};
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -176,13 +175,13 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) {
#[allow(clippy::too_many_lines)]
/// Implementation of the `ALMOST_SWAPPED` lint.
fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) {
for [first_, second_] in block.stmts.array_windows() {
for [first, second] in block.stmts.array_windows() {
if_chain! {
if let StmtKind::Semi(first) = first_.kind;
if let StmtKind::Semi(second) = second_.kind;
if let StmtKind::Semi(first_semi) = first.kind;
if let StmtKind::Semi(second_semi) = second.kind;
if first.span.ctxt() == second.span.ctxt();
if let ExprKind::Assign(lhs0, rhs0, _) = first.kind;
if let ExprKind::Assign(lhs1, rhs1, _) = second.kind;
if let ExprKind::Assign(lhs0, rhs0, _) = first_semi.kind;
if let ExprKind::Assign(lhs1, rhs1, _) = second_semi.kind;
if eq_expr_value(cx, lhs0, rhs1);
if eq_expr_value(cx, lhs1, rhs0);
then {
Expand Down Expand Up @@ -223,20 +222,51 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) {
}
}

if let Some((lhs0, rhs0)) = parse(first_)
&& let Some((lhs1, rhs1)) = parse(second_)
&& is_same(cx, &lhs0, rhs1)
&& is_same(cx, &lhs1, rhs0)
if let Some((lhs0, rhs0)) = parse(first)
&& let Some((lhs1, rhs1)) = parse(second)
&& is_same(&lhs0, rhs1)
&& is_same(&lhs1, rhs0)
{
// WIP
let lhs_opt: Option<Sugg<'_>>;
lhs_opt = if let ExprOrIdent::Expr(lhs_expr) = lhs0 {
Sugg::hir_opt(cx, lhs_expr)
} else {
None
};

let ident_l: Option<Ident>;
ident_l = if let ExprOrIdent::Ident(lhs_ident) = lhs0 {
Some(lhs_ident)
} else {
None
};

let rhs_opt = Sugg::hir_opt(cx, rhs0);
let (what, lhs, rhs) = if let (Some(first), Some(second)) = (lhs_opt, rhs_opt.clone()) {
(
format!(" `{}` and `{}`", first, second),
first.mut_addr().to_string(),
second.mut_addr().to_string(),
)
} else if let (Some(first_ident), Some(second)) = (ident_l, rhs_opt.clone()) {
(
format!(" `{}` and `{}`", first_ident.name.as_str(), second),
format!("&mut {}", first_ident.name.as_str()),
second.mut_addr().to_string(),
)
} else {
(String::new(), String::new(), String::new())
};
let span = first.span.to(second.span);
let Some(sugg) = std_or_core(cx) else { return };

lint_almost_swapped_note(cx, span, &what, sugg, &lhs, &rhs);
}
}
}

fn is_same<'a, 'hir>(cx: &LateContext<'_>, lhs: &'a ExprOrIdent<'hir>, rhs: &'a Expr<'hir>) -> bool {
if let ExprOrIdent::Expr(expr_l) = lhs {
eq_expr_value(cx, expr_l, rhs)
} else if let ExprOrIdent::Ident(ident_l) = lhs {
fn is_same<'a, 'hir>(lhs: &'a ExprOrIdent<'hir>, rhs: &'a Expr<'hir>) -> bool {
if let ExprOrIdent::Ident(ident_l) = lhs {
if let ExprKind::Path(QPath::Resolved(None, path_r)) = rhs.kind {
if ident_l.name.as_str()
== path_r
Expand All @@ -253,6 +283,32 @@ fn is_same<'a, 'hir>(cx: &LateContext<'_>, lhs: &'a ExprOrIdent<'hir>, rhs: &'a
} else {
false
}
} else if let ExprOrIdent::Expr(expr_l) = lhs && expr_l.span.ctxt() == rhs.span.ctxt() {
if let ExprKind::Path(QPath::Resolved(None, path_l)) = expr_l.kind {
if let ExprKind::Path(QPath::Resolved(None, path_r)) = rhs.kind {
if path_l
.segments
.iter()
.map(|el| el.ident.to_string())
.collect::<Vec<_>>()
.join("::")
== path_r
.segments
.iter()
.map(|el| el.ident.to_string())
.collect::<Vec<_>>()
.join("::")
{
true
} else {
false
}
} else {
false
}
} else {
false
}
} else {
false
}
Expand All @@ -274,6 +330,7 @@ fn lint_almost_swapped_note(cx: &LateContext<'_>, span: Span, what: &str, sugg:
);
}

#[derive(Debug, Clone, Copy)]
pub enum ExprOrIdent<'a> {
Expr(&'a Expr<'a>),
Ident(Ident),
Expand Down
19 changes: 14 additions & 5 deletions tests/ui/almost_swapped.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,27 @@ error: this looks like you are trying to swap `d` and `c`
|
LL | / d = c;
LL | | c = d;
| |_________^ help: try: `std::mem::swap(&mut d, &mut c)`
| |__________^ help: try: `std::mem::swap(&mut d, &mut c)`
|
= note: or maybe you should use `std::mem::replace`?

error: this looks like you are trying to swap `b` and `a`
error: this looks like you are trying to swap `d` and `c`
--> $DIR/almost_swapped.rs:12:5
|
LL | / d = c;
LL | | c = d;
| |__________^
|
= note: maybe you could use `std::mem::swap(&mut d, &mut c)` or `std::mem::replace`?

error: this looks like you are trying to swap `a` and `b`
--> $DIR/almost_swapped.rs:16:5
|
LL | / let a = b;
LL | | b = a;
| |_________^
| |__________^
|
= note: maybe you could use `std::mem::swap(&mut b, &mut a)` or `std::mem::replace`?
= note: maybe you could use `std::mem::swap(&mut a, &mut b)` or `std::mem::replace`?

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

0 comments on commit c348dd1

Please sign in to comment.