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

Lint request: Prevent lifetimes that are only used for function output #1874

Open
KamilaBorowska opened this issue Jul 5, 2017 · 9 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types

Comments

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Jul 5, 2017

The following:

fn lifetime<'a>() -> &'a str {
    "Why some people do that"
}

Can be more clearly written as this. Also, using 'static lifetime makes it clear that it was intended, especially in unsafe code where returning unbound lifetime may as well be a mistake.

fn lifetime() -> &'static str {
    "Why some people do that"
}
@oli-obk oli-obk added L-correctness Lint: Belongs in the correctness lint group good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints T-middle Type: Probably requires verifiying types labels Jul 5, 2017
@clarfonthey
Copy link
Contributor

In the standard library the opposite is true; people tend to prefer the former style to the latter.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 5, 2017

Yea, the static lifetime might mess with some inference around multiple lifetimes. If this lint is deemed too controversial, it can always become a restriction lint or allow-by-default

@clarfonthey
Copy link
Contributor

I should also add that this only works for 'static types; for non-'static types you have to use the former style to get the same effect.

@KamilaBorowska
Copy link
Contributor Author

Oh, there is a difference. What about making this a case only for public non-unsafe functions where this is definitely a mistake.

@felix91gr
Copy link
Contributor

felix91gr commented Apr 7, 2020

Where are we with regards to this lint?

What about making this a case only for public non-unsafe functions where this is definitely a mistake.

If this is what we want, I reckon I can probably implement it :)

@oli-obk
Copy link
Contributor

oli-obk commented Apr 7, 2020

The only way we can implement it without false positives is to check that the return type "has a 'static" bound, which I'm not sure how to check reliably. I'm not sure if the complexity required for this lint is worth it considering the minor improvement in API that it brings.

@felix91gr
Copy link
Contributor

Should there be a needs-discussion label in this issue then, @oli-obk? Maybe we can close this one if the complexity is indeed too much for the improvement it brings :)

@clarfonthey
Copy link
Contributor

I personally think that a means to not have both -> &'b T and <'a: 'b> -> &'a T (the static case is just 'b: 'static, which would make the 'a: 'static bound redundant). The only cases I can think of where you can't explicitly name 'b is when you're creating a type inside a block, which should be relatively easy to check for and bail out of linting.

Dunno if it's a good idea to toss this yet, I just think that it's not something that is high on the priority list.

@oli-obk oli-obk added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Apr 8, 2020
@camsteffen
Copy link
Contributor

Similar to #305

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

7 participants