Skip to content

WIP: Add `not_exhaustive_enough´ lint #6328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,7 @@ Released 2018-09-13
[`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
[`not_exhaustive_enough`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_exhaustive_enough
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ mod new_without_default;
mod no_effect;
mod non_copy_const;
mod non_expressive_names;
mod not_exhaustive_enough;
mod open_options;
mod option_env_unwrap;
mod option_if_let_else;
Expand Down Expand Up @@ -810,6 +811,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS,
&non_expressive_names::MANY_SINGLE_CHAR_NAMES,
&non_expressive_names::SIMILAR_NAMES,
&not_exhaustive_enough::NOT_EXHAUSTIVE_ENOUGH,
&open_options::NONSENSICAL_OPEN_OPTIONS,
&option_env_unwrap::OPTION_ENV_UNWRAP,
&option_if_let_else::OPTION_IF_LET_ELSE,
Expand Down Expand Up @@ -1124,6 +1126,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite);
store.register_late_pass(|| box integer_division::IntegerDivision);
store.register_late_pass(|| box inherent_to_string::InherentToString);
store.register_late_pass(|| box not_exhaustive_enough::NotExhaustiveEnough);
let max_trait_bounds = conf.max_trait_bounds;
store.register_late_pass(move || box trait_bounds::TraitBounds::new(max_trait_bounds));
store.register_late_pass(|| box comparison_chain::ComparisonChain);
Expand Down Expand Up @@ -1319,6 +1322,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&needless_continue::NEEDLESS_CONTINUE),
LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
LintId::of(&non_expressive_names::SIMILAR_NAMES),
LintId::of(&not_exhaustive_enough::NOT_EXHAUSTIVE_ENOUGH),
LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(&pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE),
LintId::of(&pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF),
Expand Down
171 changes: 171 additions & 0 deletions clippy_lints/src/not_exhaustive_enough.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
use crate::utils::span_lint_and_sugg;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, FieldPat, MatchSource, Pat, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:**
/// As suggested in the [non_exhaustive RFC](https://github.com/rust-lang/rfcs/blob/master/text/2008-non-exhaustive.md#unresolved-questions),
/// when using non-exhaustive enums and structs in patterns,
/// this lint warns the user for missing variants or fields despite having a wildcard arm or a rest pattern.
///
/// **Why is this bad?**
/// When new fields/variants are added by the upstream crate they might go unnoticed.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// #[non_exhaustive]
/// # enum E {First,Second,Third}
/// # let e = E::First;
/// // Bad
/// match e {
/// E::First => {}
/// E::Second => {}
/// _ => {}
/// }
/// // Good
/// match e {
/// E::First => {}
/// E::Second => {}
/// E::Third => {}
/// }
Comment on lines +35 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of #[non_exhaustive] is that the wildcard match is always required:

Suggested change
/// E::Third => {}
/// }
/// E::Third => {}
/// _ => {}
/// }

/// ```
pub NOT_EXHAUSTIVE_ENOUGH,
pedantic,
"missing variants or fields in a pattern despite having a wildcard arm or a rest pattern"
}

declare_lint_pass!(NotExhaustiveEnough => [NOT_EXHAUSTIVE_ENOUGH]);

impl<'tcx> LateLintPass<'tcx> for NotExhaustiveEnough {
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
if_chain! {
if let PatKind::Struct(_, ref field_pats, ref rest_pat) = &pat.kind;
if let ty::Adt(adt_def, _) = cx.typeck_results().pat_ty(pat).kind();
if is_struct_not_exhaustive(adt_def);
if *rest_pat;
if !field_pats.is_empty();
if let Some(variant) = get_variant(adt_def);
if let field_defs = &variant.fields;
then
{
check_struct_pat(cx, pat, field_pats, field_defs);
}
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Match(e, ref arms, MatchSource::Normal) = expr.kind;
if !arms.is_empty();
if let ExprKind::Path(..) = e.kind;
if let ty::Adt(adt_def, _) = cx.typeck_results().expr_ty(e).kind();
if adt_def.is_enum();
if is_enum_not_exhaustive(adt_def);
then
{
check_path_pat(cx, arms, e);
}
}
}
}

