diff --git a/clippy_lints/src/needless_parens_on_range_literals.rs b/clippy_lints/src/needless_parens_on_range_literals.rs index 021a11593f3a..b26dee16036c 100644 --- a/clippy_lints/src/needless_parens_on_range_literals.rs +++ b/clippy_lints/src/needless_parens_on_range_literals.rs @@ -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; @@ -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, ); } } diff --git a/tests/ui/needless_parens_on_range_literals.fixed b/tests/ui/needless_parens_on_range_literals.fixed index 7abcbc0c6e32..491ebf70968f 100644 --- a/tests/ui/needless_parens_on_range_literals.fixed +++ b/tests/ui/needless_parens_on_range_literals.fixed @@ -1,7 +1,5 @@ -//@edition:2018 - #![warn(clippy::needless_parens_on_range_literals)] -#![allow(clippy::almost_complete_range)] +#![expect(clippy::almost_complete_range)] fn main() { let _ = 'a'..='z'; @@ -9,11 +7,62 @@ fn main() { //~| 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; } diff --git a/tests/ui/needless_parens_on_range_literals.rs b/tests/ui/needless_parens_on_range_literals.rs index 2a6f9305883b..4247c7b8c681 100644 --- a/tests/ui/needless_parens_on_range_literals.rs +++ b/tests/ui/needless_parens_on_range_literals.rs @@ -1,7 +1,5 @@ -//@edition:2018 - #![warn(clippy::needless_parens_on_range_literals)] -#![allow(clippy::almost_complete_range)] +#![expect(clippy::almost_complete_range)] fn main() { let _ = ('a')..=('z'); @@ -9,11 +7,62 @@ fn main() { //~| 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; } diff --git a/tests/ui/needless_parens_on_range_literals.stderr b/tests/ui/needless_parens_on_range_literals.stderr index 082b2cd8cade..8fffb77533c1 100644 --- a/tests/ui/needless_parens_on_range_literals.stderr +++ b/tests/ui/needless_parens_on_range_literals.stderr @@ -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'` @@ -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