Skip to content

Commit 6afd7ea

Browse files
committed
Use span_lint_and_sugg + move infaillible lint
- moving infaillible lint to prevent collisions
1 parent 3445d41 commit 6afd7ea

File tree

10 files changed

+166
-112
lines changed

10 files changed

+166
-112
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,7 @@ Released 2018-09-13
12171217
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
12181218
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
12191219
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
1220+
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
12201221
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
12211222
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
12221223
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum

clippy_lints/src/infallible_destructuring_match.rs

Lines changed: 0 additions & 77 deletions
This file was deleted.

clippy_lints/src/lib.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ pub mod if_let_some_result;
218218
pub mod if_not_else;
219219
pub mod implicit_return;
220220
pub mod indexing_slicing;
221-
pub mod infallible_destructuring_match;
222221
pub mod infinite_iter;
223222
pub mod inherent_impl;
224223
pub mod inherent_to_string;
@@ -555,7 +554,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
555554
&implicit_return::IMPLICIT_RETURN,
556555
&indexing_slicing::INDEXING_SLICING,
557556
&indexing_slicing::OUT_OF_BOUNDS_INDEXING,
558-
&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
559557
&infinite_iter::INFINITE_ITER,
560558
&infinite_iter::MAYBE_INFINITE_ITER,
561559
&inherent_impl::MULTIPLE_INHERENT_IMPL,
@@ -600,12 +598,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
600598
&map_clone::MAP_CLONE,
601599
&map_unit_fn::OPTION_MAP_UNIT_FN,
602600
&map_unit_fn::RESULT_MAP_UNIT_FN,
601+
&matches::INFALLIBLE_DESTRUCTURING_MATCH,
603602
&matches::MATCH_AS_REF,
604603
&matches::MATCH_BOOL,
605604
&matches::MATCH_OVERLAPPING_ARM,
606605
&matches::MATCH_REF_PATS,
607-
&matches::MATCH_WILD_ERR_ARM,
608606
&matches::MATCH_SINGLE_BINDING,
607+
&matches::MATCH_WILD_ERR_ARM,
609608
&matches::SINGLE_MATCH,
610609
&matches::SINGLE_MATCH_ELSE,
611610
&matches::WILDCARD_ENUM_MATCH_ARM,
@@ -865,7 +864,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
865864
store.register_late_pass(|| box types::Casts);
866865
let type_complexity_threshold = conf.type_complexity_threshold;
867866
store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold));
868-
store.register_late_pass(|| box matches::Matches);
867+
store.register_late_pass(|| box matches::Matches::default());
869868
store.register_late_pass(|| box minmax::MinMaxPass);
870869
store.register_late_pass(|| box open_options::OpenOptions);
871870
store.register_late_pass(|| box zero_div_zero::ZeroDiv);
@@ -942,7 +941,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
942941
store.register_late_pass(|| box question_mark::QuestionMark);
943942
store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl);
944943
store.register_late_pass(|| box map_unit_fn::MapUnit);
945-
store.register_late_pass(|| box infallible_destructuring_match::InfallibleDestructingMatch);
946944
store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default());
947945
store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
948946
store.register_late_pass(|| box unwrap::Unwrap);
@@ -1167,7 +1165,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11671165
LintId::of(&identity_op::IDENTITY_OP),
11681166
LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
11691167
LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
1170-
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
11711168
LintId::of(&infinite_iter::INFINITE_ITER),
11721169
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
11731170
LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
@@ -1202,12 +1199,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12021199
LintId::of(&map_clone::MAP_CLONE),
12031200
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
12041201
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
1202+
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
12051203
LintId::of(&matches::MATCH_AS_REF),
12061204
LintId::of(&matches::MATCH_BOOL),
12071205
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
12081206
LintId::of(&matches::MATCH_REF_PATS),
1209-
LintId::of(&matches::MATCH_WILD_ERR_ARM),
12101207
LintId::of(&matches::MATCH_SINGLE_BINDING),
1208+
LintId::of(&matches::MATCH_WILD_ERR_ARM),
12111209
LintId::of(&matches::SINGLE_MATCH),
12121210
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
12131211
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
@@ -1384,7 +1382,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13841382
LintId::of(&functions::DOUBLE_MUST_USE),
13851383
LintId::of(&functions::MUST_USE_UNIT),
13861384
LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
1387-
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
13881385
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
13891386
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
13901387
LintId::of(&len_zero::LEN_ZERO),
@@ -1397,6 +1394,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13971394
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
13981395
LintId::of(&main_recursion::MAIN_RECURSION),
13991396
LintId::of(&map_clone::MAP_CLONE),
1397+
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
14001398
LintId::of(&matches::MATCH_BOOL),
14011399
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
14021400
LintId::of(&matches::MATCH_REF_PATS),

