From f072ded3bf6286668ff8eade5b58e471dbe66f2a Mon Sep 17 00:00:00 2001
From: Eduardo Broto <ebroto@tutanota.com>
Date: Fri, 1 May 2020 01:44:17 +0200
Subject: [PATCH 1/3] Implement the manual_non_exhaustive lint
---
CHANGELOG.md | 1 +
clippy_lints/src/lib.rs | 5 +
clippy_lints/src/manual_non_exhaustive.rs | 167 ++++++++++++++++++++++
src/lintlist/mod.rs | 7 +
tests/ui/manual_non_exhaustive.rs | 124 ++++++++++++++++
tests/ui/manual_non_exhaustive.stderr | 48 +++++++
6 files changed, 352 insertions(+)
create mode 100644 clippy_lints/src/manual_non_exhaustive.rs
create mode 100644 tests/ui/manual_non_exhaustive.rs
create mode 100644 tests/ui/manual_non_exhaustive.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 575773580c0..facf363e371 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index c995be5edc2..64f3a8a0ebb 100644
--- a/clippy_lints/src/lib.rs
+++ b/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,
¯o_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),
diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs
new file mode 100644
index 00000000000..ca2a2cf2e1e
--- /dev/null
+++ b/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");
+ }
+ });
+ }
+ }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 72675c25175..c5360002fa0 100644
--- a/src/lintlist/mod.rs
+++ b/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",
diff --git a/tests/ui/manual_non_exhaustive.rs b/tests/ui/manual_non_exhaustive.rs
new file mode 100644
index 00000000000..9c239db6e00
--- /dev/null
+++ b/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() {}
diff --git a/tests/ui/manual_non_exhaustive.stderr b/tests/ui/manual_non_exhaustive.stderr
new file mode 100644
index 00000000000..d6719bca0d4
--- /dev/null
+++ b/tests/ui/manual_non_exhaustive.stderr
@@ -0,0 +1,48 @@
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:5:5
+ |
+LL | / enum E {
+LL | | A,
+LL | | B,
+LL | | #[doc(hidden)]
+LL | | _C,
+LL | | }
+ | |_____^ help: add the attribute: `#[non_exhaustive] enum E`
+ |
+ = note: `-D clippy::manual-non-exhaustive` implied by `-D warnings`
+help: and remove this variant
+ --> $DIR/manual_non_exhaustive.rs:9:9
+ |
+LL | _C,
+ | ^^
+
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:67:5
+ |
+LL | / struct S {
+LL | | pub a: i32,
+LL | | pub b: i32,
+LL | | _c: (),
+LL | | }
+ | |_____^ help: add the attribute: `#[non_exhaustive] struct S`
+ |
+help: and remove this field
+ --> $DIR/manual_non_exhaustive.rs:70:9
+ |
+LL | _c: (),
+ | ^^^^^^
+
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:108:5
+ |
+LL | struct T(pub i32, pub i32, ());
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[non_exhaustive] struct T`
+ |
+help: and remove this field
+ --> $DIR/manual_non_exhaustive.rs:108:32
+ |
+LL | struct T(pub i32, pub i32, ());
+ | ^^
+
+error: aborting due to 3 previous errors
+
From 42b0b4754c881101cefb0307c489d6159c19b2f3 Mon Sep 17 00:00:00 2001
From: Eduardo Broto <ebroto@tutanota.com>
Date: Fri, 1 May 2020 22:37:14 +0200
Subject: [PATCH 2/3] Apply suggestions from PR review
---
clippy_lints/src/manual_non_exhaustive.rs | 84 ++++++++++++-----------
tests/ui/manual_non_exhaustive.rs | 39 +++++++----
tests/ui/manual_non_exhaustive.stderr | 81 ++++++++++++++++++----
3 files changed, 139 insertions(+), 65 deletions(-)
diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs
index ca2a2cf2e1e..a4273da1d74 100644
--- a/clippy_lints/src/manual_non_exhaustive.rs
+++ b/clippy_lints/src/manual_non_exhaustive.rs
@@ -1,10 +1,11 @@
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_ast::ast::{Attribute, Item, ItemKind, StructField, 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};
+use rustc_span::Span;
declare_clippy_lint! {
/// **What it does:** Checks for manual implementations of the non-exhaustive pattern.
@@ -90,11 +91,9 @@ fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants
}
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);
+ let mut markers = variants.iter().filter(|v| is_non_exhaustive_marker(v));
+ if let Some(marker) = markers.next();
+ if markers.count() == 0 && variants.len() > 1;
then {
span_lint_and_then(
cx,
@@ -102,17 +101,20 @@ fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants
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");
+ if_chain! {
+ if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
+ let header_span = cx.sess.source_map().span_until_char(item.span, '{');
+ if let Some(snippet) = snippet_opt(cx, header_span);
+ then {
+ diag.span_suggestion(
+ header_span,
+ "add the attribute",
+ format!("#[non_exhaustive] {}", snippet),
+ Applicability::Unspecified,
+ );
+ }
}
+ diag.span_help(marker.span, "remove this variant");
});
}
}
@@ -123,8 +125,18 @@ fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data:
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)
+ fn is_non_exhaustive_marker(field: &StructField) -> bool {
+ is_private(field) && field.ty.kind.is_unit() && field.ident.map_or(true, |n| n.as_str().starts_with('_'))
+ }
+
+ fn find_header_span(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) -> Span {
+ let delimiter = match data {
+ VariantData::Struct(..) => '{',
+ VariantData::Tuple(..) => '(',
+ _ => unreachable!("`VariantData::Unit` is already handled above"),
+ };
+
+ cx.sess.source_map().span_until_char(item.span, delimiter)
}
let fields = data.fields();
@@ -132,12 +144,8 @@ fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data:
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();
+ if private_fields == 1 && public_fields >= 1 && public_fields == fields.len() - 1;
+ if let Some(marker) = fields.iter().find(|f| is_non_exhaustive_marker(f));
then {
span_lint_and_then(
cx,
@@ -145,22 +153,20 @@ fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data:
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");
+ if_chain! {
+ if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
+ let header_span = find_header_span(cx, item, data);
+ if let Some(snippet) = snippet_opt(cx, header_span);
+ then {
+ diag.span_suggestion(
+ header_span,
+ "add the attribute",
+ format!("#[non_exhaustive] {}", snippet),
+ Applicability::Unspecified,
+ );
+ }
}
+ diag.span_help(marker.span, "remove this field");
});
}
}
diff --git a/tests/ui/manual_non_exhaustive.rs b/tests/ui/manual_non_exhaustive.rs
index 9c239db6e00..7a788f48520 100644
--- a/tests/ui/manual_non_exhaustive.rs
+++ b/tests/ui/manual_non_exhaustive.rs
@@ -9,7 +9,16 @@ mod enums {
_C,
}
- // last variant does not have doc hidden attribute, should be ignored
+ // user forgot to remove the marker
+ #[non_exhaustive]
+ enum Ep {
+ A,
+ B,
+ #[doc(hidden)]
+ _C,
+ }
+
+ // marker variant does not have doc hidden attribute, should be ignored
enum NoDocHidden {
A,
B,
@@ -32,21 +41,13 @@ mod enums {
_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
+ // variant with multiple markers, should be ignored
enum MultipleMarkers {
A,
#[doc(hidden)]
@@ -55,7 +56,7 @@ mod enums {
_C,
}
- // already non_exhaustive, should be ignored
+ // already non_exhaustive and no markers, should be ignored
#[non_exhaustive]
enum NonExhaustive {
A,
@@ -70,6 +71,14 @@ mod structs {
_c: (),
}
+ // user forgot to remove the private field
+ #[non_exhaustive]
+ struct Sp {
+ pub a: i32,
+ pub b: i32,
+ _c: (),
+ }
+
// some other fields are private, should be ignored
struct PrivateFields {
a: i32,
@@ -96,7 +105,7 @@ mod structs {
_a: (),
}
- // already non exhaustive, should be ignored
+ // already non exhaustive and no private fields, should be ignored
#[non_exhaustive]
struct NonExhaustive {
pub a: i32,
@@ -107,6 +116,10 @@ mod structs {
mod tuple_structs {
struct T(pub i32, pub i32, ());
+ // user forgot to remove the private field
+ #[non_exhaustive]
+ struct Tp(pub i32, pub i32, ());
+
// some other fields are private, should be ignored
struct PrivateFields(pub i32, i32, ());
@@ -116,7 +129,7 @@ mod tuple_structs {
// private field is the only field, should be ignored
struct OnlyMarker(());
- // already non exhaustive, should be ignored
+ // already non exhaustive and no private fields, should be ignored
#[non_exhaustive]
struct NonExhaustive(pub i32, pub i32);
}
diff --git a/tests/ui/manual_non_exhaustive.stderr b/tests/ui/manual_non_exhaustive.stderr
index d6719bca0d4..613c5e8ca1d 100644
--- a/tests/ui/manual_non_exhaustive.stderr
+++ b/tests/ui/manual_non_exhaustive.stderr
@@ -1,48 +1,103 @@
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:5:5
|
-LL | / enum E {
+LL | enum E {
+ | ^-----
+ | |
+ | _____help: add the attribute: `#[non_exhaustive] enum E`
+ | |
LL | | A,
LL | | B,
LL | | #[doc(hidden)]
LL | | _C,
LL | | }
- | |_____^ help: add the attribute: `#[non_exhaustive] enum E`
+ | |_____^
|
= note: `-D clippy::manual-non-exhaustive` implied by `-D warnings`
-help: and remove this variant
+help: remove this variant
--> $DIR/manual_non_exhaustive.rs:9:9
|
LL | _C,
| ^^
error: this seems like a manual implementation of the non-exhaustive pattern
- --> $DIR/manual_non_exhaustive.rs:67:5
+ --> $DIR/manual_non_exhaustive.rs:14:5
|
-LL | / struct S {
+LL | / enum Ep {
+LL | | A,
+LL | | B,
+LL | | #[doc(hidden)]
+LL | | _C,
+LL | | }
+ | |_____^
+ |
+help: remove this variant
+ --> $DIR/manual_non_exhaustive.rs:18:9
+ |
+LL | _C,
+ | ^^
+
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:68:5
+ |
+LL | struct S {
+ | ^-------
+ | |
+ | _____help: add the attribute: `#[non_exhaustive] struct S`
+ | |
LL | | pub a: i32,
LL | | pub b: i32,
LL | | _c: (),
LL | | }
- | |_____^ help: add the attribute: `#[non_exhaustive] struct S`
+ | |_____^
|
-help: and remove this field
- --> $DIR/manual_non_exhaustive.rs:70:9
+help: remove this field
+ --> $DIR/manual_non_exhaustive.rs:71:9
|
LL | _c: (),
| ^^^^^^
error: this seems like a manual implementation of the non-exhaustive pattern
- --> $DIR/manual_non_exhaustive.rs:108:5
+ --> $DIR/manual_non_exhaustive.rs:76:5
+ |
+LL | / struct Sp {
+LL | | pub a: i32,
+LL | | pub b: i32,
+LL | | _c: (),
+LL | | }
+ | |_____^
+ |
+help: remove this field
+ --> $DIR/manual_non_exhaustive.rs:79:9
+ |
+LL | _c: (),
+ | ^^^^^^
+
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:117:5
|
LL | struct T(pub i32, pub i32, ());
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[non_exhaustive] struct T`
+ | --------^^^^^^^^^^^^^^^^^^^^^^^
+ | |
+ | help: add the attribute: `#[non_exhaustive] struct T`
|
-help: and remove this field
- --> $DIR/manual_non_exhaustive.rs:108:32
+help: remove this field
+ --> $DIR/manual_non_exhaustive.rs:117:32
|
LL | struct T(pub i32, pub i32, ());
| ^^
-error: aborting due to 3 previous errors
+error: this seems like a manual implementation of the non-exhaustive pattern
+ --> $DIR/manual_non_exhaustive.rs:121:5
+ |
+LL | struct Tp(pub i32, pub i32, ());
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+help: remove this field
+ --> $DIR/manual_non_exhaustive.rs:121:33
+ |
+LL | struct Tp(pub i32, pub i32, ());
+ | ^^
+
+error: aborting due to 6 previous errors
From 350c17de24c0bc7ee1b17981fe02f88ca6ec50a4 Mon Sep 17 00:00:00 2001
From: ebroto <ebroto@tutanota.com>
Date: Fri, 1 May 2020 23:00:16 +0200
Subject: [PATCH 3/3] Use the only variant left instead of a wildcard
Co-authored-by: Philipp Krones <hello@philkrones.com>
---
clippy_lints/src/manual_non_exhaustive.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs
index a4273da1d74..f3b8902e26f 100644
--- a/clippy_lints/src/manual_non_exhaustive.rs
+++ b/clippy_lints/src/manual_non_exhaustive.rs
@@ -133,7 +133,7 @@ fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data:
let delimiter = match data {
VariantData::Struct(..) => '{',
VariantData::Tuple(..) => '(',
- _ => unreachable!("`VariantData::Unit` is already handled above"),
+ VariantData::Unit(_) => unreachable!("`VariantData::Unit` is already handled above"),
};
cx.sess.source_map().span_until_char(item.span, delimiter)