Skip to content

Handle different root contexts in SpanlessEq #15793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Handle different root contexts in SpanlessEq #15793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions clippy_lints/src/booleans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ impl<'v> Hir2Qmm<'_, '_, 'v> {
return Err("contains never type".to_owned());
}

let ctxt = e.span.ctxt();
for (n, expr) in self.terminals.iter().enumerate() {
if eq_expr_value(self.cx, e, expr) {
if eq_expr_value(self.cx, ctxt, e, expr) {
#[expect(clippy::cast_possible_truncation)]
return Ok(Bool::Term(n as u8));
}
Expand All @@ -302,8 +303,8 @@ impl<'v> Hir2Qmm<'_, '_, 'v> {
&& implements_ord(self.cx, e_lhs)
&& let ExprKind::Binary(expr_binop, expr_lhs, expr_rhs) = &expr.kind
&& negate(e_binop.node) == Some(expr_binop.node)
&& eq_expr_value(self.cx, e_lhs, expr_lhs)
&& eq_expr_value(self.cx, e_rhs, expr_rhs)
&& eq_expr_value(self.cx, ctxt, e_lhs, expr_lhs)
&& eq_expr_value(self.cx, ctxt, e_rhs, expr_rhs)
{
#[expect(clippy::cast_possible_truncation)]
return Ok(Bool::Not(Box::new(Bool::Term(n as u8))));
Expand Down
15 changes: 8 additions & 7 deletions clippy_lints/src/checked_conversions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, TyKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::impl_lint_pass;
use rustc_span::Symbol;
use rustc_span::{Symbol, SyntaxContext};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -62,7 +62,8 @@ impl LateLintPass<'_> for CheckedConversions {
},
_ => return,
}
&& !item.span.in_external_macro(cx.sess().source_map())
&& let ctxt = item.span.ctxt()
&& !ctxt.in_external_macro(cx.sess().source_map())
&& !is_in_const_context(cx)
&& let Some(cv) = match op2 {
// todo: check for case signed -> larger unsigned == only x >= 0
Expand All @@ -71,7 +72,7 @@ impl LateLintPass<'_> for CheckedConversions {
let upper_lower = |lt1, gt1, lt2, gt2| {
check_upper_bound(lt1, gt1)
.zip(check_lower_bound(lt2, gt2))
.and_then(|(l, r)| l.combine(r, cx))
.and_then(|(l, r)| l.combine(r, cx, ctxt))
};
upper_lower(lt1, gt1, lt2, gt2).or_else(|| upper_lower(lt2, gt2, lt1, gt1))
},
Expand Down Expand Up @@ -125,8 +126,8 @@ fn read_le_ge<'tcx>(

impl<'a> Conversion<'a> {
/// Combine multiple conversions if the are compatible
pub fn combine(self, other: Self, cx: &LateContext<'_>) -> Option<Conversion<'a>> {
if self.is_compatible(&other, cx) {
pub fn combine(self, other: Self, cx: &LateContext<'_>, ctxt: SyntaxContext) -> Option<Conversion<'a>> {
if self.is_compatible(&other, cx, ctxt) {
// Prefer a Conversion that contains a type-constraint
Some(if self.to_type.is_some() { self } else { other })
} else {
Expand All @@ -136,9 +137,9 @@ impl<'a> Conversion<'a> {

/// Checks if two conversions are compatible
/// same type of conversion, same 'castee' and same 'to type'
pub fn is_compatible(&self, other: &Self, cx: &LateContext<'_>) -> bool {
pub fn is_compatible(&self, other: &Self, cx: &LateContext<'_>, ctxt: SyntaxContext) -> bool {
(self.cvt == other.cvt)
&& (SpanlessEq::new(cx).eq_expr(self.expr_to_cast, other.expr_to_cast))
&& (SpanlessEq::new(cx).eq_expr(ctxt, self.expr_to_cast, other.expr_to_cast))
&& (self.has_compatible_to_type(other))
}

Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/comparison_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;
use rustc_span::{SyntaxContext, sym};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -94,8 +94,10 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {

// Check that both sets of operands are equal
let mut spanless_eq = SpanlessEq::new(cx);
let same_fixed_operands = spanless_eq.eq_expr(lhs1, lhs2) && spanless_eq.eq_expr(rhs1, rhs2);
let same_transposed_operands = spanless_eq.eq_expr(lhs1, rhs2) && spanless_eq.eq_expr(rhs1, lhs2);
let same_fixed_operands = spanless_eq.eq_expr(SyntaxContext::root(), lhs1, lhs2)
&& spanless_eq.eq_expr(SyntaxContext::root(), rhs1, rhs2);
let same_transposed_operands = spanless_eq.eq_expr(SyntaxContext::root(), lhs1, rhs2)
&& spanless_eq.eq_expr(SyntaxContext::root(), rhs1, lhs2);

if !same_fixed_operands && !same_transposed_operands {
return;
Expand Down
38 changes: 26 additions & 12 deletions clippy_lints/src/copies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;
use rustc_span::hygiene::walk_chain;
use rustc_span::source_map::SourceMap;
use rustc_span::{Span, Symbol};
use rustc_span::{Span, Symbol, SyntaxContext};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -179,8 +179,8 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste<'tcx> {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
let (conds, blocks) = if_sequence(expr);
lint_same_cond(cx, &conds, &mut self.interior_mut);
lint_same_fns_in_if_cond(cx, &conds);
lint_same_cond(cx, SyntaxContext::root(), &conds, &mut self.interior_mut);
lint_same_fns_in_if_cond(cx, SyntaxContext::root(), &conds);
let all_same =
!is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
if !all_same && conds.len() != blocks.len() {
Expand All @@ -196,7 +196,10 @@ fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&
.array_windows::<2>()
.enumerate()
.fold(true, |all_eq, (i, &[lhs, rhs])| {
if eq.eq_block(lhs, rhs) && !has_let_expr(conds[i]) && conds.get(i + 1).is_none_or(|e| !has_let_expr(e)) {
if eq.eq_block(SyntaxContext::root(), lhs, rhs)
&& !has_let_expr(conds[i])
&& conds.get(i + 1).is_none_or(|e| !has_let_expr(e))
{
span_lint_and_note(
cx,
IF_SAME_THEN_ELSE,
Expand Down Expand Up @@ -376,7 +379,12 @@ fn eq_stmts(
.all(|b| get_stmt(b).is_some_and(|s| eq_binding_names(s, new_bindings)))
} else {
true
}) && blocks.iter().all(|b| get_stmt(b).is_some_and(|s| eq.eq_stmt(s, stmt)))
}) && blocks.iter().all(|b| {
get_stmt(b).is_some_and(|s| {
eq.set_eval_ctxt(SyntaxContext::root());
eq.eq_stmt(s, stmt)
})
})
}

#[expect(clippy::too_many_lines)]
Expand All @@ -387,7 +395,7 @@ fn scan_block_for_eq<'tcx>(
blocks: &[&'tcx Block<'_>],
) -> BlockEq {
let mut eq = SpanlessEq::new(cx);
let mut eq = eq.inter_expr();
let mut eq = eq.inter_expr(SyntaxContext::root());
let mut moved_locals = Vec::new();

let mut cond_locals = HirIdSet::default();
Expand Down Expand Up @@ -498,6 +506,7 @@ fn scan_block_for_eq<'tcx>(
});
if let Some(e) = block.expr {
for block in blocks {
eq.set_eval_ctxt(SyntaxContext::root());
if block.expr.is_some_and(|expr| !eq.eq_expr(expr, e)) {
moved_locals.truncate(moved_locals_at_start);
return BlockEq {
Expand Down Expand Up @@ -603,7 +612,12 @@ fn method_caller_is_mutable<'tcx>(
}

/// Implementation of `IFS_SAME_COND`.
fn lint_same_cond<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mut: &mut InteriorMut<'tcx>) {
fn lint_same_cond<'tcx>(
cx: &LateContext<'tcx>,
ctxt: SyntaxContext,
conds: &[&Expr<'_>],
interior_mut: &mut InteriorMut<'tcx>,
) {
for group in search_same(
conds,
|e| hash_expr(cx, e),
Expand All @@ -614,10 +628,10 @@ fn lint_same_cond<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mu
if method_caller_is_mutable(cx, caller, interior_mut) {
false
} else {
SpanlessEq::new(cx).eq_expr(lhs, rhs)
SpanlessEq::new(cx).eq_expr(ctxt, lhs, rhs)
}
} else {
eq_expr_value(cx, lhs, rhs)
eq_expr_value(cx, ctxt, lhs, rhs)
}
},
) {
Expand All @@ -627,17 +641,17 @@ fn lint_same_cond<'tcx>(cx: &LateContext<'tcx>, conds: &[&Expr<'_>], interior_mu
}

/// Implementation of `SAME_FUNCTIONS_IN_IF_CONDITION`.
fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, ctxt: SyntaxContext, conds: &[&Expr<'_>]) {
let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool {
// Do not lint if any expr originates from a macro
if lhs.span.from_expansion() || rhs.span.from_expansion() {
return false;
}
// Do not spawn warning if `IFS_SAME_COND` already produced it.
if eq_expr_value(cx, lhs, rhs) {
if eq_expr_value(cx, ctxt, lhs, rhs) {
return false;
}
SpanlessEq::new(cx).eq_expr(lhs, rhs)
SpanlessEq::new(cx).eq_expr(ctxt, lhs, rhs)
};

for group in search_same(conds, |e| hash_expr(cx, e), eq) {
Expand Down
11 changes: 6 additions & 5 deletions clippy_lints/src/entry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,11 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
}

match try_parse_insert(self.cx, expr) {
Some(insert_expr) if self.spanless_eq.eq_expr(self.map, insert_expr.map) => {
Some(insert_expr) if self.spanless_eq.eq_expr(self.ctxt, self.map, insert_expr.map) => {
self.visit_insert_expr_arguments(&insert_expr);
// Multiple inserts, inserts with a different key, and inserts from a macro can't use the entry api.
if self.is_map_used
|| !self.spanless_eq.eq_expr(self.key, insert_expr.key)
|| !self.spanless_eq.eq_expr(self.ctxt, self.key, insert_expr.key)
|| expr.span.ctxt() != self.ctxt
{
self.can_use_entry = false;
Expand All @@ -514,10 +514,10 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
self.visit_non_tail_expr(insert_expr.value);
self.is_single_insert = is_single_insert;
},
_ if is_any_expr_in_map_used(self.cx, &mut self.spanless_eq, self.map, expr) => {
_ if is_any_expr_in_map_used(self.cx, &mut self.spanless_eq, self.ctxt, self.map, expr) => {
self.is_map_used = true;
},
_ if self.spanless_eq.eq_expr(self.key, expr) => {
_ if self.spanless_eq.eq_expr(self.ctxt, self.key, expr) => {
self.is_key_used = true;
},
_ => match expr.kind {
Expand Down Expand Up @@ -586,11 +586,12 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
fn is_any_expr_in_map_used<'tcx>(
cx: &LateContext<'tcx>,
spanless_eq: &mut SpanlessEq<'_, 'tcx>,
ctxt: SyntaxContext,
map: &'tcx Expr<'tcx>,
expr: &'tcx Expr<'tcx>,
) -> bool {
for_each_expr(cx, map, |e| {
if spanless_eq.eq_expr(e, expr) {
if spanless_eq.eq_expr(ctxt, e, expr) {
return ControlFlow::Break(());
}
ControlFlow::Continue(())
Expand Down
42 changes: 25 additions & 17 deletions clippy_lints/src/floating_point_arithmetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_span::SyntaxContext;
use rustc_span::source_map::Spanned;
use std::f32::consts as f32_consts;
use std::f64::consts as f64_consts;
Expand Down Expand Up @@ -363,8 +364,9 @@ fn detect_hypot(cx: &LateContext<'_>, receiver: &Expr<'_>) -> Option<String> {
rmul_lhs,
rmul_rhs,
) = add_rhs.kind
&& eq_expr_value(cx, lmul_lhs, lmul_rhs)
&& eq_expr_value(cx, rmul_lhs, rmul_rhs)
&& let ctxt = receiver.span.ctxt()
&& eq_expr_value(cx, ctxt, lmul_lhs, lmul_rhs)
&& eq_expr_value(cx, ctxt, rmul_lhs, rmul_rhs)
{
return Some(format!(
"{}.hypot({})",
Expand Down Expand Up @@ -514,11 +516,11 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
/// test is positive or an expression which tests whether or not test
/// is nonnegative.
/// Used for check-custom-abs function below
fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
fn is_testing_positive(cx: &LateContext<'_>, ctxt: SyntaxContext, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
match op {
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && eq_expr_value(cx, left, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && eq_expr_value(cx, right, test),
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, right) && eq_expr_value(cx, ctxt, left, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, left) && eq_expr_value(cx, ctxt, right, test),
_ => false,
}
} else {
Expand All @@ -527,11 +529,11 @@ fn is_testing_positive(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -
}

/// See [`is_testing_positive`]
fn is_testing_negative(cx: &LateContext<'_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
fn is_testing_negative(cx: &LateContext<'_>, ctxt: SyntaxContext, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
match op {
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && eq_expr_value(cx, right, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && eq_expr_value(cx, left, test),
BinOpKind::Gt | BinOpKind::Ge => is_zero(cx, left) && eq_expr_value(cx, ctxt, right, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(cx, right) && eq_expr_value(cx, ctxt, left, test),
_ => false,
}
} else {
Expand All @@ -555,14 +557,19 @@ fn is_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// one of the two expressions
/// If the two expressions are not negations of each other, then it
/// returns None.
fn are_negated<'a>(cx: &LateContext<'_>, expr1: &'a Expr<'a>, expr2: &'a Expr<'a>) -> Option<(bool, &'a Expr<'a>)> {
fn are_negated<'a>(
cx: &LateContext<'_>,
ctxt: SyntaxContext,
expr1: &'a Expr<'a>,
expr2: &'a Expr<'a>,
) -> Option<(bool, &'a Expr<'a>)> {
if let ExprKind::Unary(UnOp::Neg, expr1_negated) = &expr1.kind
&& eq_expr_value(cx, expr1_negated, expr2)
&& eq_expr_value(cx, ctxt, expr1_negated, expr2)
{
return Some((false, expr2));
}
if let ExprKind::Unary(UnOp::Neg, expr2_negated) = &expr2.kind
&& eq_expr_value(cx, expr1, expr2_negated)
&& eq_expr_value(cx, ctxt, expr1, expr2_negated)
{
return Some((true, expr1));
}
Expand All @@ -577,7 +584,8 @@ fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
}) = higher::If::hir(expr)
&& let if_body_expr = peel_blocks(then)
&& let else_body_expr = peel_blocks(r#else)
&& let Some((if_expr_positive, body)) = are_negated(cx, if_body_expr, else_body_expr)
&& let ctxt = expr.span.ctxt()
&& let Some((if_expr_positive, body)) = are_negated(cx, ctxt, if_body_expr, else_body_expr)
{
let positive_abs_sugg = (
"manual implementation of `abs` method",
Expand All @@ -587,13 +595,13 @@ fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
"manual implementation of negation of `abs` method",
format!("-{}.abs()", Sugg::hir(cx, body, "..").maybe_paren()),
);
let sugg = if is_testing_positive(cx, cond, body) {
let sugg = if is_testing_positive(cx, ctxt, cond, body) {
if if_expr_positive {
positive_abs_sugg
} else {
negative_abs_sugg
}
} else if is_testing_negative(cx, cond, body) {
} else if is_testing_negative(cx, ctxt, cond, body) {
if if_expr_positive {
negative_abs_sugg
} else {
Expand All @@ -614,14 +622,14 @@ fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) {
}
}

fn are_same_base_logs(cx: &LateContext<'_>, expr_a: &Expr<'_>, expr_b: &Expr<'_>) -> bool {
fn are_same_base_logs(cx: &LateContext<'_>, ctxt: SyntaxContext, expr_a: &Expr<'_>, expr_b: &Expr<'_>) -> bool {
if let ExprKind::MethodCall(PathSegment { ident: method_a, .. }, _, args_a, _) = expr_a.kind
&& let ExprKind::MethodCall(PathSegment { ident: method_b, .. }, _, args_b, _) = expr_b.kind
{
return method_a.name == method_b.name
&& args_a.len() == args_b.len()
&& (matches!(method_a.name, sym::ln | sym::log2 | sym::log10)
|| method_a.name == sym::log && args_a.len() == 1 && eq_expr_value(cx, &args_a[0], &args_b[0]));
|| method_a.name == sym::log && args_a.len() == 1 && eq_expr_value(cx, ctxt, &args_a[0], &args_b[0]));
}

false
Expand All @@ -636,7 +644,7 @@ fn check_log_division(cx: &LateContext<'_>, expr: &Expr<'_>) {
lhs,
rhs,
) = &expr.kind
&& are_same_base_logs(cx, lhs, rhs)
&& are_same_base_logs(cx, expr.span.ctxt(), lhs, rhs)
&& let ExprKind::MethodCall(_, largs_self, ..) = &lhs.kind
&& let ExprKind::MethodCall(_, rargs_self, ..) = &rhs.kind
{
Expand Down
Loading