Skip to content

Commit

Permalink
Added restriction lint: pattern-type-mismatch
Browse files Browse the repository at this point in the history
  • Loading branch information
phaylon committed Jul 3, 2020
1 parent c493090 commit c3c4027
Show file tree
Hide file tree
Showing 14 changed files with 874 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1588,6 +1588,7 @@ Released 2018-09-13
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
[`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
[`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
[`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
[`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -268,6 +268,7 @@ mod overflow_check_conditional;
mod panic_unimplemented;
mod partialeq_ne_impl;
mod path_buf_push_overwrite;
pub mod pattern_type_mismatch;
mod precedence;
mod ptr;
mod ptr_offset_with_cast;
Expand Down Expand Up @@ -741,6 +742,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&panic_unimplemented::UNREACHABLE,
&partialeq_ne_impl::PARTIALEQ_NE_IMPL,
&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE,
&pattern_type_mismatch::PATTERN_TYPE_MISMATCH,
&precedence::PRECEDENCE,
&ptr::CMP_NULL,
&ptr::MUT_FROM_REF,
Expand Down Expand Up @@ -1064,6 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns);
store.register_late_pass(|| box macro_use::MacroUseImports::default());
store.register_late_pass(|| box map_identity::MapIdentity);
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1097,6 +1100,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&panic_unimplemented::TODO),
LintId::of(&panic_unimplemented::UNIMPLEMENTED),
LintId::of(&panic_unimplemented::UNREACHABLE),
LintId::of(&pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
LintId::of(&shadow::SHADOW_REUSE),
LintId::of(&shadow::SHADOW_SAME),
LintId::of(&strings::STRING_ADD),
Expand Down
276 changes: 276 additions & 0 deletions clippy_lints/src/pattern_type_mismatch.rs
@@ -0,0 +1,276 @@
use crate::utils::{last_path_segment, span_help_and_lint};
use rustc::lint::in_external_macro;
use rustc::ty::subst::SubstsRef;
use rustc::ty::{AdtDef, FieldDef, Ty, TyKind, VariantDef};
use rustc_hir::{
intravisit, Body, Expr, ExprKind, FieldPat, FnDecl, HirId, LocalSource, MatchSource, Mutability, Pat, PatKind,
QPath, Stmt, StmtKind,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;

declare_clippy_lint! {
/// **What it does:** Checks for patterns that aren't exact representations of the types
/// they are applied to.
///
/// **Why is this bad?** It isn't bad in general. But in some contexts it can be desirable
/// because it increases ownership hints in the code, and will guard against some changes
/// in ownership.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust,ignore
/// // Bad
/// let value = &Some(Box::new(23));
/// match value {
/// Some(inner) => println!("{}", inner),
/// None => println!("none"),
/// }
///
/// // Good
/// let value = &Some(Box::new(23));
/// match *value {
/// Some(ref inner) => println!("{}", inner),
/// None => println!("none"),
/// }
/// ```
pub PATTERN_TYPE_MISMATCH,
restriction,
"type of pattern does not match the expression type"
}

declare_lint_pass!(PatternTypeMismatch => [PATTERN_TYPE_MISMATCH]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PatternTypeMismatch {
fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt<'_>) {
if let StmtKind::Local(ref local) = stmt.kind {
if let Some(init) = &local.init {
if let Some(init_ty) = cx.tables.node_type_opt(init.hir_id) {
let pat = &local.pat;
if in_external_macro(cx.sess(), pat.span) {
return;
}
let deref_possible = match local.source {
LocalSource::Normal => DerefPossible::Possible,
_ => DerefPossible::Impossible,
};
apply_lint(cx, pat, init_ty, deref_possible);
}
}
}
}

fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(ref expr, arms, source) = expr.kind {
match source {
MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar => {
if let Some(expr_ty) = cx.tables.node_type_opt(expr.hir_id) {
'pattern_checks: for arm in arms {
let pat = &arm.pat;
if in_external_macro(cx.sess(), pat.span) {
continue 'pattern_checks;
}
if apply_lint(cx, pat, expr_ty, DerefPossible::Possible) {
break 'pattern_checks;
}
}
}
},
_ => (),
}
}
}

fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
_: intravisit::FnKind<'tcx>,
_: &'tcx FnDecl<'_>,
body: &'tcx Body<'_>,
_: Span,
hir_id: HirId,
) {
if let Some(fn_sig) = cx.tables.liberated_fn_sigs().get(hir_id) {
for (param, ty) in body.params.iter().zip(fn_sig.inputs().iter()) {
apply_lint(cx, &param.pat, ty, DerefPossible::Impossible);
}
}
}
}

#[derive(Debug, Clone, Copy)]
enum DerefPossible {
Possible,
Impossible,
}

fn apply_lint<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
pat: &Pat<'_>,
expr_ty: Ty<'tcx>,
deref_possible: DerefPossible,
) -> bool {
let maybe_mismatch = find_first_mismatch(cx, pat, expr_ty, Level::Top);
if let Some((span, mutability, level)) = maybe_mismatch {
span_help_and_lint(
cx,
PATTERN_TYPE_MISMATCH,
span,
"type of pattern does not match the expression type",
&format!(
"{}explicitly match against a `{}` pattern and adjust the enclosed variable bindings",
match (deref_possible, level) {
(DerefPossible::Possible, Level::Top) => "use `*` to dereference the match expression or ",
_ => "",
},
match mutability {
Mutability::Mut => "&mut _",
Mutability::Not => "&_",
},
),
);
true
} else {
false
}
}

#[derive(Debug, Copy, Clone)]
enum Level {
Top,
Lower,
}

#[allow(rustc::usage_of_ty_tykind)]
fn find_first_mismatch<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
pat: &Pat<'_>,
ty: Ty<'tcx>,
level: Level,
) -> Option<(Span, Mutability, Level)> {
if let PatKind::Ref(ref sub_pat, _) = pat.kind {
if let TyKind::Ref(_, sub_ty, _) = ty.kind {
return find_first_mismatch(cx, sub_pat, sub_ty, Level::Lower);
}
}

if let TyKind::Ref(_, _, mutability) = ty.kind {
if is_non_ref_pattern(&pat.kind) {
return Some((pat.span, mutability, level));
}
}

if let PatKind::Struct(ref qpath, ref field_pats, _) = pat.kind {
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
if let Some(variant) = get_variant(adt_def, qpath) {
let field_defs = &variant.fields;
return find_first_mismatch_in_struct(cx, field_pats, field_defs, substs_ref);
}
}
}

if let PatKind::TupleStruct(ref qpath, ref pats, _) = pat.kind {
if let TyKind::Adt(ref adt_def, ref substs_ref) = ty.kind {
if let Some(variant) = get_variant(adt_def, qpath) {
let field_defs = &variant.fields;
let ty_iter = field_defs.iter().map(|field_def| field_def.ty(cx.tcx, substs_ref));
return find_first_mismatch_in_tuple(cx, pats, ty_iter);
}
}
}

if let PatKind::Tuple(ref pats, _) = pat.kind {
if let TyKind::Tuple(..) = ty.kind {
return find_first_mismatch_in_tuple(cx, pats, ty.tuple_fields());
}
}

if let PatKind::Or(sub_pats) = pat.kind {
for pat in sub_pats {
let maybe_mismatch = find_first_mismatch(cx, pat, ty, level);
if let Some(mismatch) = maybe_mismatch {
return Some(mismatch);
}
}
}

None
}

fn get_variant<'a>(adt_def: &'a AdtDef, qpath: &QPath<'_>) -> Option<&'a VariantDef> {
if adt_def.is_struct() {
if let Some(variant) = adt_def.variants.iter().next() {
return Some(variant);
}
}

if adt_def.is_enum() {
let pat_ident = last_path_segment(qpath).ident;
for variant in &adt_def.variants {
if variant.ident == pat_ident {
return Some(variant);
}
}
}

None
}

fn find_first_mismatch_in_tuple<'a, 'tcx, I>(
cx: &LateContext<'a, 'tcx>,
pats: &[&Pat<'_>],
ty_iter_src: I,
) -> Option<(Span, Mutability, Level)>
where
I: IntoIterator<Item = Ty<'tcx>>,
{
let mut field_tys = ty_iter_src.into_iter();
'fields: for pat in pats {
let field_ty = if let Some(ty) = field_tys.next() {
ty
} else {
break 'fields;
};

let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
if let Some(mismatch) = maybe_mismatch {
return Some(mismatch);
}
}

None
}

fn find_first_mismatch_in_struct<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
field_pats: &[FieldPat<'_>],
field_defs: &[FieldDef],
substs_ref: SubstsRef<'tcx>,
) -> Option<(Span, Mutability, Level)> {
for field_pat in field_pats {
'definitions: for field_def in field_defs {
if field_pat.ident == field_def.ident {
let field_ty = field_def.ty(cx.tcx, substs_ref);
let pat = &field_pat.pat;
let maybe_mismatch = find_first_mismatch(cx, pat, field_ty, Level::Lower);
if let Some(mismatch) = maybe_mismatch {
return Some(mismatch);
}
break 'definitions;
}
}
}

None
}

fn is_non_ref_pattern(pat_kind: &PatKind<'_>) -> bool {
match pat_kind {
PatKind::Struct(..) | PatKind::Tuple(..) | PatKind::TupleStruct(..) | PatKind::Path(..) => true,
PatKind::Or(sub_pats) => sub_pats.iter().any(|pat| is_non_ref_pattern(&pat.kind)),
_ => false,
}
}
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Expand Up @@ -1697,6 +1697,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "path_buf_push_overwrite",
},
Lint {
name: "pattern_type_mismatch",
group: "restriction",
desc: "type of pattern does not match the expression type",
deprecation: None,
module: "pattern_type_mismatch",
},
Lint {
name: "possible_missing_comma",
group: "correctness",
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/pattern_type_mismatch/mutability.rs
@@ -0,0 +1,40 @@
#![allow(clippy::all)]
#![warn(clippy::pattern_type_mismatch)]

fn main() {}

fn should_lint() {
let value = &Some(23);
match value {
Some(_) => (),
_ => (),
}

let value = &mut Some(23);
match value {
Some(_) => (),
_ => (),
}
}

fn should_not_lint() {
let value = &Some(23);
match value {
&Some(_) => (),
_ => (),
}
match *value {
Some(_) => (),
_ => (),
}

let value = &mut Some(23);
match value {
&mut Some(_) => (),
_ => (),
}
match *value {
Some(_) => (),
_ => (),
}
}
19 changes: 19 additions & 0 deletions tests/ui/pattern_type_mismatch/mutability.stderr
@@ -0,0 +1,19 @@
error: type of pattern does not match the expression type
--> $DIR/mutability.rs:9:9
|
LL | Some(_) => (),
| ^^^^^^^
|
= note: `-D clippy::pattern-type-mismatch` implied by `-D warnings`
= help: use `*` to dereference the match expression or explicitly match against a `&_` pattern and adjust the enclosed variable bindings

error: type of pattern does not match the expression type
--> $DIR/mutability.rs:15:9
|
LL | Some(_) => (),
| ^^^^^^^
|
= help: use `*` to dereference the match expression or explicitly match against a `&mut _` pattern and adjust the enclosed variable bindings

error: aborting due to 2 previous errors

0 comments on commit c3c4027

Please sign in to comment.