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
This commit is contained in:
bors
commit
cc9088f287
@ -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
|
||||
|
||||
@ -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),
|
||||
@ -1281,6 +1284,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),
|
||||
|
||||
173
clippy_lints/src/manual_non_exhaustive.rs
Normal file
173
clippy_lints/src/manual_non_exhaustive.rs
Normal file
@ -0,0 +1,173 @@
|
||||
use crate::utils::{snippet_opt, span_lint_and_then};
|
||||
use if_chain::if_chain;
|
||||
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.
|
||||
///
|
||||
/// **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! {
|
||||
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,
|
||||
MANUAL_NON_EXHAUSTIVE,
|
||||
item.span,
|
||||
"this seems like a manual implementation of the non-exhaustive pattern",
|
||||
|diag| {
|
||||
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");
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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(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(..) => '(',
|
||||
VariantData::Unit(_) => unreachable!("`VariantData::Unit` is already handled above"),
|
||||
};
|
||||
|
||||
cx.sess.source_map().span_until_char(item.span, delimiter)
|
||||
}
|
||||
|
||||
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 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,
|
||||
MANUAL_NON_EXHAUSTIVE,
|
||||
item.span,
|
||||
"this seems like a manual implementation of the non-exhaustive pattern",
|
||||
|diag| {
|
||||
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");
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -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",
|
||||
|
||||
137
tests/ui/manual_non_exhaustive.rs
Normal file
137
tests/ui/manual_non_exhaustive.rs
Normal file
@ -0,0 +1,137 @@
|
||||
#![warn(clippy::manual_non_exhaustive)]
|
||||
#![allow(unused)]
|
||||
|
||||
mod enums {
|
||||
enum E {
|
||||
A,
|
||||
B,
|
||||
#[doc(hidden)]
|
||||
_C,
|
||||
}
|
||||
|
||||
// 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,
|
||||
_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 the only one, should be ignored
|
||||
enum OnlyMarker {
|
||||
#[doc(hidden)]
|
||||
_A,
|
||||
}
|
||||
|
||||
// variant with multiple markers, should be ignored
|
||||
enum MultipleMarkers {
|
||||
A,
|
||||
#[doc(hidden)]
|
||||
_B,
|
||||
#[doc(hidden)]
|
||||
_C,
|
||||
}
|
||||
|
||||
// already non_exhaustive and no markers, should be ignored
|
||||
#[non_exhaustive]
|
||||
enum NonExhaustive {
|
||||
A,
|
||||
B,
|
||||
}
|
||||
}
|
||||
|
||||
mod structs {
|
||||
struct S {
|
||||
pub a: i32,
|
||||
pub b: i32,
|
||||
_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,
|
||||
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 and no private fields, should be ignored
|
||||
#[non_exhaustive]
|
||||
struct NonExhaustive {
|
||||
pub a: i32,
|
||||
pub b: i32,
|
||||
}
|
||||
}
|
||||
|
||||
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, ());
|
||||
|
||||
// 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 and no private fields, should be ignored
|
||||
#[non_exhaustive]
|
||||
struct NonExhaustive(pub i32, pub i32);
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
103
tests/ui/manual_non_exhaustive.stderr
Normal file
103
tests/ui/manual_non_exhaustive.stderr
Normal file
@ -0,0 +1,103 @@
|
||||
error: this seems like a manual implementation of the non-exhaustive pattern
|
||||
--> $DIR/manual_non_exhaustive.rs:5:5
|
||||
|
|
||||
LL | enum E {
|
||||
| ^-----
|
||||
| |
|
||||
| _____help: add the attribute: `#[non_exhaustive] enum E`
|
||||
| |
|
||||
LL | | A,
|
||||
LL | | B,
|
||||
LL | | #[doc(hidden)]
|
||||
LL | | _C,
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
= note: `-D clippy::manual-non-exhaustive` implied by `-D warnings`
|
||||
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:14:5
|
||||
|
|
||||
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: 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: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: remove this field
|
||||
--> $DIR/manual_non_exhaustive.rs:117:32
|
||||
|
|
||||
LL | struct T(pub i32, pub i32, ());
|
||||
| ^^
|
||||
|
||||
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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user