Skip to content

Commit 57cdf2d

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 fff8e72 + c0fd452 commit 57cdf2d

14 files changed

+910
-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,
@@ -1065,6 +1067,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10651067
store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns);
10661068
store.register_late_pass(|| box macro_use::MacroUseImports::default());
10671069
store.register_late_pass(|| box map_identity::MapIdentity);
1070+
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
10681071

10691072
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10701073
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1098,6 +1101,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10981101
LintId::of(&panic_unimplemented::TODO),
10991102
LintId::of(&panic_unimplemented::UNIMPLEMENTED),
11001103
LintId::of(&panic_unimplemented::UNREACHABLE),
1104+
LintId::of(&pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
11011105
LintId::of(&shadow::SHADOW_REUSE),
11021106
LintId::of(&shadow::SHADOW_SAME),
11031107
LintId::of(&strings::STRING_ADD),
+311
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
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<'tcx> LateLintPass<'tcx> for PatternTypeMismatch {
87+
fn check_stmt(&mut self, cx: &LateContext<'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<'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<'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<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>, expr_ty: Ty<'tcx>, deref_possible: DerefPossible) -> bool {
150+
let maybe_mismatch = find_first_mismatch(cx, pat, expr_ty, Level::Top);
151+
if let Some((span, mutability, level)) = maybe_mismatch {
152+
span_lint_and_help(
153+
cx,
154+
PATTERN_TYPE_MISMATCH,
155+
span,
156+
"type of pattern does not match the expression type",
157+
None,
158+
&format!(
159+
"{}explicitly match against a `{}` pattern and adjust the enclosed variable bindings",
160+
match (deref_possible, level) {
161+
(DerefPossible::Possible, Level::Top) => "use `*` to dereference the match expression or ",
162+
_ => "",
163+
},
164+
match mutability {
165+
Mutability::Mut => "&mut _",
166+
Mutability::Not => "&_",
167+
},
168+
),
169+
);
170+
true
171+
} else {
172+
false
173+
}
174+
}
175+
176+
#[derive(Debug, Copy, Clone)]
177+
enum Level {
178+
Top,
179+
Lower,
180+
}
181+
182+
#[allow(rustc::usage_of_ty_tykind)]
183+
fn find_first_mismatch<'tcx>(
184+
cx: &LateContext<'tcx>,
185+
pat: &Pat<'_>,
186+
ty: Ty<'tcx>,
187+
level: Level,
188+
) -> Option<(Span, Mutability, Level)> {
189+
if let PatKind::Ref(ref sub_pat, _) = pat.kind {
190+
if let TyKind::Ref(_, sub_ty, _) = ty.kind {
191+
return find_first_mismatch(cx, sub_pat, sub_ty, Level::Lower);
192+
}
193+
}
194+
195+
if let TyKind::Ref(_, _, mutability) = ty.kind {
196+
if is_non_ref_pattern(&pat.kind) {
197+
return Some((pat.span, mutability, level));
198+
}
199+
}
200+
201+
if let PatKind::Struct(ref qpath, ref field_pats, _) = pat.kind {
202+
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
203+
if let Some(variant) = get_variant(adt_def, qpath) {
204+
let field_defs = &variant.fields;
205+
return find_first_mismatch_in_struct(cx, field_pats, field_defs, substs_ref);
206+
}
207+
}
208+
}
209+
210+
if let PatKind::TupleStruct(ref qpath, ref pats, _) = pat.kind {
211+
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
212+
if let Some(variant) = get_variant(adt_def, qpath) {
213+
let field_defs = &variant.fields;
214+
let ty_iter = field_defs.iter().map(|field_def| field_def.ty(cx.tcx, substs_ref));
215+
return find_first_mismatch_in_tuple(cx, pats, ty_iter);
216+
}
217+
}
218+
}
219+
220+
if let PatKind::Tuple(ref pats, _) = pat.kind {
221+
if let TyKind::Tuple(..) = ty.kind {
222+
return find_first_mismatch_in_tuple(cx, pats, ty.tuple_fields());
223+
}
224+
}
225+
226+
if let PatKind::Or(sub_pats) = pat.kind {
227+
for pat in sub_pats {
228+
let maybe_mismatch = find_first_mismatch(cx, pat, ty, level);
229+
if let Some(mismatch) = maybe_mismatch {
230+
return Some(mismatch);
231+
}
232+
}
233+
}
234+
235+
None
236+
}
237+
238+
fn get_variant<'a>(adt_def: &'a AdtDef, qpath: &QPath<'_>) -> Option<&'a VariantDef> {
239+
if adt_def.is_struct() {
240+
if let Some(variant) = adt_def.variants.iter().next() {
241+
return Some(variant);
242+
}
243+
}
244+
245+
if adt_def.is_enum() {
246+
let pat_ident = last_path_segment(qpath).ident;
247+
for variant in &adt_def.variants {
248+
if variant.ident == pat_ident {
249+
return Some(variant);
250+
}
251+
}
252+
}
253+
254+
None
255+
}
256+
257+
fn find_first_mismatch_in_tuple<'tcx, I>(
258+
cx: &LateContext<'tcx>,
259+
pats: &[&Pat<'_>],
260+
ty_iter_src: I,
261+
) -> Option<(Span, Mutability, Level)>
262+
where
263+
I: IntoIterator<Item = Ty<'tcx>>,
264+
{
265+
let mut field_tys = ty_iter_src.into_iter();
266+
'fields: for pat in pats {
267+
let field_ty = if let Some(ty) = field_tys.next() {
268+
ty
269+
} else {
270+
break 'fields;
271+
};
272+
273+
let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
274+
if let Some(mismatch) = maybe_mismatch {
275+
return Some(mismatch);
276+
}
277+
}
278+
279+
None
280+
}
281+
282+
fn find_first_mismatch_in_struct<'tcx>(
283+
cx: &LateContext<'tcx>,
284+
field_pats: &[FieldPat<'_>],
285+
field_defs: &[FieldDef],
286+
substs_ref: SubstsRef<'tcx>,
287+
) -> Option<(Span, Mutability, Level)> {
288+
for field_pat in field_pats {
289+
'definitions: for field_def in field_defs {
290+
if field_pat.ident == field_def.ident {
291+
let field_ty = field_def.ty(cx.tcx, substs_ref);
292+
let pat = &field_pat.pat;
293+
let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
294+
if let Some(mismatch) = maybe_mismatch {
295+
return Some(mismatch);
296+
}
297+
break 'definitions;
298+
}
299+
}
300+
}
301+
302+
None
303+
}
304+
305+
fn is_non_ref_pattern(pat_kind: &PatKind<'_>) -> bool {
306+
match pat_kind {
307+
PatKind::Struct(..) | PatKind::Tuple(..) | PatKind::TupleStruct(..) | PatKind::Path(..) => true,
308+
PatKind::Or(sub_pats) => sub_pats.iter().any(|pat| is_non_ref_pattern(&pat.kind)),
309+
_ => false,
310+
}
311+
}

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)