Skip to content

add structured suggestions to double_parens #15420

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.

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 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 24 additions & 10 deletions clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{HasSession, snippet_with_applicability};
use rustc_ast::ast::{Expr, ExprKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::declare_lint_pass;

Expand Down Expand Up @@ -40,28 +42,40 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);

impl EarlyLintPass for DoubleParens {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
let span = match &expr.kind {
ExprKind::Paren(in_paren) if matches!(in_paren.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => expr.span,
let (outer_span, inner_span) = match &expr.kind {
ExprKind::Paren(in_paren) => {
let inner_span = match &in_paren.kind {
ExprKind::Paren(inner) => inner.span,
ExprKind::Tup(_) => in_paren.span,
_ => return,
};
(expr.span, inner_span)
},
ExprKind::Call(_, params)
if let [param] = &**params
&& let ExprKind::Paren(_) = param.kind =>
&& let ExprKind::Paren(inner) = &param.kind =>
{
param.span
(param.span, inner.span)
},
ExprKind::MethodCall(call)
if let [arg] = &*call.args
&& let ExprKind::Paren(_) = arg.kind =>
&& let ExprKind::Paren(inner) = &arg.kind =>
{
arg.span
(arg.span, inner.span)
},
_ => return,
};
if !expr.span.from_expansion() {
span_lint(
let mut applicability = Applicability::MachineApplicable;
let sugg = snippet_with_applicability(cx.sess(), inner_span, "_", &mut applicability);
span_lint_and_sugg(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this should be span_lint_hir_and_then instead, since, in the (Method)Call scenario, we lint on the parenthesised expression inside the call, rather than the call itself

cx,
DOUBLE_PARENS,
span,
"consider removing unnecessary double parentheses",
outer_span,
"unnecessary parentheses",
"remove them",
sugg.to_string(),
applicability,
);
}
}
Expand Down
63 changes: 63 additions & 0 deletions tests/ui/double_parens.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#![warn(clippy::double_parens)]
#![allow(dead_code, clippy::eq_op)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip]

fn dummy_fn<T>(_: T) {}

struct DummyStruct;

impl DummyStruct {
fn dummy_method<T>(self, _: T) {}
}

fn simple_double_parens() -> i32 {
0
//~^ double_parens
}

fn fn_double_parens() {
dummy_fn(0);
//~^ double_parens
}

fn method_double_parens(x: DummyStruct) {
x.dummy_method(0);
//~^ double_parens
}

fn tuple_double_parens() -> (i32, i32) {
(1, 2)
//~^ double_parens
}

#[allow(clippy::unused_unit)]
fn unit_double_parens() {
()
//~^ double_parens
}

fn fn_tuple_ok() {
dummy_fn((1, 2));
}

fn method_tuple_ok(x: DummyStruct) {
x.dummy_method((1, 2));
}

fn fn_unit_ok() {
dummy_fn(());
}

fn method_unit_ok(x: DummyStruct) {
x.dummy_method(());
}

// Issue #3206
fn inside_macro() {
assert_eq!((1, 2), (1, 2), "Error");
assert_eq!((1, 2), (1, 2), "Error");
//~^ double_parens
}

fn main() {}
8 changes: 1 addition & 7 deletions tests/ui/double_parens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,27 @@ impl DummyStruct {
fn simple_double_parens() -> i32 {
((0))
//~^ double_parens


}

fn fn_double_parens() {
dummy_fn((0));
//~^ double_parens

}

fn method_double_parens(x: DummyStruct) {
x.dummy_method((0));
//~^ double_parens

}

fn tuple_double_parens() -> (i32, i32) {
((1, 2))
//~^ double_parens

}

#[allow(clippy::unused_unit)]
fn unit_double_parens() {
(())
//~^ double_parens

}

fn fn_tuple_ok() {
Expand All @@ -63,7 +58,6 @@ fn inside_macro() {
assert_eq!((1, 2), (1, 2), "Error");
assert_eq!(((1, 2)), (1, 2), "Error");
//~^ double_parens

}

fn main() {}
34 changes: 17 additions & 17 deletions tests/ui/double_parens.stderr
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
error: consider removing unnecessary double parentheses
error: unnecessary parentheses
--> tests/ui/double_parens.rs:15:5
|
LL | ((0))
| ^^^^^
| ^^^^^ help: remove them: `0`
|
= note: `-D clippy::double-parens` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::double_parens)]`

error: consider removing unnecessary double parentheses
--> tests/ui/double_parens.rs:22:14
error: unnecessary parentheses
--> tests/ui/double_parens.rs:20:14
|
LL | dummy_fn((0));
| ^^^
| ^^^ help: remove them: `0`

error: consider removing unnecessary double parentheses
--> tests/ui/double_parens.rs:28:20
error: unnecessary parentheses
--> tests/ui/double_parens.rs:25:20
|
LL | x.dummy_method((0));
| ^^^
| ^^^ help: remove them: `0`

error: consider removing unnecessary double parentheses
--> tests/ui/double_parens.rs:34:5
error: unnecessary parentheses
--> tests/ui/double_parens.rs:30:5
|
LL | ((1, 2))
| ^^^^^^^^
| ^^^^^^^^ help: remove them: `(1, 2)`

error: consider removing unnecessary double parentheses
--> tests/ui/double_parens.rs:40:5
error: unnecessary parentheses
--> tests/ui/double_parens.rs:36:5
|
LL | (())
| ^^^^
| ^^^^ help: remove them: `()`

error: consider removing unnecessary double parentheses
--> tests/ui/double_parens.rs:64:16
error: unnecessary parentheses
--> tests/ui/double_parens.rs:59:16
|
LL | assert_eq!(((1, 2)), (1, 2), "Error");
| ^^^^^^^^
| ^^^^^^^^ help: remove them: `(1, 2)`

error: aborting due to 6 previous errors