Skip to content

Commit b613be2

Browse files
committed
fix a bug while we're at it
1 parent 82b2eb1 commit b613be2

File tree

4 files changed

+136
-32
lines changed

4 files changed

+136
-32
lines changed

clippy_lints/src/double_parens.rs

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::{HasSession, snippet_with_applicability};
2+
use clippy_utils::source::{HasSession, snippet_with_applicability, snippet_with_context};
33
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
44
use rustc_errors::Applicability;
55
use rustc_lint::{EarlyContext, EarlyLintPass};
@@ -42,27 +42,28 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);
4242

4343
impl EarlyLintPass for DoubleParens {
4444
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
45-
if expr.span.from_expansion() {
46-
return;
47-
}
48-
4945
match &expr.kind {
5046
// ((..))
5147
// ^^^^^^ expr
5248
// ^^^^ inner
5349
ExprKind::Paren(inner) if matches!(inner.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => {
5450
// suggest removing the outer parens
55-
let mut applicability = Applicability::MachineApplicable;
56-
let sugg = snippet_with_applicability(cx.sess(), inner.span, "_", &mut applicability);
57-
span_lint_and_sugg(
58-
cx,
59-
DOUBLE_PARENS,
60-
expr.span,
61-
"unnecessary parentheses",
62-
"remove them",
63-
sugg.to_string(),
64-
applicability,
65-
);
51+
if expr.span.eq_ctxt(inner.span) {
52+
let mut applicability = Applicability::MachineApplicable;
53+
// We don't need to use `snippet_with_context` here, because:
54+
// - if `inner`'s `ctxt` is from macro, we don't lint in the first place (see the check above)
55+
// - otherwise, calling `snippet_with_applicability` on a not-from-macro span is fine
56+
let sugg = snippet_with_applicability(cx.sess(), inner.span, "_", &mut applicability);
57+
span_lint_and_sugg(
58+
cx,
59+
DOUBLE_PARENS,
60+
expr.span,
61+
"unnecessary parentheses",
62+
"remove them",
63+
sugg.to_string(),
64+
applicability,
65+
);
66+
}
6667
},
6768

6869
// func((n))
@@ -74,17 +75,19 @@ impl EarlyLintPass for DoubleParens {
7475
&& let ExprKind::Paren(inner) = &arg.kind =>
7576
{
7677
// suggest removing the inner parens
77-
let mut applicability = Applicability::MachineApplicable;
78-
let sugg = snippet_with_applicability(cx.sess(), inner.span, "_", &mut applicability);
79-
span_lint_and_sugg(
80-
cx,
81-
DOUBLE_PARENS,
82-
arg.span,
83-
"unnecessary parentheses",
84-
"remove them",
85-
sugg.to_string(),
86-
applicability,
87-
);
78+
if expr.span.eq_ctxt(arg.span) {
79+
let mut applicability = Applicability::MachineApplicable;
80+
let sugg = snippet_with_context(cx.sess(), inner.span, arg.span.ctxt(), "_", &mut applicability).0;
81+
span_lint_and_sugg(
82+
cx,
83+
DOUBLE_PARENS,
84+
arg.span,
85+
"unnecessary parentheses",
86+
"remove them",
87+
sugg.to_string(),
88+
applicability,
89+
);
90+
}
8891
},
8992
_ => {},
9093
}

tests/ui/double_parens.fixed

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::double_parens)]
2-
#![expect(clippy::eq_op)]
2+
#![expect(clippy::eq_op, clippy::no_effect)]
33
#![feature(custom_inner_attributes)]
44
#![rustfmt::skip]
55

@@ -8,7 +8,7 @@ fn dummy_fn<T>(_: T) {}
88
struct DummyStruct;
99

1010
impl DummyStruct {
11-
fn dummy_method<T>(self, _: T) {}
11+
fn dummy_method<T>(&self, _: T) {}
1212
}
1313

1414
fn simple_double_parens() -> i32 {
@@ -60,4 +60,40 @@ fn inside_macro() {
6060
//~^ double_parens
6161
}
6262

63+
fn issue9000(x: DummyStruct) {
64+
macro_rules! foo {
65+
() => {(100)}
66+
}
67+
// don't lint: the inner paren comes from the macro expansion
68+
(foo!());
69+
dummy_fn(foo!());
70+
x.dummy_method(foo!());
71+
72+
macro_rules! baz {
73+
($n:literal) => {($n)}
74+
}
75+
// don't lint: don't get confused by the expression inside the inner paren
76+
// having the same `ctxt` as the overall expression
77+
// (this is a bug that happened during the development of the fix)
78+
(baz!(100));
79+
dummy_fn(baz!(100));
80+
x.dummy_method(baz!(100));
81+
82+
// should lint: both parens are from inside the macro
83+
macro_rules! bar {
84+
() => {(100)}
85+
//~^ double_parens
86+
}
87+
bar!();
88+
89+
// should lint: both parens are from outside the macro;
90+
// make sure to suggest the macro unexpanded
91+
(vec![1, 2]);
92+
//~^ double_parens
93+
dummy_fn(vec![1, 2]);
94+
//~^ double_parens
95+
x.dummy_method(vec![1, 2]);
96+
//~^ double_parens
97+
}
98+
6399
fn main() {}

tests/ui/double_parens.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::double_parens)]
2-
#![expect(clippy::eq_op)]
2+
#![expect(clippy::eq_op, clippy::no_effect)]
33
#![feature(custom_inner_attributes)]
44
#![rustfmt::skip]
55