fn check_path_pat<'tcx>(cx: &LateContext<'tcx>, arms: &[Arm<'_>], e: &'tcx Expr<'_>) {
let missing_variants = get_missing_variants(cx, arms, e);
span_lint_and_sugg(
cx,
NOT_EXHAUSTIVE_ENOUGH,
e.span,
"enum match is not exhaustive enough",
"try adding missing variants",
missing_variants.join(" , "),
Applicability::MaybeIncorrect,
);
}

fn check_struct_pat<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
field_pats: &[FieldPat<'_>],
field_defs: &[ty::FieldDef],
) {
let missing_fields = get_missing_fields(field_pats, field_defs);
if !missing_fields.is_empty() {
let suggestions: Vec<String> = missing_fields.iter().map(|v| v.to_owned() + ": _").collect();
span_lint_and_sugg(
cx,
NOT_EXHAUSTIVE_ENOUGH,
pat.span,
"struct match is not exhaustive enough",
"try adding missing fields",
suggestions.join(" , "),
Applicability::MaybeIncorrect,
);
}
}

fn get_missing_variants<'tcx>(cx: &LateContext<'tcx>, arms: &[Arm<'_>], e: &'tcx Expr<'_>) -> Vec<String> {
let ty = cx.typeck_results().expr_ty(e);
let mut missing_variants = vec![];
if let ty::Adt(def, _) = ty.kind() {
for variant in &def.variants {
missing_variants.push(variant);
}
}
for arm in arms {
if let PatKind::Path(ref path) = arm.pat.kind {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
} else if let PatKind::TupleStruct(QPath::Resolved(_, p), ref patterns, ..) = arm.pat.kind {
let is_pattern_exhaustive = |pat: &&Pat<'_>| matches!(pat.kind, PatKind::Wild | PatKind::Binding(.., None));
if patterns.iter().all(is_pattern_exhaustive) {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
}
}
let missing_variants = missing_variants.iter().map(|v| cx.tcx.def_path_str(v.def_id)).collect();
missing_variants
}

fn get_missing_fields(field_pats: &[FieldPat<'_>], field_defs: &[ty::FieldDef]) -> Vec<String> {
let mut missing_fields = vec![];
let mut field_match = false;

for field_def in field_defs {
for field_pat in field_pats {
if field_def.ident == field_pat.ident {
field_match = true;
break;
}
}
if !&field_match && field_def.vis.is_visible_locally() {
missing_fields.push(field_def.ident.name.to_ident_string())
}
field_match = false;
}
missing_fields
}

fn is_enum_not_exhaustive(adt_def: &ty::AdtDef) -> bool {
adt_def.is_variant_list_non_exhaustive()
}

fn is_struct_not_exhaustive(adt_def: &ty::AdtDef) -> bool {
if let Some(variant) = adt_def.variants.iter().next() {
return variant.is_field_list_non_exhaustive();
}
false
}

fn get_variant(adt_def: &ty::AdtDef) -> Option<&ty::VariantDef> {
if let Some(variant) = adt_def.variants.iter().next() {
return Some(variant);
}
None
}
18 changes: 18 additions & 0 deletions tests/ui/auxiliary/not_exhaustive_enough_helper.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#[non_exhaustive]
pub enum AnotherCrateEnum {
AFirst,
ASecond,
AThird,
}

#[derive(Default)]
#[non_exhaustive]
pub struct AnotherCrateStruct {
pub a1: i32,
pub b1: i32,
pub c1: i32,
}

#[derive(Default)]
#[non_exhaustive]
pub struct TPrivateField(pub i32, pub i32, i32);
Loading