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 Suggestion: reimplementing .and_then with Some(x?) #6410

Closed
NoraCodes opened this issue Dec 1, 2020 · 5 comments · Fixed by #6507
Closed

Lint Suggestion: reimplementing .and_then with Some(x?) #6410

NoraCodes opened this issue Dec 1, 2020 · 5 comments · Fixed by #6507
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@NoraCodes
Copy link

What it does

Suggest option.and_then(|value| value.inner_option) instead of option.and_then(|value| Some(value.inner_option?))

Categories (optional)

  • Kind: style or complexity

What is the advantage of the recommended code over the original code

There's no reason to take an option, wrap it in another option, and use ? to short-circuit. and_then will work fine without it, and it's needlessly difficult to read.

Drawbacks

None.

Example

struct Outer {
    inner: Option<String>
}

let a = Some(Outer { inner: Some("hello".into()) });
a.and_then(|a| Some(a.inner?))

Could be written as:

struct Outer {
    inner: Option<String>
}

let a = Some(Outer { inner: Some("hello".into()) });
a.and_then(|a| a.inner)
@NoraCodes NoraCodes added the A-lint Area: New lints label Dec 1, 2020
@flip1995 flip1995 added L-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions good-first-issue These issues are a good way to get started with Clippy labels Dec 2, 2020
@camsteffen
Copy link
Contributor

Nice. This can include any case of return Some(option?) or return Ok(result?). Maybe call it needless_question_mark.

@bengsparks
Copy link
Contributor

If no one has attempted this yet, I'd like to give it a shot.

@bengsparks
Copy link
Contributor

Ok, so let me see if I've understood this correctly.

What needs to be recognized;
Option<T> and Result<T, E>, i.e. types for which the question mark operator are implemented for.

How can it be matched:
By matching Some(instance_of_option?) and Ok(instance_of_result?).

What help can be suggested:
The instance_of_* parameter be kept and the rest discarded.

Have I overseen anything?

@ghost
Copy link

ghost commented Dec 25, 2020

You have to worry about the early exit of ?. This is probably only applicable on the last expression of a body or in a return statement.

bengsparks added a commit to bengsparks/rust-clippy that referenced this issue Dec 26, 2020
bengsparks added a commit to bengsparks/rust-clippy that referenced this issue Dec 26, 2020
TODO: The suggestion message still contains the ?
Issue rust-lang#6410
bengsparks added a commit to bengsparks/rust-clippy that referenced this issue Dec 26, 2020
bengsparks added a commit to bengsparks/rust-clippy that referenced this issue Dec 26, 2020
TODO: The suggestion message still contains the ?
Issue rust-lang#6410
bengsparks added a commit to bengsparks/rust-clippy that referenced this issue Dec 26, 2020
@bengsparks
Copy link
Contributor

Ok, I think I've got it done. I'll go open a PR.

bors added a commit that referenced this issue Jan 4, 2021
Needless Question Mark Lint

Fixes #6410, i.e the needless question mark lint
@bors bors closed this as completed in ae9ae97 Jan 4, 2021
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-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants