Skip to content
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

Separate lint needless_lifetimes for types with multiple lifetimes #12495

Open
antoyo opened this issue Mar 15, 2024 · 5 comments
Open

Separate lint needless_lifetimes for types with multiple lifetimes #12495

antoyo opened this issue Mar 15, 2024 · 5 comments
Assignees
Labels
A-lint Area: New lints

Comments

@antoyo
Copy link

antoyo commented Mar 15, 2024

What it does

I'd like to propose to make the lint needless_lifetimes only warn for references (and possibly types with a single lifetime) and add a new separate lint like needless_lifetimes, but that warns for types with multiple lifetimes (where at least one could be elided).

An alternative could to be not warn about needless_lifetimes when the lifetimes contain multiple characters (like 'stuff).

Advantage

For types with multiple lifetimes, it might help readability to keep the lifetimes in the code even if they could be elided.
So, by having a new separate lint, users who prefer to keep lifetimes on types with multiple lifetimes, they could disable this new lint and keep the ability to get warned about needless_lifetimes for simple cases.

Drawbacks

None since the choice would be up to the user.

Example

struct Test<'a, 'b> {
    a: &'a i32,
    b: &'b i32,
}

fn test<'a, 'b>(test: Test<'a, 'b>) -> &'a i32 {
    test.a
}

Could be written as:

fn test<'a>(test: Test<'a, '_>) -> &'a i32 {
    test.a
}
@m-rph
Copy link
Contributor

m-rph commented Apr 1, 2024

@rustbot claim

@m-rph
Copy link
Contributor

m-rph commented Apr 5, 2024

@antoyo instead of providing a separate lint, I would like to introduce a configuration in this lint, allowing you to toggle this functionality as follows:

  • both: needless_lifetimes will trigger for single LT and multiple LT
  • single-lt: needless_lifetimes will trigger for single LT
  • multiple-lt: needless_lifetimes will trigger only when multiple LTs exist but not for cases with a single LT that could be elided.

@antoyo
Copy link
Author

antoyo commented Apr 5, 2024

I didn't know lints could be configured.
Are you talking about this?
If so, there's this note:

Note: The configuration file is unstable and may be deprecated in the future.

Not sure if this going to be a problem.

@m-rph
Copy link
Contributor

m-rph commented Apr 5, 2024

Yes, that. We discussed this a bit on zulip and the consensus is that this lint and suggestion are good candidates for this.

@antoyo
Copy link
Author

antoyo commented Apr 5, 2024

I'm good with this.
Thanks for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants