Implement the manual_non_exhaustive lint

CHANGELOG.md

@@ -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

clippy_lints/src/lib.rs

@@ -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;
@@ -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,
@@ -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),
@@ -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),
@@ -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),

clippy_lints/src/manual_non_exhaustive.rs

@@ -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");
+                    }
+                });
+        }
+    }
+}

src/lintlist/mod.rs

@@ -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",

tests/ui/manual_non_exhaustive.rs

@@ -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() {}