Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement lint for destructuring tuple structs with a let and a match (…
…closes #2671)
- Loading branch information
1 parent
ae32137
commit 3c38a36
Showing
4 changed files
with
194 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
use super::utils::{get_arg_name, match_var, remove_blocks, snippet, span_lint_and_sugg}; | ||
use rustc::hir::*; | ||
use rustc::lint::*; | ||
|
||
/// **What it does:** Checks for matches being used to destructure a single-variant enum | ||
/// or tuple struct where a `let` will suffice. | ||
/// | ||
/// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does. | ||
/// | ||
/// **Known problems:** None. | ||
/// | ||
/// **Example:** | ||
/// ```rust | ||
/// enum Wrapper { | ||
/// Data(i32), | ||
/// } | ||
/// | ||
/// let wrapper = Wrapper::Data(42); | ||
/// | ||
/// let data = match wrapper { | ||
/// Wrapper::Data(i) => i, | ||
/// }; | ||
/// ``` | ||
/// | ||
/// The correct use would be: | ||
/// ```rust | ||
/// enum Wrapper { | ||
/// Data(i32), | ||
/// } | ||
/// | ||
/// let wrapper = Wrapper::Data(42); | ||
/// let Wrapper::Data(data) = wrapper; | ||
/// ``` | ||
declare_clippy_lint! { | ||
pub INFALLIBLE_DESTRUCTURING_MATCH, | ||
style, | ||
"a match statement with a single infallible arm instead of a `let`" | ||
} | ||
|
||
#[derive(Copy, Clone, Default)] | ||
pub struct Pass; | ||
|
||
impl LintPass for Pass { | ||
fn get_lints(&self) -> LintArray { | ||
lint_array!(INFALLIBLE_DESTRUCTURING_MATCH) | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { | ||
fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local) { | ||
if_chain! { | ||
if let Some(ref expr) = local.init; | ||
if let Expr_::ExprMatch(ref target, ref arms, MatchSource::Normal) = expr.node; | ||
if arms.len() == 1 && arms[0].pats.len() == 1 && arms[0].guard.is_none(); | ||
if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pats[0].node; | ||
if args.len() == 1; | ||
if let Some(arg) = get_arg_name(&args[0]); | ||
let body = remove_blocks(&arms[0].body); | ||
if match_var(body, arg); | ||
|
||
then { | ||
span_lint_and_sugg( | ||
cx, | ||
INFALLIBLE_DESTRUCTURING_MATCH, | ||
local.span, | ||
"you seem to be trying to use match to destructure a single infallible pattern. \ | ||
Consider using `let`", | ||
"try this", | ||
format!( | ||
"let {}({}) = {};", | ||
snippet(cx, variant_name.span, ".."), | ||
snippet(cx, local.pat.span, ".."), | ||
snippet(cx, target.span, ".."), | ||
), | ||
); | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
#![feature(exhaustive_patterns)] | ||
#![allow(let_and_return)] | ||
|
||
enum SingleVariantEnum { | ||
Variant(i32), | ||
} | ||
|
||
struct TupleStruct(i32); | ||
|
||
enum EmptyEnum {} | ||
|
||
fn infallible_destructuring_match_enum() { | ||
let wrapper = SingleVariantEnum::Variant(0); | ||
|
||
// This should lint! | ||
let data = match wrapper { | ||
SingleVariantEnum::Variant(i) => i, | ||
}; | ||
|
||
// This shouldn't! | ||
let data = match wrapper { | ||
SingleVariantEnum::Variant(_) => -1, | ||
}; | ||
|
||
// Neither should this! | ||
let data = match wrapper { | ||
SingleVariantEnum::Variant(i) => -1, | ||
}; | ||
|
||
let SingleVariantEnum::Variant(data) = wrapper; | ||
} | ||
|
||
fn infallible_destructuring_match_struct() { | ||
let wrapper = TupleStruct(0); | ||
|
||
// This should lint! | ||
let data = match wrapper { | ||
TupleStruct(i) => i, | ||
}; | ||
|
||
// This shouldn't! | ||
let data = match wrapper { | ||
TupleStruct(_) => -1, | ||
}; | ||
|
||
// Neither should this! | ||
let data = match wrapper { | ||
TupleStruct(i) => -1, | ||
}; | ||
|
||
let TupleStruct(data) = wrapper; | ||
} | ||
|
||
fn never_enum() { | ||
let wrapper: Result<i32, !> = Ok(23); | ||
|
||
// This should lint! | ||
let data = match wrapper { | ||
Ok(i) => i, | ||
}; | ||
|
||
// This shouldn't! | ||
let data = match wrapper { | ||
Ok(_) => -1, | ||
}; | ||
|
||
// Neither should this! | ||
let data = match wrapper { | ||
Ok(i) => -1, | ||
}; | ||
|
||
let Ok(data) = wrapper; | ||
} | ||
|
||
impl EmptyEnum { | ||
fn match_on(&self) -> ! { | ||
// The lint shouldn't pick this up, as `let` won't work here! | ||
let data = match *self {}; | ||
data | ||
} | ||
} | ||
|
||
fn main() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let` | ||
--> $DIR/infallible_destructuring_match.rs:16:5 | ||
| | ||
16 | / let data = match wrapper { | ||
17 | | SingleVariantEnum::Variant(i) => i, | ||
18 | | }; | ||
| |______^ help: try this: `let SingleVariantEnum::Variant(data) = wrapper;` | ||
| | ||
= note: `-D infallible-destructuring-match` implied by `-D warnings` | ||
|
||
error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let` | ||
--> $DIR/infallible_destructuring_match.rs:37:5 | ||
| | ||
37 | / let data = match wrapper { | ||
38 | | TupleStruct(i) => i, | ||
39 | | }; | ||
| |______^ help: try this: `let TupleStruct(data) = wrapper;` | ||
|
||
error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let` | ||
--> $DIR/infallible_destructuring_match.rs:58:5 | ||
| | ||
58 | / let data = match wrapper { | ||
59 | | Ok(i) => i, | ||
60 | | }; | ||
| |______^ help: try this: `let Ok(data) = wrapper;` | ||
|
||
error: aborting due to 3 previous errors | ||
|