Skip to content

Overhaul needless_parens_on_range_literals #15717

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 3 commits into
base: master
Choose a base branch
from
+147 −33
Open

Overhaul needless_parens_on_range_literals #15717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83f2cd5
clean-up
ada4a Sep 19, 2025
114d4fa
fix FN on unsuffixed floats that don't end with `.`
ada4a Sep 19, 2025
fc1a9ef
fix FP with MBEs
ada4a Sep 19, 2025
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
38 changes: 18 additions & 20 deletions clippy_lints/src/needless_parens_on_range_literals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::source::{SpanRangeExt, snippet_with_context};

use rustc_ast::ast;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -40,34 +40,32 @@ declare_clippy_lint! {

declare_lint_pass!(NeedlessParensOnRangeLiterals => [NEEDLESS_PARENS_ON_RANGE_LITERALS]);

fn snippet_enclosed_in_parenthesis(snippet: &str) -> bool {
snippet.starts_with('(') && snippet.ends_with(')')
}

fn check_for_parens(cx: &LateContext<'_>, e: &Expr<'_>, is_start: bool) {
if is_start
if !e.span.from_expansion()
&& let ExprKind::Lit(literal) = e.kind
&& let ast::LitKind::Float(_sym, ast::LitFloatType::Unsuffixed) = literal.node
{
// don't check floating point literals on the start expression of a range
return;
}
if let ExprKind::Lit(literal) = e.kind
// the indicator that parenthesis surround the literal is that the span of the expression and the literal differ
&& (literal.span.data().hi - literal.span.data().lo) != (e.span.data().hi - e.span.data().lo)
&& literal.span != e.span
// inspect the source code of the expression for parenthesis
&& snippet_enclosed_in_parenthesis(&snippet(cx, e.span, ""))
&& e.span.check_source_text(cx, |s| s.starts_with('(') && s.ends_with(')'))
{
if is_start
&& let ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) = literal.node
&& literal.span.check_source_text(cx, |s| s.ends_with('.'))
{
// don't lint `(2.)..end`, since removing the parens would result in invalid syntax
return;
}

let mut applicability = Applicability::MachineApplicable;
span_lint_and_then(
let suggestion = snippet_with_context(cx, literal.span, e.span.ctxt(), "_", &mut applicability).0;
span_lint_and_sugg(
cx,
NEEDLESS_PARENS_ON_RANGE_LITERALS,
e.span,
"needless parenthesis on range literals can be removed",
|diag| {
let suggestion = snippet_with_applicability(cx, literal.span, "_", &mut applicability);
diag.span_suggestion(e.span, "try", suggestion, applicability);
},
"try",
suggestion.to_string(),
applicability,
);
}
}
Expand Down
55 changes: 52 additions & 3 deletions tests/ui/needless_parens_on_range_literals.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,68 @@
//@edition:2018

#![warn(clippy::needless_parens_on_range_literals)]
#![allow(clippy::almost_complete_range)]
#![expect(clippy::almost_complete_range)]

fn main() {
let _ = 'a'..='z';
//~^ needless_parens_on_range_literals
//~| needless_parens_on_range_literals
let _ = 'a'..'z';
//~^ needless_parens_on_range_literals

let _ = (1.)..2.;
let _ = (1.)..2.;
//~^ needless_parens_on_range_literals
let _ = 1.0..2.;
//~^ needless_parens_on_range_literals

let _ = 'a'..;
//~^ needless_parens_on_range_literals
let _ = ..'z';
//~^ needless_parens_on_range_literals

macro_rules! verbatim {
($e:expr) => {
$e
};
}
macro_rules! add_paren {
($e:expr) => {
($e)
};
}

// lint: the paren was added by the user
let _ = verbatim!(0)..1;
//~^ needless_parens_on_range_literals
let _ = 0..verbatim!(1);
//~^ needless_parens_on_range_literals

// lint: the macro doesn't have anything to do with the paren
// NOTE: This doesn't actually work currently.
//
// If we have a case like this:
// ```
// macro_rules! zero {
// () => {
// 0
// };
// }
// let _ = (zero!())..1;
// ```
// then the lint doesn't trigger, because the span comes from expansion.
// "But surely the outer `()` don't come from an expansion!", I hear you say. But the thing is that
// those parens aren't present in the HIR in the first place, and so the expression we actually
// end up looking at is `zero!()`, which is in fact an expansion.
//
// And if we have a case like this:
// ```
// let _ = (verbatim!(0))..1;
// ```
// then the parens get ignored as described above, but in addition to that, the whole `verbatim!(0)`
// gets replaced with just `0`, which is something that we've written, so its span now gets
// doubly-incorrectly identified as not coming from an expansion, and so the lint fires.
// But the problem arises when we get to creating the suggestion -- as it _also_ doesn't see the
// `verbatim!`, our suggestion ends up being just `let _ = 0..1;`, which is of course incorrect.

// don't lint: the paren was added by the macro
let _ = add_paren!(0)..1;
}
55 changes: 52 additions & 3 deletions tests/ui/needless_parens_on_range_literals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,68 @@
//@edition:2018

#![warn(clippy::needless_parens_on_range_literals)]
#![allow(clippy::almost_complete_range)]
#![expect(clippy::almost_complete_range)]

fn main() {
let _ = ('a')..=('z');
//~^ needless_parens_on_range_literals
//~| needless_parens_on_range_literals
let _ = 'a'..('z');
//~^ needless_parens_on_range_literals

let _ = (1.)..2.;
let _ = (1.)..(2.);
//~^ needless_parens_on_range_literals
let _ = (1.0)..2.;
//~^ needless_parens_on_range_literals

let _ = ('a')..;
//~^ needless_parens_on_range_literals
let _ = ..('z');
//~^ needless_parens_on_range_literals

macro_rules! verbatim {
($e:expr) => {
$e
};
}
macro_rules! add_paren {
($e:expr) => {
($e)
};
}

// lint: the paren was added by the user
let _ = verbatim!((0))..1;
//~^ needless_parens_on_range_literals
let _ = 0..verbatim!((1));
//~^ needless_parens_on_range_literals

// lint: the macro doesn't have anything to do with the paren
// NOTE: This doesn't actually work currently.
//
// If we have a case like this:
// ```
// macro_rules! zero {
// () => {
// 0
// };
// }
// let _ = (zero!())..1;
// ```
// then the lint doesn't trigger, because the span comes from expansion.
// "But surely the outer `()` don't come from an expansion!", I hear you say. But the thing is that
// those parens aren't present in the HIR in the first place, and so the expression we actually
// end up looking at is `zero!()`, which is in fact an expansion.
//
// And if we have a case like this:
// ```
// let _ = (verbatim!(0))..1;
// ```
// then the parens get ignored as described above, but in addition to that, the whole `verbatim!(0)`
// gets replaced with just `0`, which is something that we've written, so its span now gets
// doubly-incorrectly identified as not coming from an expansion, and so the lint fires.
// But the problem arises when we get to creating the suggestion -- as it _also_ doesn't see the
// `verbatim!`, our suggestion ends up being just `let _ = 0..1;`, which is of course incorrect.

// don't lint: the paren was added by the macro
let _ = add_paren!(0)..1;
}
32 changes: 25 additions & 7 deletions tests/ui/needless_parens_on_range_literals.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:7:13
--> tests/ui/needless_parens_on_range_literals.rs:5:13
|
LL | let _ = ('a')..=('z');
| ^^^^^ help: try: `'a'`
Expand All @@ -8,34 +8,52 @@ LL | let _ = ('a')..=('z');
= help: to override `-D warnings` add `#[allow(clippy::needless_parens_on_range_literals)]`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:7:21
--> tests/ui/needless_parens_on_range_literals.rs:5:21
|
LL | let _ = ('a')..=('z');
| ^^^^^ help: try: `'z'`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:10:18
--> tests/ui/needless_parens_on_range_literals.rs:8:18
|
LL | let _ = 'a'..('z');
| ^^^^^ help: try: `'z'`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:13:19
--> tests/ui/needless_parens_on_range_literals.rs:12:19
|
LL | let _ = (1.)..(2.);
| ^^^^ help: try: `2.`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:15:13
--> tests/ui/needless_parens_on_range_literals.rs:14:13
|
LL | let _ = (1.0)..2.;
| ^^^^^ help: try: `1.0`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:17:13
|
LL | let _ = ('a')..;
| ^^^^^ help: try: `'a'`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:17:15
--> tests/ui/needless_parens_on_range_literals.rs:19:15
|
LL | let _ = ..('z');
| ^^^^^ help: try: `'z'`

error: aborting due to 6 previous errors
error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:34:23
|
LL | let _ = verbatim!((0))..1;
| ^^^ help: try: `0`

error: needless parenthesis on range literals can be removed
--> tests/ui/needless_parens_on_range_literals.rs:36:26
|
LL | let _ = 0..verbatim!((1));
| ^^^ help: try: `1`

error: aborting due to 9 previous errors