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

Warn on leading pipes in patterns #10717

Closed
LeoniePhiline opened this issue Apr 25, 2023 · 6 comments
Closed

Warn on leading pipes in patterns #10717

LeoniePhiline opened this issue Apr 25, 2023 · 6 comments
Assignees
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-restriction Lint: Belongs in the restriction lint group

Comments

@LeoniePhiline
Copy link

LeoniePhiline commented Apr 25, 2023

What it does

Warns on a |x| x pattern, which looks like a closure.

Lint Name

pattern_leading_pipe

Category

suspicious

Advantage

  • Avoids confusion of "or" pattern having a leading pipe (which is discarded), with a closure, enclosing arguments in pipes.

Drawbacks

None

Example

    let (|x| x) = match x(..) {
        |_| Some(2) => |_| Some(3),
        |_| _ => unreachable!(),
    };
    assert!(matches!(x(..), |_| Some(4)));

(borrowed from weird-exprs.rs)

Could be written as:

    let (x | x) = match x(..) {
        _ | Some(2) => |_| Some(3),
        _ | _ => unreachable!(),
    };
    assert!(matches!(x(..), _ | Some(4)));
@LeoniePhiline LeoniePhiline added the A-lint Area: New lints label Apr 25, 2023
@LeoniePhiline
Copy link
Author

@rustbot claim

@llogiq llogiq added good-first-issue These issues are a good way to get started with Clippy L-restriction Lint: Belongs in the restriction lint group labels Apr 25, 2023
@LeoniePhiline
Copy link
Author

Covered by rustfmt.

Should be closed?

@Alexendoo
Copy link
Member

Yeah, no need for a lint if rustfmt has it covered

@llogiq
Copy link
Contributor

llogiq commented Apr 26, 2023

I think we still might lint this in a matches!(..) macro invocation, because rustfmt doesn't cover macros. We'd still need to check if the pattern comes from another expansion, but otherwise it would be an OK lint. I think the category should still be pedantic or style rather than suspicious. Also we may try not to lint if the pattern spans multiple lines where each line starts with a pipe symbol (something I hear Haskell programmers are quite fond of).

So I think we may reopen this, but it's certainly no longer an easy issue.

@llogiq llogiq reopened this Apr 26, 2023
@llogiq llogiq added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. and removed good-first-issue These issues are a good way to get started with Clippy labels Apr 26, 2023
@Alexendoo
Copy link
Member

There's an open PR on the rustfmt side to fix the matches! case - rust-lang/rustfmt#5554

@llogiq
Copy link
Contributor

llogiq commented Apr 26, 2023

Ah, I had overlooked that. In that case, it's probably not a good idea to invest in this.

@llogiq llogiq closed this as completed Apr 26, 2023
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-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-restriction Lint: Belongs in the restriction lint group
Projects
None yet
Development

No branches or pull requests

3 participants