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

single_match should suggest plain if when possible #173

Closed
Manishearth opened this issue Aug 15, 2015 · 4 comments · Fixed by #6574
Closed

single_match should suggest plain if when possible #173

Manishearth opened this issue Aug 15, 2015 · 4 comments · Fixed by #6574
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types

Comments

@Manishearth
Copy link
Member

Currently, for things like:

match foo
  Enum::Variant => {...}
  _ => {...}
}

we suggest using if let Enum::Variant = foo {...}. However, in many cases, the enum implements PartialEq, so we can just suggest if Enum::Variant == foo.

This involves checking if the patterns in the match are simple PatIdents or PatEnums with empty pat-vecs (http://manishearth.github.io/rust-internals-docs/syntax/ast/enum.Pat_.html), and then checking if the type implements Eq (not sure how this can be done, probably needs some mucking about in middle::ty)

@Manishearth Manishearth added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Aug 15, 2015
@llogiq
Copy link
Contributor

llogiq commented Aug 15, 2015

Is there any downside in using if let as opposed to equality? Performance-wise?

Also equality may be implemented differently (e.g. broader than a match) – we should probably tread carefully here.

@birkenfeld
Copy link
Contributor

I agree, we should not propose changes that could alter semantics.

@llogiq
Copy link
Contributor

llogiq commented Aug 15, 2015

We could note the alternative but caution that semantics may be different depending on the PartialEq implementation.

I note that there is a case where this tripped me in eq_op.

@llogiq
Copy link
Contributor

llogiq commented Aug 17, 2015

Also the same blacklist approach as in #181 could apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants