Skip to content

Commit

Permalink
Implement the manual_non_exhaustive lint
Browse files Browse the repository at this point in the history
  • Loading branch information
ebroto committed Apr 30, 2020
1 parent 0a53ed2 commit 1b49ae7
Show file tree
Hide file tree
Showing 6 changed files with 352 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,7 @@ Released 2018-09-13
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ mod literal_representation;
mod loops;
mod macro_use;
mod main_recursion;
mod manual_non_exhaustive;
mod map_clone;
mod map_unit_fn;
mod match_on_vec_items;
Expand Down Expand Up @@ -628,6 +629,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&loops::WHILE_LET_ON_ITERATOR,
&macro_use::MACRO_USE_IMPORTS,
&main_recursion::MAIN_RECURSION,
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
&map_clone::MAP_CLONE,
&map_unit_fn::OPTION_MAP_UNIT_FN,
&map_unit_fn::RESULT_MAP_UNIT_FN,
Expand Down Expand Up @@ -1064,6 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
store.register_late_pass(|| box if_let_mutex::IfLetMutex);
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1280,6 +1283,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::WHILE_LET_LOOP),
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(&map_clone::MAP_CLONE),
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
Expand Down Expand Up @@ -1474,6 +1478,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(&map_clone::MAP_CLONE),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
Expand Down
167 changes: 167 additions & 0 deletions clippy_lints/src/manual_non_exhaustive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
use crate::utils::{snippet_opt, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::{Attribute, Ident, Item, ItemKind, StructField, TyKind, Variant, VariantData, VisibilityKind};
use rustc_attr as attr;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for manual implementations of the non-exhaustive pattern.
///
/// **Why is this bad?** Using the #[non_exhaustive] attribute expresses better the intent
/// and allows possible optimizations when applied to enums.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// struct S {
/// pub a: i32,
/// pub b: i32,
/// _c: (),
/// }
///
/// enum E {
/// A,
/// B,
/// #[doc(hidden)]
/// _C,
/// }
///
/// struct T(pub i32, pub i32, ());
/// ```
/// Use instead:
/// ```rust
/// #[non_exhaustive]
/// struct S {
/// pub a: i32,
/// pub b: i32,
/// }
///
/// #[non_exhaustive]
/// enum E {
/// A,
/// B,
/// }
///
/// #[non_exhaustive]
/// struct T(pub i32, pub i32);
/// ```
pub MANUAL_NON_EXHAUSTIVE,
style,
"manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]"
}

declare_lint_pass!(ManualNonExhaustive => [MANUAL_NON_EXHAUSTIVE]);

impl EarlyLintPass for ManualNonExhaustive {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
match &item.kind {
ItemKind::Enum(def, _) => {
check_manual_non_exhaustive_enum(cx, item, &def.variants);
},
ItemKind::Struct(variant_data, _) => {
if let VariantData::Unit(..) = variant_data {
return;
}

check_manual_non_exhaustive_struct(cx, item, variant_data);
},
_ => {},
}
}
}

fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants: &[Variant]) {
fn is_non_exhaustive_marker(variant: &Variant) -> bool {
matches!(variant.data, VariantData::Unit(_))
&& variant.ident.as_str().starts_with('_')
&& variant.attrs.iter().any(|a| is_doc_hidden(a))
}

fn is_doc_hidden(attr: &Attribute) -> bool {
attr.check_name(sym!(doc))
&& match attr.meta_item_list() {
Some(l) => attr::list_contains_name(&l, sym!(hidden)),
None => false,
}
}

if_chain! {
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
let markers = variants.iter().filter(|v| is_non_exhaustive_marker(v)).count();
if markers == 1 && variants.len() > markers;
if let Some(variant) = variants.last();
if is_non_exhaustive_marker(variant);
then {
span_lint_and_then(
cx,
MANUAL_NON_EXHAUSTIVE,
item.span,
"this seems like a manual implementation of the non-exhaustive pattern",
|diag| {
let header_span = cx.sess.source_map().span_until_char(item.span, '{');

if let Some(snippet) = snippet_opt(cx, header_span) {
diag.span_suggestion(
item.span,
"add the attribute",
format!("#[non_exhaustive] {}", snippet),
Applicability::Unspecified,
);
diag.span_help(variant.span, "and remove this variant");
}
});
}
}
}

fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) {
fn is_private(field: &StructField) -> bool {
matches!(field.vis.node, VisibilityKind::Inherited)
}

fn is_non_exhaustive_marker(name: &Option<Ident>) -> bool {
name.map(|n| n.as_str().starts_with('_')).unwrap_or(true)
}

let fields = data.fields();
let private_fields = fields.iter().filter(|f| is_private(f)).count();
let public_fields = fields.iter().filter(|f| f.vis.node.is_pub()).count();

if_chain! {
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
if private_fields == 1 && public_fields > private_fields && public_fields == fields.len() - 1;
if let Some(field) = fields.iter().find(|f| is_private(f));
if is_non_exhaustive_marker(&field.ident);
if let TyKind::Tup(tup_fields) = &field.ty.kind;
if tup_fields.is_empty();
then {
span_lint_and_then(
cx,
MANUAL_NON_EXHAUSTIVE,
item.span,
"this seems like a manual implementation of the non-exhaustive pattern",
|diag| {
let delimiter = match data {
VariantData::Struct(..) => '{',
VariantData::Tuple(..) => '(',
_ => unreachable!(),
};
let header_span = cx.sess.source_map().span_until_char(item.span, delimiter);

if let Some(snippet) = snippet_opt(cx, header_span) {
diag.span_suggestion(
item.span,
"add the attribute",
format!("#[non_exhaustive] {}", snippet),
Applicability::Unspecified,
);
diag.span_help(field.span, "and remove this field");
}
});
}
}
}
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "loops",
},
Lint {
name: "manual_non_exhaustive",
group: "style",
desc: "manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]",
deprecation: None,
module: "manual_non_exhaustive",
},
Lint {
name: "manual_saturating_arithmetic",
group: "style",
Expand Down
124 changes: 124 additions & 0 deletions tests/ui/manual_non_exhaustive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
#![warn(clippy::manual_non_exhaustive)]
#![allow(unused)]

mod enums {
enum E {
A,
B,
#[doc(hidden)]
_C,
}

// last variant does not have doc hidden attribute, should be ignored
enum NoDocHidden {
A,
B,
_C,
}

// name of variant with doc hidden does not start with underscore, should be ignored
enum NoUnderscore {
A,
B,
#[doc(hidden)]
C,
}

// variant with doc hidden is not unit, should be ignored
enum NotUnit {
A,
B,
#[doc(hidden)]
_C(bool),
}

// variant with doc hidden is not the last one, should be ignored
enum NotLast {
A,
#[doc(hidden)]
_B,
C,
}

// variant with doc hidden is the only one, should be ignored
enum OnlyMarker {
#[doc(hidden)]
_A,
}

// variant with multiple non-exhaustive "markers", should be ignored
enum MultipleMarkers {
A,
#[doc(hidden)]
_B,
#[doc(hidden)]
_C,
}

// already non_exhaustive, should be ignored
#[non_exhaustive]
enum NonExhaustive {
A,
B,
}
}

mod structs {
struct S {
pub a: i32,
pub b: i32,
_c: (),
}

// some other fields are private, should be ignored
struct PrivateFields {
a: i32,
pub b: i32,
_c: (),
}

// private field name does not start with underscore, should be ignored
struct NoUnderscore {
pub a: i32,
pub b: i32,
c: (),
}

// private field is not unit type, should be ignored
struct NotUnit {
pub a: i32,
pub b: i32,
_c: i32,
}

// private field is the only field, should be ignored
struct OnlyMarker {
_a: (),
}

// already non exhaustive, should be ignored
#[non_exhaustive]
struct NonExhaustive {
pub a: i32,
pub b: i32,
}
}

mod tuple_structs {
struct T(pub i32, pub i32, ());

// some other fields are private, should be ignored
struct PrivateFields(pub i32, i32, ());

// private field is not unit type, should be ignored
struct NotUnit(pub i32, pub i32, i32);

// private field is the only field, should be ignored
struct OnlyMarker(());

// already non exhaustive, should be ignored
#[non_exhaustive]
struct NonExhaustive(pub i32, pub i32);
}

fn main() {}
Loading

0 comments on commit 1b49ae7

Please sign in to comment.