Add new lint: match with a single binding statement

[ThibsG]

This lint catches match statements that binds to only one block and could be rewritten using a simple let statement.

Lint name: MATCH_SINGLE_BINDING

fixes: #3664

changelog: add lint for match with single binding statement

[flip1995]

IMO suggesting

let x = y;
{ 
    // match body using `x`
} 

Is the best thing we could do here. Detecting if the let binding is necessary after rewriting the match expression is to hard to do. And only suggesting the match body breaks the code, when auto fixed

[flip1995]

You can also use 2 different spans for the lint and the suggestion with span_lint_and_then. The first span is for the faulty code, the second for the code that will be replaced when applying the suggestion

[ThibsG]

Thanks a lot !
Indeed it's better now, thought I can do better about the auto fix it provides:

    let x = a;
{
    println!("{}", x);
}

I still need to fix let assignment based for this kind of match (infallible_destructuring_match lint colliding)

[flip1995]

Suggestion is fine as it is. Adding indentation to the suggestion is a bit out off scope for Clippy currently. This has to be handled by rustfmt afterwards.

I still need to fix let assignment based for this kind of match (infallible_destructuring_match lint colliding)

What you can do to fix is this is:

1. Move infallible_destructuring_match into the Matches lint pass, by copying the check_local method incl. lint declaration, ... to the matches.rs module.
2. Use impl_lint_pass! macro, instead of declare_lint_pass and define

   #[derive(Default)]
   pub struct Matches {
       infallible_destructuring_match_linted: bool // or a better name
   }

3. When the infallible_destructuring_match lint triggers, set self.infallible_destructuring_match_linted = true
4. Condition the check_match_single_binding(cx, ex, arms, expr); on self.infallible_destructuring_match_linted:

   if self.infallible_destructuring_match_linted {
       self.infallible_destructuring_match_linted = false;
   } else {
       check_match_single_binding(cx, ex, arms, expr);
   }

[flip1995]

With my suggestion below, you should be able to revert commit 6f7e035 without failing the dogfood test.

[bors]

☔ The latest upstream changes (presumably #4945) made this pull request unmergeable. Please resolve the merge conflicts.

[ThibsG]

Is there a way to make a clean distinction between:

match x {
    _ => println!("..."),
}

and

match x {
    _ => {
        println!("...");
    }
}

I don't like very much my solution to add semi-colon to body expression if needed (first println! in my case).

[flip1995]

You could just check if the match_body expression is of ExprKind::Block. If it is, you do not have to add a semicolon. If it is not, you have to add a semicolon, iff expr_ty(match_body) == ().

Let's go through the cases:

1. match _ { ... }; match with ; (fallback case): You just replace the match _ {}, so the ; is already there => all good
2. match _ { a => {} }:
   2a. match _ { a => { x; } }: match_body = "{ x; }" => all good
   2b. match _ { a => { x } }:
   2b(i). match_body = "x" && expr_ty(x) == (): Add a ; to the expression
   2b(ii). match_body = "x" && expr_ty(x) != (): The match expr either returns x or has a ; after it (1.) => all good
   2c. match _ { a => { x; ... y } }: match_body = "{ x; ... y }", either the match returns or (1.) => all good
3. match _ { a => x, }: Same as (2b.)

See cx.tables.expr_ty

[bors]

☔ The latest upstream changes (presumably #5098) made this pull request unmergeable. Please resolve the merge conflicts.

[flip1995]

Thanks! Can you squash some commits? After that, this should be ready to merge.

[ThibsG]

Here you go !

Thanks a lot for hints and patience 😆

[flip1995]

@bors r+

Thanks for the contribution!

[bors]

📌 Commit 00904cb has been approved by flip1995

[bors]

⌛ Testing commit 00904cb with merge 298df70...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 298df70 to master...