mirror of
https://github.com/rust-lang/rust.git
synced 2025-01-18 18:53:04 +00:00
Apply suggestions from PR review
This commit is contained in:
Eduardo Broto
parent
f072ded3bf
commit
42b0b4754c
@ -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");
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@ -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);
|
||||
}
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user