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

New lint: Unnecessarily complex types #4844

Open
jyn514 opened this issue Nov 24, 2019 · 4 comments
Open

New lint: Unnecessarily complex types #4844

jyn514 opened this issue Nov 24, 2019 · 4 comments
Labels
A-lint Area: New lints E-hard Call for participation: This a hard problem and requires more experience or effort to work on L-unnecessary Lint: Warn about unnecessary code

Comments

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2019

It would be nice to lint on return types that are more complex than necessary. Take for example the following function (playground):

fn unnecessary_result() -> Result<usize, ()> {
    Ok(1)
}

The Result here is not necessary, because the function always returns an Ok variant.
I would also like to see a similar lint for fn f() -> Option<T> where f always returns Some.

Note this is not the same as https://rust-lang.github.io/rust-clippy/current/index.html#type_complexity, which is about return types that are hard to read, not return types that are unnecessary.

Possible output for the lint (just making this up here):

warning: unnecessary `Result` in return type
 --> src/lib.rs:1:1
   |
1  |  fn unnecessary_result() -> Result<usize, ()> {
   |                             ^^^^^^^^^^^^^^^^^ help: try removing the `Result` type: `usize`
2  |         Ok(1)
   |         ^^^^^ note: only return value is an `Ok` variant
@jfrimmel
Copy link

Sometimes such a construct is necessary, e.g. to stay SemVer-compatible (e.g. you changed the internals of a function, so that it now cannot fail anymore, but your users still expect a Result/Option).
Another possibility is, that you want to be future-proof: currently the implementation cannot fail, but it is expected to be so in the future.
I still think, that this might be a useful lint, but the SemVer hazard should be documented in "known problems".

@jyn514
Copy link
Member Author

jyn514 commented Nov 24, 2019

Maybe it could be 'allow' by default and you'd have to explicitly set it to warn.

@flip1995
Copy link
Member

Definitely an allow-by-default lint. It may be hard to determine all the return points in the function and find out what exactly is returned. Making this generic over multiple types may also be quite hard.

@flip1995 flip1995 added L-unnecessary Lint: Warn about unnecessary code E-hard Call for participation: This a hard problem and requires more experience or effort to work on A-lint Area: New lints labels Nov 25, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

unnecessary_filter_map already does something similar. It warns if the closure never returns None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-hard Call for participation: This a hard problem and requires more experience or effort to work on L-unnecessary Lint: Warn about unnecessary code
Projects
None yet
Development

No branches or pull requests

3 participants