clippy_lints/src/matches.rs

Lines changed: 110 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::utils::usage::is_unused;
55
use crate::utils::{
66
span_lint_and_help, span_lint_and_note,
77
expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks,
8-
snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
8+
snippet, snippet_block, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
99
};
1010
use if_chain::if_chain;
1111
use rustc::lint::in_external_macro;
@@ -14,7 +14,7 @@ use rustc_errors::Applicability;
1414
use rustc_hir::def::CtorKind;
1515
use rustc_hir::*;
1616
use rustc_lint::{LateContext, LateLintPass, LintContext};
17-
use rustc_session::{declare_lint_pass, declare_tool_lint};
17+
use rustc_session::{declare_tool_lint, impl_lint_pass};
1818
use rustc_span::source_map::Span;
1919
use std::cmp::Ordering;
2020
use std::collections::Bound;
@@ -245,12 +245,47 @@ declare_clippy_lint! {
245245
"a wildcard pattern used with others patterns in same match arm"
246246
}
247247

248+
declare_clippy_lint! {
249+
/// **What it does:** Checks for matches being used to destructure a single-variant enum
250+
/// or tuple struct where a `let` will suffice.
251+
///
252+
/// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
253+
///
254+
/// **Known problems:** None.
255+
///
256+
/// **Example:**
257+
/// ```rust
258+
/// enum Wrapper {
259+
/// Data(i32),
260+
/// }
261+
///
262+
/// let wrapper = Wrapper::Data(42);
263+
///
264+
/// let data = match wrapper {
265+
/// Wrapper::Data(i) => i,
266+
/// };
267+
/// ```
268+
///
269+
/// The correct use would be:
270+
/// ```rust
271+
/// enum Wrapper {
272+
/// Data(i32),
273+
/// }
274+
///
275+
/// let wrapper = Wrapper::Data(42);
276+
/// let Wrapper::Data(data) = wrapper;
277+
/// ```
278+
pub INFALLIBLE_DESTRUCTURING_MATCH,
279+
style,
280+
"a `match` statement with a single infallible arm instead of a `let`"
281+
}
282+
248283
declare_clippy_lint! {
249284
/// **What it does:** Checks for useless match that binds to only one value.
250285
///
251286
/// **Why is this bad?** Readability and needless complexity.
252287
///
253-
/// **Known problems:** This situation frequently happen in macros, so can't lint there.
288+
/// **Known problems:** None.
254289
///
255290
/// **Example:**
256291
/// ```rust
@@ -272,7 +307,12 @@ declare_clippy_lint! {
272307
"a match with a single binding instead of using `let` statement"
273308
}
274309

275-
declare_lint_pass!(Matches => [
310+
#[derive(Default)]
311+
pub struct Matches {
312+
infallible_destructuring_match_linted: bool,
313+
}
314+
315+
impl_lint_pass!(Matches => [
276316
SINGLE_MATCH,
277317
MATCH_REF_PATS,
278318
MATCH_BOOL,
@@ -283,6 +323,7 @@ declare_lint_pass!(Matches => [
283323
WILDCARD_ENUM_MATCH_ARM,
284324
WILDCARD_IN_OR_PATTERNS,
285325
MATCH_SINGLE_BINDING,
326+
INFALLIBLE_DESTRUCTURING_MATCH
286327
]);
287328

288329
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
@@ -298,12 +339,51 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
298339
check_wild_enum_match(cx, ex, arms);
299340
check_match_as_ref(cx, ex, arms, expr);
300341
check_wild_in_or_pats(cx, arms);
301-
check_match_single_binding(cx, ex, arms, expr);
342+
343+
if self.infallible_destructuring_match_linted {
344+
self.infallible_destructuring_match_linted = false;
345+
} else {
346+
check_match_single_binding(cx, ex, arms, expr);
347+
}
302348
}
303349
if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
304350
check_match_ref_pats(cx, ex, arms, expr);
305351
}
306352
}
353+
354+
fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
355+
if_chain! {
356+
if let Some(ref expr) = local.init;
357+
if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
358+
if arms.len() == 1 && arms[0].guard.is_none();
359+
if let PatKind::TupleStruct(
360+
QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
361+
if args.len() == 1;
362+
if let Some(arg) = get_arg_name(&args[0]);
363+
let body = remove_blocks(&arms[0].body);
364+
if match_var(body, arg);
365+
366+
then {
367+
let mut applicability = Applicability::MachineApplicable;
368+
self.infallible_destructuring_match_linted = true;
369+
span_lint_and_sugg(
370+
cx,
371+
INFALLIBLE_DESTRUCTURING_MATCH,
372+
local.span,
373+
"you seem to be trying to use `match` to destructure a single infallible pattern. \
374+
Consider using `let`",
375+
"try this",
376+
format!(
377+
"let {}({}) = {};",
378+
snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
379+
snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
380+
snippet_with_applicability(cx, target.span, "..", &mut applicability),
381+
),
382+
applicability,
383+
);
384+
}
385+
}
386+
}
307387
}
308388

309389
#[rustfmt::skip]
@@ -746,21 +826,31 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A
746826
return;
747827
}
748828
if arms.len() == 1 {
749-
let bind_names = arms[0].pat.span;
750-
let matched_vars = ex.span;
751-
span_lint_and_sugg(
752-
cx,
753-
MATCH_SINGLE_BINDING,
754-
expr.span,
755-
"this match could be written as a `let` statement",
756-
"try this",
757-
format!(
758-
"let {} = {};",
759-
snippet(cx, bind_names, ".."),
760-
snippet(cx, matched_vars, "..")
761-
),
762-
Applicability::HasPlaceholders,
763-
);
829+
if is_refutable(cx, arms[0].pat) {
830+
return;
831+
}
832+
match arms[0].pat.kind {
833+
PatKind::Binding(..) | PatKind::Tuple(_, _) => {
834+
let bind_names = arms[0].pat.span;
835+
let matched_vars = ex.span;
836+
let match_body = remove_blocks(&arms[0].body);
837+
span_lint_and_sugg(
838+
cx,
839+
MATCH_SINGLE_BINDING,
840+
expr.span,
841+
"this match could be written as a `let` statement",
842+
"consider using `let` statement",
843+
format!(
844+
"let {} = {};\n{}",
845+
snippet(cx, bind_names, ".."),
846+
snippet(cx, matched_vars, ".."),
847+
snippet_block(cx, match_body.span, "..")
848+
),
849+
Applicability::MachineApplicable,
850+
);
851+
},
852+
_ => (),
853+
}
764854
}
765855
}
766856

src/lintlist/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,7 @@ pub const ALL_LINTS: [Lint; 351] = [
775775
group: "style",
776776
desc: "a `match` statement with a single infallible arm instead of a `let`",
777777
deprecation: None,
778-
module: "infallible_destructuring_match",
778+
module: "matches",
779779
},
780780
Lint {
781781
name: "infinite_iter",
@@ -1092,6 +1092,13 @@ pub const ALL_LINTS: [Lint; 351] = [
10921092
deprecation: None,
10931093
module: "copies",
10941094
},
1095+
Lint {
1096+
name: "match_single_binding",
1097+
group: "complexity",
1098+
desc: "a match with a single binding instead of using `let` statement",
1099+
deprecation: None,
1100+
module: "matches",
1101+
},
10951102
Lint {
10961103
name: "match_wild_err_arm",
10971104
group: "style",

tests/ui/escape_analysis.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
clippy::borrowed_box,
44
clippy::needless_pass_by_value,
55
clippy::unused_unit,
6-
clippy::redundant_clone
6+
clippy::redundant_clone,
77
)]
88
#![warn(clippy::boxed_local)]
99

0 commit comments

Comments
 (0)