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

Suggest use of Option::filter() #8822

Closed
tomers opened this issue May 12, 2022 · 2 comments · Fixed by #9451
Closed

Suggest use of Option::filter() #8822

tomers opened this issue May 12, 2022 · 2 comments · Fixed by #9451
Assignees
Labels
A-lint Area: New lints

Comments

@tomers
Copy link

tomers commented May 12, 2022

What it does

I stumbled upon cases when developers are not familiar enough with all the functions that come with standard libs, such as Option::filter(). Instead, they program that functionality each time, which makes the code much more complex, error prone, and less readable.

Lint Name

filter

Category

style, complexity

Advantage

  • Reduces complexity substantially.
  • Improves code readability.
  • Makes use of lib function instead of coding its functionality in each usage.

Drawbacks

None that I can think of.

Example

match s {
    None => None,
    Some(x) => {
        if x.is_empty() {
            None
        } else {
            Some(x)
        }
    }
}

Could be written as:

s.filter(|x| !x.is_empty())

Full example:

fn bad(s: Option<String>) -> Option<String> {
    match s {
        None => None,
        Some(x) => {
            if x.is_empty() {
                None
            } else {
                Some(x)
            }
        }
    }
}

fn good(s: Option<String>) -> Option<String> {
    s.filter(|x| !x.is_empty())
}

fn main() {
    assert_eq!(bad(None), None);
    assert_eq!(bad(Some("".to_owned())), None);
    assert_eq!(bad(Some("foo".to_owned())), Some("foo".to_owned()));
    assert_eq!(good(None), None);
    assert_eq!(good(Some("".to_owned())), None);
    assert_eq!(good(Some("foo".to_owned())), Some("foo".to_owned()));
}
@tomers tomers added the A-lint Area: New lints label May 12, 2022
@kraktus
Copy link
Contributor

kraktus commented Sep 5, 2022

@rustbot claim

@kraktus
Copy link
Contributor

kraktus commented Sep 5, 2022

Another example while I can think about it

option match {
      Some(x) if cond(x) => x,
      _ => x
}

kraktus added a commit to kraktus/rust-clippy that referenced this issue Sep 9, 2022
Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback

close rust-lang#8822
@bors bors closed this as completed in 292e313 Oct 8, 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