Skip to content

Commit 58132cb

Browse files
JarchoJarcho
committed
Improve SpanlessEq
* Don't consider expansions of different macros to be the same, even if they expand to the same tokens * Don't consider `cfg!` expansions to be equal if they check different configs.
1 parent 5351170 commit 58132cb

12 files changed

+135
-34
lines changed

Diff for: ‎clippy_utils/src/consts.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![allow(clippy::float_cmp)]
22

3-
use crate::source::{span_source_range, walk_span_to_context};
3+
use crate::source::{get_source_text, walk_span_to_context};
44
use crate::{clip, is_direct_expn_of, sext, unsext};
55
use if_chain::if_chain;
66
use rustc_ast::ast::{self, LitFloatType, LitKind};
@@ -516,7 +516,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
516516
if let Some(expr_span) = walk_span_to_context(expr.span, span.ctxt)
517517
&& let expr_lo = expr_span.lo()
518518
&& expr_lo >= span.lo
519-
&& let Some(src) = span_source_range(self.lcx, span.lo..expr_lo)
519+
&& let Some(src) = get_source_text(self.lcx, span.lo..expr_lo)
520520
&& let Some(src) = src.as_str()
521521
{
522522
use rustc_lexer::TokenKind::{Whitespace, LineComment, BlockComment, Semi, OpenBrace};

Diff for: ‎clippy_utils/src/hir_utils.rs

+49-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use rustc_hir::{
1414
use rustc_lexer::{tokenize, TokenKind};
1515
use rustc_lint::LateContext;
1616
use rustc_middle::ty::TypeckResults;
17-
use rustc_span::{sym, BytePos, Symbol, SyntaxContext};
17+
use rustc_span::{sym, BytePos, ExpnKind, MacroKind, Symbol, SyntaxContext};
1818
use std::hash::{Hash, Hasher};
1919
use std::ops::Range;
2020

@@ -67,6 +67,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
6767
pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> {
6868
HirEqInterExpr {
6969
inner: self,
70+
left_ctxt: SyntaxContext::root(),
71+
right_ctxt: SyntaxContext::root(),
7072
locals: HirIdMap::default(),
7173
}
7274
}
@@ -94,6 +96,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
9496

9597
pub struct HirEqInterExpr<'a, 'b, 'tcx> {
9698
inner: &'a mut SpanlessEq<'b, 'tcx>,
99+
left_ctxt: SyntaxContext,
100+
right_ctxt: SyntaxContext,
97101

98102
// When binding are declared, the binding ID in the left expression is mapped to the one on the
99103
// right. For example, when comparing `{ let x = 1; x + 2 }` and `{ let y = 1; y + 2 }`,
@@ -128,6 +132,7 @@ impl HirEqInterExpr<'_, '_, '_> {
128132
}
129133

130134
/// Checks whether two blocks are the same.
135+
#[expect(clippy::similar_names)]
131136
fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
132137
use TokenKind::{BlockComment, LineComment, Semi, Whitespace};
133138
if left.stmts.len() != right.stmts.len() {
@@ -166,7 +171,8 @@ impl HirEqInterExpr<'_, '_, '_> {
166171
// Can happen when macros expand to multiple statements, or rearrange statements.
167172
// Nothing in between the statements to check in this case.
168173
continue;
169-
} else if lstmt_span.lo < lstart || rstmt_span.lo < rstart {
174+
}
175+
if lstmt_span.lo < lstart || rstmt_span.lo < rstart {
170176
// Only one of the blocks had a weird macro.
171177
return false;
172178
}
@@ -243,7 +249,7 @@ impl HirEqInterExpr<'_, '_, '_> {
243249

244250
#[expect(clippy::similar_names)]
245251
pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
246-
if !self.inner.allow_side_effects && left.span.ctxt() != right.span.ctxt() {
252+
if !self.check_ctxt(left.span.ctxt(), right.span.ctxt()) {
247253
return false;
248254
}
249255

@@ -476,6 +482,45 @@ impl HirEqInterExpr<'_, '_, '_> {
476482
fn eq_type_binding(&mut self, left: &TypeBinding<'_>, right: &TypeBinding<'_>) -> bool {
477483
left.ident.name == right.ident.name && self.eq_ty(left.ty(), right.ty())
478484
}
485+
486+
fn check_ctxt(&mut self, left: SyntaxContext, right: SyntaxContext) -> bool {
487+
if self.left_ctxt == left && self.right_ctxt == right {
488+
return true;
489+
} else if self.left_ctxt == left || self.right_ctxt == right {
490+
// Only one context has changed. This can only happen if the two nodes are written differently.
491+
return false;
492+
} else if left != SyntaxContext::root() {
493+
let mut left_data = left.outer_expn_data();
494+
let mut right_data = right.outer_expn_data();
495+
loop {
496+
use TokenKind::{BlockComment, LineComment, Whitespace};
497+
if left_data.macro_def_id != right_data.macro_def_id
498+
|| (matches!(left_data.kind, ExpnKind::Macro(MacroKind::Bang, name) if name == sym::cfg)
499+
&& !eq_span_tokens(self.inner.cx, left_data.call_site, right_data.call_site, |t| {
500+
!matches!(t, Whitespace | LineComment { .. } | BlockComment { .. })
501+
}))
502+
{
503+
// Either a different chain of macro calls, or different arguments to the `cfg` macro.
504+
return false;
505+
}
506+
let left_ctxt = left_data.call_site.ctxt();
507+
let right_ctxt = right_data.call_site.ctxt();
508+
if left_ctxt == SyntaxContext::root() && right_ctxt == SyntaxContext::root() {
509+
break;
510+
}
511+
if left_ctxt == SyntaxContext::root() || right_ctxt == SyntaxContext::root() {
512+
// Different lengths for the expansion stack. This can only happen if nodes are written differently,
513+
// or shouldn't be compared to start with.
514+
return false;
515+
}
516+
left_data = left_ctxt.outer_expn_data();
517+
right_data = right_ctxt.outer_expn_data();
518+
}
519+
}
520+
self.left_ctxt = left;
521+
self.right_ctxt = right;
522+
true
523+
}
479524
}
480525

481526
/// Some simple reductions like `{ return }` => `return`
@@ -1075,6 +1120,7 @@ pub fn hash_expr(cx: &LateContext<'_>, e: &Expr<'_>) -> u64 {
10751120
h.finish()
10761121
}
10771122

1123+
#[expect(clippy::similar_names)]
10781124
fn eq_span_tokens(
10791125
cx: &LateContext<'_>,
10801126
left: impl SpanRange,

Diff for: ‎clippy_utils/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti
14991499
&& let const_val = cx.tcx.valtree_to_const_val((bnd_ty, min_val.to_valtree()))
15001500
&& let min_const_kind = ConstantKind::from_value(const_val, bnd_ty)
15011501
&& let Some(min_const) = miri_to_const(cx.tcx, min_const_kind)
1502-
&& let Some((start_const, _)) = constant(cx, cx.typeck_results(), start)
1502+
&& let Some(start_const) = constant(cx, cx.typeck_results(), start)
15031503
{
15041504
start_const == min_const
15051505
} else {
@@ -1515,7 +1515,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti
15151515
&& let const_val = cx.tcx.valtree_to_const_val((bnd_ty, max_val.to_valtree()))
15161516
&& let max_const_kind = ConstantKind::from_value(const_val, bnd_ty)
15171517
&& let Some(max_const) = miri_to_const(cx.tcx, max_const_kind)
1518-
&& let Some((end_const, _)) = constant(cx, cx.typeck_results(), end)
1518+
&& let Some(end_const) = constant(cx, cx.typeck_results(), end)
15191519
{
15201520
end_const == max_const
15211521
} else {

Diff for: ‎tests/ui/collapsible_if.fixed

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
//@run-rustfix
2-
#![allow(clippy::assertions_on_constants, clippy::equatable_if_let)]
2+
#![allow(
3+
clippy::assertions_on_constants,
4+
clippy::equatable_if_let,
5+
clippy::nonminimal_bool,
6+
clippy::eq_op
7+
)]
38

49
#[rustfmt::skip]
510
#[warn(clippy::collapsible_if)]

Diff for: ‎tests/ui/collapsible_if.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
//@run-rustfix
2-
#![allow(clippy::assertions_on_constants, clippy::equatable_if_let)]
2+
#![allow(
3+
clippy::assertions_on_constants,
4+
clippy::equatable_if_let,
5+
clippy::nonminimal_bool,
6+
clippy::eq_op
7+
)]
38

49
#[rustfmt::skip]
510
#[warn(clippy::collapsible_if)]

Diff for: ‎tests/ui/collapsible_if.stderr

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` statement can be collapsed
2-
--> $DIR/collapsible_if.rs:9:5
2+
--> $DIR/collapsible_if.rs:14:5
33
|
44
LL | / if x == "hello" {
55
LL | | if y == "world" {
@@ -17,7 +17,7 @@ LL + }
1717
|
1818

1919
error: this `if` statement can be collapsed
20-
--> $DIR/collapsible_if.rs:15:5
20+
--> $DIR/collapsible_if.rs:20:5
2121
|
2222
LL | / if x == "hello" || x == "world" {
2323
LL | | if y == "world" || y == "hello" {
@@ -34,7 +34,7 @@ LL + }
3434
|
3535

3636
error: this `if` statement can be collapsed
37-
--> $DIR/collapsible_if.rs:21:5
37+
--> $DIR/collapsible_if.rs:26:5
3838
|
3939
LL | / if x == "hello" && x == "world" {
4040
LL | | if y == "world" || y == "hello" {
@@ -51,7 +51,7 @@ LL + }
5151
|
5252

5353
error: this `if` statement can be collapsed
54-
--> $DIR/collapsible_if.rs:27:5
54+
--> $DIR/collapsible_if.rs:32:5
5555
|
5656
LL | / if x == "hello" || x == "world" {
5757
LL | | if y == "world" && y == "hello" {
@@ -68,7 +68,7 @@ LL + }
6868
|
6969

7070
error: this `if` statement can be collapsed
71-
--> $DIR/collapsible_if.rs:33:5
71+
--> $DIR/collapsible_if.rs:38:5
7272
|
7373
LL | / if x == "hello" && x == "world" {
7474
LL | | if y == "world" && y == "hello" {
@@ -85,7 +85,7 @@ LL + }
8585
|
8686

8787
error: this `if` statement can be collapsed
88-
--> $DIR/collapsible_if.rs:39:5
88+
--> $DIR/collapsible_if.rs:44:5
8989
|
9090
LL | / if 42 == 1337 {
9191
LL | | if 'a' != 'A' {
@@ -102,7 +102,7 @@ LL + }
102102
|
103103

104104
error: this `if` statement can be collapsed
105-
--> $DIR/collapsible_if.rs:95:5
105+
--> $DIR/collapsible_if.rs:100:5
106106
|
107107
LL | / if x == "hello" {
108108
LL | | if y == "world" { // Collapsible
@@ -119,15 +119,15 @@ LL + }
119119
|
120120

121121
error: this `if` statement can be collapsed
122-
--> $DIR/collapsible_if.rs:154:5
122+
--> $DIR/collapsible_if.rs:159:5
123123
|
124124
LL | / if matches!(true, true) {
125125
LL | | if matches!(true, true) {}
126126
LL | | }
127127
| |_____^ help: collapse nested if block: `if matches!(true, true) && matches!(true, true) {}`
128128

129129
error: this `if` statement can be collapsed
130-
--> $DIR/collapsible_if.rs:159:5
130+
--> $DIR/collapsible_if.rs:164:5
131131
|
132132
LL | / if matches!(true, true) && truth() {
133133
LL | | if matches!(true, true) {}

Diff for: ‎tests/ui/match_same_arms.rs

+22
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ macro_rules! m {
5757
(foo) => {};
5858
(bar) => {};
5959
}
60+
macro_rules! foo {
61+
() => {
62+
1
63+
};
64+
}
65+
macro_rules! bar {
66+
() => {
67+
1
68+
};
69+
}
6070

6171
fn main() {
6272
let x = 0;
@@ -111,4 +121,16 @@ fn main() {
111121
},
112122
_ => 0,
113123
};
124+
125+
let _ = match 0 {
126+
0 => foo!(),
127+
1 => bar!(),
128+
_ => 1,
129+
};
130+
131+
let _ = match 0 {
132+
0 => cfg!(not_enabled),
133+
1 => cfg!(also_not_enabled),
134+
_ => false,
135+
};
114136
}

Diff for: ‎tests/ui/match_same_arms2.rs

+6
Original file line numberDiff line numberDiff line change
@@ -239,4 +239,10 @@ fn main() {
239239
3 => core::convert::identity::<u32>(todo!()),
240240
_ => 5,
241241
};
242+
243+
let _ = match 0 {
244+
0 => cfg!(not_enable),
245+
1 => cfg!(not_enable),
246+
_ => false,
247+
};
242248
}

Diff for: ‎tests/ui/match_same_arms2.stderr

+16-1
Original file line numberDiff line numberDiff line change
@@ -192,5 +192,20 @@ note: other arm here
192192
LL | Some(Bar { x: 0, y: 5, .. }) => 1,
193193
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
194194

195-
error: aborting due to 12 previous errors
195+
error: this match arm has an identical body to another arm
196+
--> $DIR/match_same_arms2.rs:245:9
197+
|
198+
LL | 1 => cfg!(not_enable),
199+
| -^^^^^^^^^^^^^^^^^^^^
200+
| |
201+
| help: try merging the arm patterns: `1 | 0`
202+
|
203+
= help: or try changing either arm body
204+
note: other arm here
205+
--> $DIR/match_same_arms2.rs:244:9
206+
|
207+
LL | 0 => cfg!(not_enable),
208+
| ^^^^^^^^^^^^^^^^^^^^^
209+
210+
error: aborting due to 13 previous errors
196211

Diff for: ‎tests/ui/partialeq_to_none.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@run-rustfix
22
#![warn(clippy::partialeq_to_none)]
3+
#![allow(clippy::eq_op)]
34

45
struct Foobar;
56

Diff for: ‎tests/ui/partialeq_to_none.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@run-rustfix
22
#![warn(clippy::partialeq_to_none)]
3+
#![allow(clippy::eq_op)]
34

45
struct Foobar;
56

0 commit comments

Comments
 (0)