Skip to content

Commit 92dbc64

Browse files
committed
Auto merge of #4841 - phaylon:pattern-type-mismatch, r=flip1995
Added restriction lint: pattern-type-mismatch changelog: Added a new restriction lint `pattern-type-mismatch`. This lint is especially helpful for beginners learning about the magic behind pattern matching. (This explanation might be worth to include in the next changelog.)
2 parents c493090 + 77b6130 commit 92dbc64

14 files changed

+915
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,7 @@ Released 2018-09-13
15881588
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
15891589
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
15901590
[`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
1591+
[`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
15911592
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
15921593
[`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
15931594
[`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal

clippy_lints/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ mod overflow_check_conditional;
268268
mod panic_unimplemented;
269269
mod partialeq_ne_impl;
270270
mod path_buf_push_overwrite;
271+
mod pattern_type_mismatch;
271272
mod precedence;
272273
mod ptr;
273274
mod ptr_offset_with_cast;
@@ -741,6 +742,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
741742
&panic_unimplemented::UNREACHABLE,
742743
&partialeq_ne_impl::PARTIALEQ_NE_IMPL,
743744
&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,
745+
&pattern_type_mismatch::PATTERN_TYPE_MISMATCH,
744746
&precedence::PRECEDENCE,
745747
&ptr::CMP_NULL,
746748
&ptr::MUT_FROM_REF,
@@ -1064,6 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10641066
store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns);
10651067
store.register_late_pass(|| box macro_use::MacroUseImports::default());
10661068
store.register_late_pass(|| box map_identity::MapIdentity);
1069+
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
10671070

10681071
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10691072
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1097,6 +1100,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10971100
LintId::of(&panic_unimplemented::TODO),
10981101
LintId::of(&panic_unimplemented::UNIMPLEMENTED),
10991102
LintId::of(&panic_unimplemented::UNREACHABLE),
1103+
LintId::of(&pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
11001104
LintId::of(&shadow::SHADOW_REUSE),
11011105
LintId::of(&shadow::SHADOW_SAME),
11021106
LintId::of(&strings::STRING_ADD),
+316
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,316 @@
1+
use crate::utils::{last_path_segment, span_lint_and_help};
2+
use rustc_hir::{
3+
intravisit, Body, Expr, ExprKind, FieldPat, FnDecl, HirId, LocalSource, MatchSource, Mutability, Pat, PatKind,
4+
QPath, Stmt, StmtKind,
5+
};
6+
use rustc_lint::{LateContext, LateLintPass, LintContext};
7+
use rustc_middle::lint::in_external_macro;
8+
use rustc_middle::ty::subst::SubstsRef;
9+
use rustc_middle::ty::{AdtDef, FieldDef, Ty, TyKind, VariantDef};
10+
use rustc_session::{declare_lint_pass, declare_tool_lint};
11+
use rustc_span::source_map::Span;
12+
13+
declare_clippy_lint! {
14+
/// **What it does:** Checks for patterns that aren't exact representations of the types
15+
/// they are applied to.
16+
///
17+
/// To satisfy this lint, you will have to adjust either the expression that is matched
18+
/// against or the pattern itself, as well as the bindings that are introduced by the
19+
/// adjusted patterns. For matching you will have to either dereference the expression
20+
/// with the `*` operator, or amend the patterns to explicitly match against `&<pattern>`
21+
/// or `&mut <pattern>` depending on the reference mutability. For the bindings you need
22+
/// to use the inverse. You can leave them as plain bindings if you wish for the value
23+
/// to be copied, but you must use `ref mut <variable>` or `ref <variable>` to construct
24+
/// a reference into the matched structure.
25+
///
26+
/// If you are looking for a way to learn about ownership semantics in more detail, it
27+
/// is recommended to look at IDE options available to you to highlight types, lifetimes
28+
/// and reference semantics in your code. The available tooling would expose these things
29+
/// in a general way even outside of the various pattern matching mechanics. Of course
30+
/// this lint can still be used to highlight areas of interest and ensure a good understanding
31+
/// of ownership semantics.
32+
///
33+
/// **Why is this bad?** It isn't bad in general. But in some contexts it can be desirable
34+
/// because it increases ownership hints in the code, and will guard against some changes
35+
/// in ownership.
36+
///
37+
/// **Known problems:** None.
38+
///
39+
/// **Example:**
40+
///
41+
/// This example shows the basic adjustments necessary to satisfy the lint. Note how
42+
/// the matched expression is explicitly dereferenced with `*` and the `inner` variable
43+
/// is bound to a shared borrow via `ref inner`.
44+
///
45+
/// ```rust,ignore
46+
/// // Bad
47+
/// let value = &Some(Box::new(23));
48+
/// match value {
49+
/// Some(inner) => println!("{}", inner),
50+
/// None => println!("none"),
51+
/// }
52+
///
53+
/// // Good
54+
/// let value = &Some(Box::new(23));
55+
/// match *value {
56+
/// Some(ref inner) => println!("{}", inner),
57+
/// None => println!("none"),
58+
/// }
59+
/// ```
60+
///
61+
/// The following example demonstrates one of the advantages of the more verbose style.
62+
/// Note how the second version uses `ref mut a` to explicitly declare `a` a shared mutable
63+
/// borrow, while `b` is simply taken by value. This ensures that the loop body cannot
64+
/// accidentally modify the wrong part of the structure.
65+
///
66+
/// ```rust,ignore
67+
/// // Bad
68+
/// let mut values = vec![(2, 3), (3, 4)];
69+
/// for (a, b) in &mut values {
70+
/// *a += *b;
71+
/// }
72+
///
73+
/// // Good
74+
/// let mut values = vec![(2, 3), (3, 4)];
75+
/// for &mut (ref mut a, b) in &mut values {
76+
/// *a += b;
77+
/// }
78+
/// ```
79+
pub PATTERN_TYPE_MISMATCH,
80+
restriction,
81+
"type of pattern does not match the expression type"
82+
}
83+
84+
declare_lint_pass!(PatternTypeMismatch => [PATTERN_TYPE_MISMATCH]);
85+
86+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PatternTypeMismatch {
87+
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt<'_>) {
88+
if let StmtKind::Local(ref local) = stmt.kind {
89+
if let Some(init) = &local.init {
90+
if let Some(init_ty) = cx.tables().node_type_opt(init.hir_id) {
91+
let pat = &local.pat;
92+
if in_external_macro(cx.sess(), pat.span) {
93+
return;
94+
}
95+
let deref_possible = match local.source {
96+
LocalSource::Normal => DerefPossible::Possible,
97+
_ => DerefPossible::Impossible,
98+
};
99+
apply_lint(cx, pat, init_ty, deref_possible);
100+
}
101+
}
102+
}
103+
}
104+
105+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
106+
if let ExprKind::Match(ref expr, arms, source) = expr.kind {
107+
match source {
108+
MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar => {
109+
if let Some(expr_ty) = cx.tables().node_type_opt(expr.hir_id) {
110+
'pattern_checks: for arm in arms {
111+
let pat = &arm.pat;
112+
if in_external_macro(cx.sess(), pat.span) {
113+
continue 'pattern_checks;
114+
}
115+
if apply_lint(cx, pat, expr_ty, DerefPossible::Possible) {
116+
break 'pattern_checks;
117+
}
118+
}
119+
}
120+
},
121+
_ => (),
122+
}
123+
}
124+
}
125+
126+
fn check_fn(
127+
&mut self,
128+
cx: &LateContext<'a, 'tcx>,
129+
_: intravisit::FnKind<'tcx>,
130+
_: &'tcx FnDecl<'_>,
131+
body: &'tcx Body<'_>,
132+
_: Span,
133+
hir_id: HirId,
134+
) {
135+
if let Some(fn_sig) = cx.tables().liberated_fn_sigs().get(hir_id) {
136+
for (param, ty) in body.params.iter().zip(fn_sig.inputs().iter()) {
137+
apply_lint(cx, &param.pat, ty, DerefPossible::Impossible);
138+
}
139+
}
140+
}
141+
}
142+
143+
#[derive(Debug, Clone, Copy)]
144+
enum DerefPossible {
145+
Possible,
146+
Impossible,
147+
}
148+
149+
fn apply_lint<'a, 'tcx>(
150+
cx: &LateContext<'a, 'tcx>,
151+
pat: &Pat<'_>,
152+
expr_ty: Ty<'tcx>,
153+
deref_possible: DerefPossible,
154+
) -> bool {
155+
let maybe_mismatch = find_first_mismatch(cx, pat, expr_ty, Level::Top);
156+
if let Some((span, mutability, level)) = maybe_mismatch {
157+
span_lint_and_help(
158+
cx,
159+
PATTERN_TYPE_MISMATCH,
160+
span,
161+
"type of pattern does not match the expression type",
162+
None,
163+
&format!(
164+
"{}explicitly match against a `{}` pattern and adjust the enclosed variable bindings",
165+
match (deref_possible, level) {
166+
(DerefPossible::Possible, Level::Top) => "use `*` to dereference the match expression or ",
167+
_ => "",
168+
},
169+
match mutability {
170+
Mutability::Mut => "&mut _",
171+
Mutability::Not => "&_",
172+
},
173+
),
174+
);
175+
true
176+
} else {
177+
false
178+
}
179+
}
180+
181+
#[derive(Debug, Copy, Clone)]
182+
enum Level {
183+
Top,
184+
Lower,
185+
}
186+
187+
#[allow(rustc::usage_of_ty_tykind)]
188+
fn find_first_mismatch<'a, 'tcx>(
189+
cx: &LateContext<'a, 'tcx>,
190+
pat: &Pat<'_>,
191+
ty: Ty<'tcx>,
192+
level: Level,
193+
) -> Option<(Span, Mutability, Level)> {
194+
if let PatKind::Ref(ref sub_pat, _) = pat.kind {
195+
if let TyKind::Ref(_, sub_ty, _) = ty.kind {
196+
return find_first_mismatch(cx, sub_pat, sub_ty, Level::Lower);
197+
}
198+
}
199+
200+
if let TyKind::Ref(_, _, mutability) = ty.kind {
201+
if is_non_ref_pattern(&pat.kind) {
202+
return Some((pat.span, mutability, level));
203+
}
204+
}
205+
206+
if let PatKind::Struct(ref qpath, ref field_pats, _) = pat.kind {
207+
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
208+
if let Some(variant) = get_variant(adt_def, qpath) {
209+
let field_defs = &variant.fields;
210+
return find_first_mismatch_in_struct(cx, field_pats, field_defs, substs_ref);
211+
}
212+
}
213+
}
214+
215+
if let PatKind::TupleStruct(ref qpath, ref pats, _) = pat.kind {
216+
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
217+
if let Some(variant) = get_variant(adt_def, qpath) {
218+
let field_defs = &variant.fields;
219+
let ty_iter = field_defs.iter().map(|field_def| field_def.ty(cx.tcx, substs_ref));
220+
return find_first_mismatch_in_tuple(cx, pats, ty_iter);
221+
}
222+
}
223+
}
224+
225+
if let PatKind::Tuple(ref pats, _) = pat.kind {
226+
if let TyKind::Tuple(..) = ty.kind {
227+
return find_first_mismatch_in_tuple(cx, pats, ty.tuple_fields());
228+
}
229+
}
230+
231+
if let PatKind::Or(sub_pats) = pat.kind {
232+
for pat in sub_pats {
233+
let maybe_mismatch = find_first_mismatch(cx, pat, ty, level);
234+
if let Some(mismatch) = maybe_mismatch {
235+
return Some(mismatch);
236+
}
237+
}
238+
}
239+
240+
None
241+
}
242+
243+
fn get_variant<'a>(adt_def: &'a AdtDef, qpath: &QPath<'_>) -> Option<&'a VariantDef> {
244+
if adt_def.is_struct() {
245+
if let Some(variant) = adt_def.variants.iter().next() {
246+
return Some(variant);
247+
}
248+
}
249+
250+
if adt_def.is_enum() {
251+
let pat_ident = last_path_segment(qpath).ident;
252+
for variant in &adt_def.variants {
253+
if variant.ident == pat_ident {
254+
return Some(variant);
255+
}
256+
}
257+
}
258+
259+
None
260+
}
261+
262+
fn find_first_mismatch_in_tuple<'a, 'tcx, I>(
263+
cx: &LateContext<'a, 'tcx>,
264+
pats: &[&Pat<'_>],
265+
ty_iter_src: I,
266+
) -> Option<(Span, Mutability, Level)>
267+
where
268+
I: IntoIterator<Item = Ty<'tcx>>,
269+
{
270+
let mut field_tys = ty_iter_src.into_iter();
271+
'fields: for pat in pats {
272+
let field_ty = if let Some(ty) = field_tys.next() {
273+
ty
274+
} else {
275+
break 'fields;
276+
};
277+
278+
let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
279+
if let Some(mismatch) = maybe_mismatch {
280+
return Some(mismatch);
281+
}
282+
}
283+
284+
None
285+
}
286+
287+
fn find_first_mismatch_in_struct<'a, 'tcx>(
288+
cx: &LateContext<'a, 'tcx>,
289+
field_pats: &[FieldPat<'_>],
290+
field_defs: &[FieldDef],
291+
substs_ref: SubstsRef<'tcx>,
292+
) -> Option<(Span, Mutability, Level)> {
293+
for field_pat in field_pats {
294+
'definitions: for field_def in field_defs {
295+
if field_pat.ident == field_def.ident {
296+
let field_ty = field_def.ty(cx.tcx, substs_ref);
297+
let pat = &field_pat.pat;
298+
let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
299+
if let Some(mismatch) = maybe_mismatch {
300+
return Some(mismatch);
301+
}
302+
break 'definitions;
303+
}
304+
}
305+
}
306+
307+
None
308+
}
309+
310+
fn is_non_ref_pattern(pat_kind: &PatKind<'_>) -> bool {
311+
match pat_kind {
312+
PatKind::Struct(..) | PatKind::Tuple(..) | PatKind::TupleStruct(..) | PatKind::Path(..) => true,
313+
PatKind::Or(sub_pats) => sub_pats.iter().any(|pat| is_non_ref_pattern(&pat.kind)),
314+
_ => false,
315+
}
316+
}

src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1697,6 +1697,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
16971697
deprecation: None,
16981698
module: "path_buf_push_overwrite",
16991699
},
1700+
Lint {
1701+
name: "pattern_type_mismatch",
1702+
group: "restriction",
1703+
desc: "type of pattern does not match the expression type",
1704+
deprecation: None,
1705+
module: "pattern_type_mismatch",
1706+
},
17001707
Lint {
17011708
name: "possible_missing_comma",
17021709
group: "correctness",

0 commit comments

Comments
 (0)