Add new lint [needless_parens_on_range_literals]

[DennisOSRM]

changelog: Adds a new lint [needless_parens_on_range_literals] to warn on needless braces on literals in a range statement

For example, the lint would catch

error: needless parenthesis on range literals can be removed
  --> $DIR/needless_parens_on_range_literals.rs:8:13
   |
LL |     let _ = ('a')..=('z');
   |             ^^^^^ help: try: `'a'`
   |
   = note: `-D clippy::needless-parens-on-range-literals` implied by `-D warnings`

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) soon.

Please see the contribution instructions for more information.

[ghost]

Not a review. Just some comments.

...

AFAIK, Rust uses the term parenthesis for (...) and braces for {...} so the name might be wrong.

Here is an example where the suggestion fails because the f64 has a trailing ..

fn main() {
    use rand::{thread_rng, Rng};
    
    let mut rng = thread_rng();
    
    let m: f64 = rng.gen_range((1.)..2.);
    println!("{}", m);
}

How does the lint handle macros? E.g. if the parens and literal are in a macro, does it recommend replacing the macro call with a literal? Maybe add some tests for that.

[Serial-ATA]

Welcome 🙂!

I'm not the reviewer, but I have some comments on top of @mikerite's.

[Serial-ATA]

It looks like you took the examples out of here and put them at the top of the file. They need to be within this macro for the docs to appear on the lint list.

[DennisOSRM]

Implemented in 8185ecd

[Serial-ATA]

What about the following cases:

let _ = ('a')..;
let _ = ..('z');

[DennisOSRM]

Implemented in 496155d

[Serial-ATA]

I think this is better expressed with snippet_with_applicability.

[DennisOSRM]

Implemented in afe37e1

[DennisOSRM]

Not a review. Just some comments.

...

AFAIK, Rust uses the term parenthesis for (...) and braces for {...} so the name might be wrong.

Here is an example where the suggestion fails because the f64 has a trailing ..

fn main() {
    use rand::{thread_rng, Rng};
    
    let mut rng = thread_rng();
    
    let m: f64 = rng.gen_range((1.)..2.);
    println!("{}", m);
}

How does the lint handle macros? E.g. if the parens and literal are in a macro, does it recommend replacing the macro call with a literal? Maybe add some tests for that.

That's a great suggestion. Thanks, will add it

[DennisOSRM]

Rewrote the check by impl LateLintPass. This gives access to type information and makes it easy to ignore parenthesis on a floating point start literal. The detection of parenthesis is done by inspecting the span of the corresponding expression and whether its length lines up with the length of the literal expression.

[dswij]

Thanks for the PR, and welcome!

1. The lint itself looks good, just some nits on spellings.
2. Can you do a rebase instead of merge commits? We follow a no-merge policy
3. We prefer plural form when naming lint. So the lint name would be needless_parens_on_range_literals. Can you also add the name to the changelog?

---

Also, huge thanks to @mikerite and @Serial-ATA for the suggestions and reviews!

[DennisOSRM]

Thanks for the PR, and welcome!

1. The lint itself looks good, just some nits on spellings.
2. Can you do a rebase instead of merge commits? We follow a no-merge policy
3. We prefer plural form when naming lint. So the lint name would be needless_parens_on_range_literals. Can you also add the name to the changelog?

Also, huge thanks to @mikerite and @Serial-ATA for the suggestions and reviews!

Thanks a lot. Did the following changes:

• Rebase. The merge commits are now gone.
• Followed your suggestion to use the plural form.
• CHANGELOG.md is updated.

[dswij]

Thanks for this! The lint looks great to me.

@bors r+

[bors]

📌 Commit 61d7dd2 has been approved by dswij

[bors]

⌛ Testing commit 61d7dd2 with merge 0f6e50f...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 0f6e50f to master...