@@ -8,7 +8,7 @@ fn dummy_fn<T>(_: T) {}
88
struct DummyStruct;
99

1010
impl DummyStruct {
11-
fn dummy_method<T>(self, _: T) {}
11+
fn dummy_method<T>(&self, _: T) {}
1212
}
1313

1414
fn simple_double_parens() -> i32 {
@@ -60,4 +60,40 @@ fn inside_macro() {
6060
//~^ double_parens
6161
}
6262

63+
fn issue9000(x: DummyStruct) {
64+
macro_rules! foo {
65+
() => {(100)}
66+
}
67+
// don't lint: the inner paren comes from the macro expansion
68+
(foo!());
69+
dummy_fn(foo!());
70+
x.dummy_method(foo!());
71+
72+
macro_rules! baz {
73+
($n:literal) => {($n)}
74+
}
75+
// don't lint: don't get confused by the expression inside the inner paren
76+
// having the same `ctxt` as the overall expression
77+
// (this is a bug that happened during the development of the fix)
78+
(baz!(100));
79+
dummy_fn(baz!(100));
80+
x.dummy_method(baz!(100));
81+
82+
// should lint: both parens are from inside the macro
83+
macro_rules! bar {
84+
() => {((100))}
85+
//~^ double_parens
86+
}
87+
bar!();
88+
89+
// should lint: both parens are from outside the macro;
90+
// make sure to suggest the macro unexpanded
91+
((vec![1, 2]));
92+
//~^ double_parens
93+
dummy_fn((vec![1, 2]));
94+
//~^ double_parens
95+
x.dummy_method((vec![1, 2]));
96+
//~^ double_parens
97+
}
98+
6399
fn main() {}

tests/ui/double_parens.stderr

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,34 @@ error: unnecessary parentheses
3737
LL | assert_eq!(((1, 2)), (1, 2), "Error");
3838
| ^^^^^^^^ help: remove them: `(1, 2)`
3939

40-
error: aborting due to 6 previous errors
40+
error: unnecessary parentheses
41+
--> tests/ui/double_parens.rs:84:16
42+
|
43+
LL | () => {((100))}
44+
| ^^^^^^^ help: remove them: `(100)`
45+
...
46+
LL | bar!();
47+
| ------ in this macro invocation
48+
|
49+
= note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
50+
51+
error: unnecessary parentheses
52+
--> tests/ui/double_parens.rs:91:5
53+
|
54+
LL | ((vec![1, 2]));
55+
| ^^^^^^^^^^^^^^ help: remove them: `(vec![1, 2])`
56+
57+
error: unnecessary parentheses
58+
--> tests/ui/double_parens.rs:93:14
59+
|
60+
LL | dummy_fn((vec![1, 2]));
61+
| ^^^^^^^^^^^^ help: remove them: `vec![1, 2]`
62+
63+
error: unnecessary parentheses
64+
--> tests/ui/double_parens.rs:95:20
65+
|
66+
LL | x.dummy_method((vec![1, 2]));
67+
| ^^^^^^^^^^^^ help: remove them: `vec![1, 2]`
68+
69+
error: aborting due to 10 previous errors
4170

0 commit comments

Comments
 (0)