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

Introduce a lint for single letter lifetimes #8233

Closed
cameron1024 opened this issue Jan 6, 2022 · 1 comment
Closed

Introduce a lint for single letter lifetimes #8233

cameron1024 opened this issue Jan 6, 2022 · 1 comment
Assignees
Labels
A-lint Area: New lints

Comments

@cameron1024
Copy link
Contributor

cameron1024 commented Jan 6, 2022

What it does

Highlights when a named lifetime is introduced that is a single character long. This feels like a very opinionated lint, and definitely overly pedantic in many cases, but I think there could be contexts where it's useful.

For example, I had to explain what the following impl meant in our Rocket web service:

impl<'r, 'o: 'r> Responder<'r, 'o> for Error {
    // ...
}

After reading the docs, I found a comment saying that 'r was the lifetime of the request, and 'o was the lifetime of the response. Admittedly this is somewhat intuitive by looking at the uses of the references in the type signature of the function in the trait, but I can't help but feel if it was instead written as: impl <'req, 'res: 'req> ..., I wouldn't even have to look at the docs to understand, which is significantly more valuable to those less familiar with Rust and borrowing/lifetimes.

Lint Name

single_char_lifetime_name

Category

pedantic

Advantage

Easier to understand meaning of a specific named lifetime, especially if new to Rust

Drawbacks

Likely overly pedantic in some codebases/teams/contexts

  • some contexts have only one lifetime that is sensible, and a single letter lifetime would be fine, e.g.:
struct Foo<'foo> {
  inner: &'foo str
}

is not (in my opinion) more readable/understandable than:

struct Foo<'a> {
  inner: &'a str
}
  • some teams/companies are familiar enough with the language that reading function signatures to work out the meaning of lifetimes doesn't add significant overhead

Example

fn dispatch<'r, 'o: 'r>(req: &'r Request) -> &'o Repsonse {
  todo!()
}

Could be written as:

fn dispatch<'req, 'res: 'req>(req: &'req Request) -> &'res Repsonse {
  todo!()
}
@cameron1024 cameron1024 added the A-lint Area: New lints label Jan 6, 2022
@PatchMixolydic
Copy link
Contributor

PatchMixolydic commented Jan 6, 2022

I happen to have already written this lint for a personal Dylint library, so I'd be glad to try porting it to Clippy. Not sure if rustbot is active here, but just in case...
@rustbot claim

bors added a commit that referenced this issue Jan 9, 2022
…ogiq

new lint: `single_char_lifetime_names`

This pull request adds a lint against single character lifetime names, as they might not divulge enough information about the purpose of the lifetime. This can make code harder to understand. I placed this in `restriction` rather than `pedantic` (as suggested in #8233) since most of the Rust ecosystem already uses single character lifetime names (to my knowledge, at least) and since single character lifetime names aren't incorrect. I'd be happy to change this upon request, however. Fixes #8233.

- [x] Followed lint naming conventions
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `cargo dev update_lints`
- [x] Added lint documentation
- [x] Run `cargo dev fmt`

changelog: new lint: [`single_char_lifetime_names`]
@bors bors closed this as completed in a6f80fc Jan 9, 2022
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