Skip to content

Commit cc9088f

Browse files
committed
Auto merge of #5550 - ebroto:manual_non_exhaustive, r=flip1995
Implement the manual_non_exhaustive lint Some implementation notes: * Not providing automatic fixups because additional changes may be needed in other parts of the code, e.g. when constructing a struct. * Even though the attribute is valid on enum variants, it's not possible to use the manual implementation of the pattern because the visibility is always public, so the lint ignores enum variants. * Unit structs are also ignored, it's not possible to implement the pattern manually without fields. * The attribute is not accepted in unions, so those are ignored too. * Even though the original issue did not mention it, tuple structs are also linted because it's possible to apply the pattern manually. changelog: Added the manual non-exhaustive implementation lint Closes #2017
2 parents 991efa6 + 350c17d commit cc9088f

File tree

6 files changed

+426
-0
lines changed

6 files changed

+426
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1423,6 +1423,7 @@ Released 2018-09-13
14231423
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
14241424
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
14251425
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
1426+
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
14261427
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
14271428
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
14281429
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ mod literal_representation;
247247
mod loops;
248248
mod macro_use;
249249
mod main_recursion;
250+
mod manual_non_exhaustive;
250251
mod map_clone;
251252
mod map_unit_fn;
252253
mod match_on_vec_items;
@@ -628,6 +629,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
628629
&loops::WHILE_LET_ON_ITERATOR,
629630
&macro_use::MACRO_USE_IMPORTS,
630631
&main_recursion::MAIN_RECURSION,
632+
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
631633
&map_clone::MAP_CLONE,
632634
&map_unit_fn::OPTION_MAP_UNIT_FN,
633635
&map_unit_fn::RESULT_MAP_UNIT_FN,
@@ -1064,6 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10641066
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
10651067
store.register_late_pass(|| box if_let_mutex::IfLetMutex);
10661068
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
1069+
store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive);
10671070

10681071
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10691072
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1281,6 +1284,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12811284
LintId::of(&loops::WHILE_LET_LOOP),
12821285
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
12831286
LintId::of(&main_recursion::MAIN_RECURSION),
1287+
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
12841288
LintId::of(&map_clone::MAP_CLONE),
12851289
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
12861290
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:
14741478
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
14751479
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
14761480
LintId::of(&main_recursion::MAIN_RECURSION),
1481+
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
14771482
LintId::of(&map_clone::MAP_CLONE),
14781483
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
14791484
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
+173
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
use crate::utils::{snippet_opt, span_lint_and_then};
2+
use if_chain::if_chain;
3+
use rustc_ast::ast::{Attribute, Item, ItemKind, StructField, Variant, VariantData, VisibilityKind};
4+
use rustc_attr as attr;
5+
use rustc_errors::Applicability;
6+
use rustc_lint::{EarlyContext, EarlyLintPass};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::Span;
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for manual implementations of the non-exhaustive pattern.
12+
///
13+
/// **Why is this bad?** Using the #[non_exhaustive] attribute expresses better the intent
14+
/// and allows possible optimizations when applied to enums.
15+
///
16+
/// **Known problems:** None.
17+
///
18+
/// **Example:**
19+
///
20+
/// ```rust
21+
/// struct S {
22+
/// pub a: i32,
23+
/// pub b: i32,
24+
/// _c: (),
25+
/// }
26+
///
27+
/// enum E {
28+
/// A,
29+
/// B,
30+
/// #[doc(hidden)]
31+
/// _C,
32+
/// }
33+
///
34+
/// struct T(pub i32, pub i32, ());
35+
/// ```
36+
/// Use instead:
37+
/// ```rust
38+
/// #[non_exhaustive]
39+
/// struct S {
40+
/// pub a: i32,
41+
/// pub b: i32,
42+
/// }
43+
///
44+
/// #[non_exhaustive]
45+
/// enum E {
46+
/// A,
47+
/// B,
48+
/// }
49+
///
50+
/// #[non_exhaustive]
51+
/// struct T(pub i32, pub i32);
52+
/// ```
53+
pub MANUAL_NON_EXHAUSTIVE,
54+
style,
55+
"manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]"
56+
}
57+
58+
declare_lint_pass!(ManualNonExhaustive => [MANUAL_NON_EXHAUSTIVE]);
59+
60+
impl EarlyLintPass for ManualNonExhaustive {
61+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
62+
match &item.kind {
63+
ItemKind::Enum(def, _) => {
64+
check_manual_non_exhaustive_enum(cx, item, &def.variants);
65+
},
66+
ItemKind::Struct(variant_data, _) => {
67+
if let VariantData::Unit(..) = variant_data {
68+
return;
69+
}
70+
71+
check_manual_non_exhaustive_struct(cx, item, variant_data);
72+
},
73+
_ => {},
74+
}
75+
}
76+
}
77+
78+
fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants: &[Variant]) {
79+
fn is_non_exhaustive_marker(variant: &Variant) -> bool {
80+
matches!(variant.data, VariantData::Unit(_))
81+
&& variant.ident.as_str().starts_with('_')
82+
&& variant.attrs.iter().any(|a| is_doc_hidden(a))
83+
}
84+
85+
fn is_doc_hidden(attr: &Attribute) -> bool {
86+
attr.check_name(sym!(doc))
87+
&& match attr.meta_item_list() {
88+
Some(l) => attr::list_contains_name(&l, sym!(hidden)),
89+
None => false,
90+
}
91+
}
92+
93+
if_chain! {
94+
let mut markers = variants.iter().filter(|v| is_non_exhaustive_marker(v));
95+
if let Some(marker) = markers.next();
96+
if markers.count() == 0 && variants.len() > 1;
97+
then {
98+
span_lint_and_then(
99+
cx,
100+
MANUAL_NON_EXHAUSTIVE,
101+
item.span,
102+
"this seems like a manual implementation of the non-exhaustive pattern",
103+
|diag| {
104+
if_chain! {
105+
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
106+
let header_span = cx.sess.source_map().span_until_char(item.span, '{');
107+
if let Some(snippet) = snippet_opt(cx, header_span);
108+
then {
109+
diag.span_suggestion(
110+
header_span,
111+
"add the attribute",
112+
format!("#[non_exhaustive] {}", snippet),
113+
Applicability::Unspecified,
114+
);
115+
}
116+
}
117+
diag.span_help(marker.span, "remove this variant");
118+
});
119+
}
120+
}
121+
}
122+
123+
fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) {
124+
fn is_private(field: &StructField) -> bool {
125+
matches!(field.vis.node, VisibilityKind::Inherited)
126+
}
127+
128+
fn is_non_exhaustive_marker(field: &StructField) -> bool {
129+
is_private(field) && field.ty.kind.is_unit() && field.ident.map_or(true, |n| n.as_str().starts_with('_'))
130+
}
131+
132+
fn find_header_span(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) -> Span {
133+
let delimiter = match data {
134+
VariantData::Struct(..) => '{',
135+
VariantData::Tuple(..) => '(',
136+
VariantData::Unit(_) => unreachable!("`VariantData::Unit` is already handled above"),
137+
};
138+
139+
cx.sess.source_map().span_until_char(item.span, delimiter)
140+
}
141+
142+
let fields = data.fields();
143+
let private_fields = fields.iter().filter(|f| is_private(f)).count();
144+
let public_fields = fields.iter().filter(|f| f.vis.node.is_pub()).count();
145+
146+
if_chain! {
147+
if private_fields == 1 && public_fields >= 1 && public_fields == fields.len() - 1;
148+
if let Some(marker) = fields.iter().find(|f| is_non_exhaustive_marker(f));
149+
then {
150+
span_lint_and_then(
151+
cx,
152+
MANUAL_NON_EXHAUSTIVE,
153+
item.span,
154+
"this seems like a manual implementation of the non-exhaustive pattern",
155+
|diag| {
156+
if_chain! {
157+
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
158+
let header_span = find_header_span(cx, item, data);
159+
if let Some(snippet) = snippet_opt(cx, header_span);
160+
then {
161+
diag.span_suggestion(
162+
header_span,
163+
"add the attribute",
164+
format!("#[non_exhaustive] {}", snippet),
165+
Applicability::Unspecified,
166+
);
167+
}
168+
}
169+
diag.span_help(marker.span, "remove this field");
170+
});
171+
}
172+
}
173+
}

src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
10881088
deprecation: None,
10891089
module: "loops",
10901090
},
1091+
Lint {
1092+
name: "manual_non_exhaustive",
1093+
group: "style",
1094+
desc: "manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]",
1095+
deprecation: None,
1096+
module: "manual_non_exhaustive",
1097+
},
10911098
Lint {
10921099
name: "manual_saturating_arithmetic",
10931100
group: "style",

tests/ui/manual_non_exhaustive.rs

+137
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
#![warn(clippy::manual_non_exhaustive)]
2+
#![allow(unused)]
3+
4+
mod enums {
5+
enum E {
6+
A,
7+
B,
8+
#[doc(hidden)]
9+
_C,
10+
}
11+
12+
// user forgot to remove the marker
13+
#[non_exhaustive]
14+
enum Ep {
15+
A,
16+
B,
17+
#[doc(hidden)]
18+
_C,
19+
}
20+
21+
// marker variant does not have doc hidden attribute, should be ignored
22+
enum NoDocHidden {
23+
A,
24+
B,
25+
_C,
26+
}
27+
28+
// name of variant with doc hidden does not start with underscore, should be ignored
29+
enum NoUnderscore {
30+
A,
31+
B,
32+
#[doc(hidden)]
33+
C,
34+
}
35+
36+
// variant with doc hidden is not unit, should be ignored
37+
enum NotUnit {
38+
A,
39+
B,
40+
#[doc(hidden)]
41+
_C(bool),
42+
}
43+
44+
// variant with doc hidden is the only one, should be ignored
45+
enum OnlyMarker {
46+
#[doc(hidden)]
47+
_A,
48+
}
49+
50+
// variant with multiple markers, should be ignored
51+
enum MultipleMarkers {
52+
A,
53+
#[doc(hidden)]
54+
_B,
55+
#[doc(hidden)]
56+
_C,
57+
}
58+
59+
// already non_exhaustive and no markers, should be ignored
60+
#[non_exhaustive]
61+
enum NonExhaustive {
62+
A,
63+
B,
64+
}
65+
}
66+
67+
mod structs {
68+
struct S {
69+
pub a: i32,
70+
pub b: i32,
71+
_c: (),
72+
}
73+
74+
// user forgot to remove the private field
75+
#[non_exhaustive]
76+
struct Sp {
77+
pub a: i32,
78+
pub b: i32,
79+
_c: (),
80+
}
81+
82+
// some other fields are private, should be ignored
83+
struct PrivateFields {
84+
a: i32,
85+
pub b: i32,
86+
_c: (),
87+
}
88+
89+
// private field name does not start with underscore, should be ignored
90+
struct NoUnderscore {
91+
pub a: i32,
92+
pub b: i32,
93+
c: (),
94+
}
95+
96+
// private field is not unit type, should be ignored
97+
struct NotUnit {
98+
pub a: i32,
99+
pub b: i32,
100+
_c: i32,
101+
}
102+
103+
// private field is the only field, should be ignored
104+
struct OnlyMarker {
105+
_a: (),
106+
}
107+
108+
// already non exhaustive and no private fields, should be ignored
109+
#[non_exhaustive]
110+
struct NonExhaustive {
111+
pub a: i32,
112+
pub b: i32,
113+
}
114+
}
115+
116+
mod tuple_structs {
117+
struct T(pub i32, pub i32, ());
118+
119+
// user forgot to remove the private field
120+
#[non_exhaustive]
121+
struct Tp(pub i32, pub i32, ());
122+
123+
// some other fields are private, should be ignored
124+
struct PrivateFields(pub i32, i32, ());
125+
126+
// private field is not unit type, should be ignored
127+
struct NotUnit(pub i32, pub i32, i32);
128+
129+
// private field is the only field, should be ignored
130+
struct OnlyMarker(());
131+
132+
// already non exhaustive and no private fields, should be ignored
133+
#[non_exhaustive]
134+
struct NonExhaustive(pub i32, pub i32);
135+
}
136+
137+
fn main() {}

0 commit comments

Comments
 (0)