Skip to content
Merged
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
37 changes: 32 additions & 5 deletions clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{HasSession, snippet_with_applicability, snippet_with_context};
use clippy_utils::source::{HasSession, SpanRangeExt, snippet_with_applicability, snippet_with_context};
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
Expand Down Expand Up @@ -47,8 +47,12 @@ impl EarlyLintPass for DoubleParens {
// ^^^^^^ expr
// ^^^^ inner
ExprKind::Paren(inner) if matches!(inner.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => {
// suggest removing the outer parens
if expr.span.eq_ctxt(inner.span) {
if expr.span.eq_ctxt(inner.span)
&& !expr.span.in_external_macro(cx.sess().source_map())
&& check_source(cx, inner)
{
// suggest removing the outer parens

let mut applicability = Applicability::MachineApplicable;
// We don't need to use `snippet_with_context` here, because:
// - if `inner`'s `ctxt` is from macro, we don't lint in the first place (see the check above)
Expand All @@ -74,8 +78,12 @@ impl EarlyLintPass for DoubleParens {
if let [arg] = &**args
&& let ExprKind::Paren(inner) = &arg.kind =>
{
// suggest removing the inner parens
if expr.span.eq_ctxt(arg.span) {
if expr.span.eq_ctxt(arg.span)
&& !arg.span.in_external_macro(cx.sess().source_map())
&& check_source(cx, arg)
{
// suggest removing the inner parens

let mut applicability = Applicability::MachineApplicable;
let sugg = snippet_with_context(cx.sess(), inner.span, arg.span.ctxt(), "_", &mut applicability).0;
span_lint_and_sugg(
Expand All @@ -93,3 +101,22 @@ impl EarlyLintPass for DoubleParens {
}
}
}

/// Check that the span does indeed look like `( (..) )`
fn check_source(cx: &EarlyContext<'_>, inner: &Expr) -> bool {
if let Some(sfr) = inner.span.get_source_range(cx)
// this is the same as `SourceFileRange::as_str`, but doesn't apply the range right away, because
// we're interested in the source code outside it
&& let Some(src) = sfr.sf.src.as_ref().map(|src| src.as_str())
&& let Some((start, outer_after_inner)) = src.split_at_checked(sfr.range.end)
&& let Some((outer_before_inner, inner)) = start.split_at_checked(sfr.range.start)
&& outer_before_inner.trim_end().ends_with('(')
&& inner.starts_with('(')
&& inner.ends_with(')')
&& outer_after_inner.trim_start().starts_with(')')
{
true
} else {
false
}
}
12 changes: 12 additions & 0 deletions tests/ui/auxiliary/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,15 @@ macro_rules! bad_transmute {
std::mem::transmute($e)
};
}

#[macro_export]
#[rustfmt::skip]
macro_rules! double_parens {
($a:expr, $b:expr, $c:expr, $d:expr) => {{
let a = ($a);
let a = (());
let b = ((5));
let c = std::convert::identity((5));
InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32)
}};
}
11 changes: 11 additions & 0 deletions tests/ui/auxiliary/proc_macro_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,14 @@ pub fn allow_lint_same_span_derive(input: TokenStream) -> TokenStream {
span_help(Group::new(Delimiter::Brace, TokenStream::new()).into()),
])
}

#[proc_macro_derive(DoubleParens)]
pub fn derive_double_parens(_: TokenStream) -> TokenStream {
quote! {
fn foo() {
let a = (());
let b = ((5));
let c = std::convert::identity((5));
}
}
}
65 changes: 65 additions & 0 deletions tests/ui/double_parens.fixed
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
//@aux-build:proc_macros.rs
//@aux-build:proc_macro_derive.rs
//@aux-build:macro_rules.rs
#![warn(clippy::double_parens)]
#![expect(clippy::eq_op, clippy::no_effect)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip]

use proc_macros::{external, with_span};

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

struct DummyStruct;
Expand Down Expand Up @@ -96,4 +101,64 @@ fn issue9000(x: DummyStruct) {
//~^ double_parens
}

fn issue15892() {
use macro_rules::double_parens as double_parens_external;

macro_rules! double_parens{
($a:expr, $b:expr, $c:expr, $d:expr) => {{
let a = ($a);
let a = ();
//~^ double_parens
let b = (5);
//~^ double_parens
let c = std::convert::identity(5);
//~^ double_parens
InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32)
}};
}

// Don't lint: external macro
(external!((5)));
external!(((5)));

#[repr(transparent)]
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct InterruptMask(u32);

impl InterruptMask {
pub const OE: InterruptMask = InterruptMask(1 << 10);
pub const BE: InterruptMask = InterruptMask(1 << 9);
pub const PE: InterruptMask = InterruptMask(1 << 8);
pub const FE: InterruptMask = InterruptMask(1 << 7);
// Lint: internal macro
pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE);
// Don't lint: external macro
pub const F: InterruptMask = double_parens_external!((Self::OE), Self::BE, Self::PE, Self::FE);
#[allow(clippy::unnecessary_cast)]
pub const G: InterruptMask = external!(
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
);
#[allow(clippy::unnecessary_cast)]
// Don't lint: external proc-macro
pub const H: InterruptMask = with_span!(span
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
);
pub const fn into_bits(self) -> u32 {
self.0
}
#[must_use]
pub const fn union(self, rhs: Self) -> Self {
InterruptMask(self.0 | rhs.0)
}
}
}

fn issue15940() {
use proc_macro_derive::DoubleParens;

#[derive(DoubleParens)]
// Don't lint: external derive macro
pub struct Person;
}

fn main() {}
65 changes: 65 additions & 0 deletions tests/ui/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
//@aux-build:proc_macros.rs
//@aux-build:proc_macro_derive.rs
//@aux-build:macro_rules.rs
#![warn(clippy::double_parens)]
#![expect(clippy::eq_op, clippy::no_effect)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip]

use proc_macros::{external, with_span};

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

struct DummyStruct;
Expand Down Expand Up @@ -96,4 +101,64 @@ fn issue9000(x: DummyStruct) {
//~^ double_parens
}

fn issue15892() {
use macro_rules::double_parens as double_parens_external;

macro_rules! double_parens{
($a:expr, $b:expr, $c:expr, $d:expr) => {{
let a = ($a);
let a = (());
//~^ double_parens
let b = ((5));
//~^ double_parens
let c = std::convert::identity((5));
//~^ double_parens
InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32)
}};
}

// Don't lint: external macro
(external!((5)));
external!(((5)));

#[repr(transparent)]
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct InterruptMask(u32);

impl InterruptMask {
pub const OE: InterruptMask = InterruptMask(1 << 10);
pub const BE: InterruptMask = InterruptMask(1 << 9);
pub const PE: InterruptMask = InterruptMask(1 << 8);
pub const FE: InterruptMask = InterruptMask(1 << 7);
// Lint: internal macro
pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE);
// Don't lint: external macro
pub const F: InterruptMask = double_parens_external!((Self::OE), Self::BE, Self::PE, Self::FE);
#[allow(clippy::unnecessary_cast)]
pub const G: InterruptMask = external!(
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
);
#[allow(clippy::unnecessary_cast)]
// Don't lint: external proc-macro
pub const H: InterruptMask = with_span!(span
InterruptMask((((Self::OE.union(Self::BE).union(Self::PE).union(Self::FE))).into_bits()) as u32)
);
pub const fn into_bits(self) -> u32 {
self.0
}
#[must_use]
pub const fn union(self, rhs: Self) -> Self {
InterruptMask(self.0 | rhs.0)
}
}
}

fn issue15940() {
use proc_macro_derive::DoubleParens;

#[derive(DoubleParens)]
// Don't lint: external derive macro
pub struct Person;
}

fn main() {}
55 changes: 44 additions & 11 deletions tests/ui/double_parens.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unnecessary parentheses
--> tests/ui/double_parens.rs:15:5
--> tests/ui/double_parens.rs:20:5
|
LL | ((0))
| ^^^^^ help: remove them: `(0)`
Expand All @@ -8,37 +8,37 @@ LL | ((0))
= help: to override `-D warnings` add `#[allow(clippy::double_parens)]`

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

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

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

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

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

error: unnecessary parentheses
--> tests/ui/double_parens.rs:84:16
--> tests/ui/double_parens.rs:89:16
|
LL | () => {((100))}
| ^^^^^^^ help: remove them: `(100)`
Expand All @@ -49,22 +49,55 @@ LL | bar!();
= note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)

error: unnecessary parentheses
--> tests/ui/double_parens.rs:91:5
--> tests/ui/double_parens.rs:96:5
|
LL | ((vec![1, 2]));
| ^^^^^^^^^^^^^^ help: remove them: `(vec![1, 2])`

error: unnecessary parentheses
--> tests/ui/double_parens.rs:93:14
--> tests/ui/double_parens.rs:98:14
|
LL | dummy_fn((vec![1, 2]));
| ^^^^^^^^^^^^ help: remove them: `vec![1, 2]`

error: unnecessary parentheses
--> tests/ui/double_parens.rs:95:20
--> tests/ui/double_parens.rs:100:20
|
LL | x.dummy_method((vec![1, 2]));
| ^^^^^^^^^^^^ help: remove them: `vec![1, 2]`

error: aborting due to 10 previous errors
error: unnecessary parentheses
--> tests/ui/double_parens.rs:110:21
|
LL | let a = (());
| ^^^^ help: remove them: `()`
...
LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE);
| -------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info)

error: unnecessary parentheses
--> tests/ui/double_parens.rs:112:21
|
LL | let b = ((5));
| ^^^^^ help: remove them: `(5)`
...
LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE);
| -------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info)

error: unnecessary parentheses
--> tests/ui/double_parens.rs:114:44
|
LL | let c = std::convert::identity((5));
| ^^^ help: remove them: `5`
...
LL | pub const E: InterruptMask = double_parens!((Self::OE), Self::BE, Self::PE, Self::FE);
| -------------------------------------------------------- in this macro invocation
|
= note: this error originates in the macro `double_parens` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 13 previous errors