From f1ab3024b27cc7c02a80fd54382a10a1b4ef3bcd Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 12:48:30 -0800 Subject: [PATCH 1/9] New lint: exhaustive_enums --- CHANGELOG.md | 1 + clippy_lints/src/exhaustive_enums.rs | 68 ++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ tests/ui/exhaustive_enums.fixed | 24 ++++++++++ tests/ui/exhaustive_enums.rs | 23 ++++++++++ tests/ui/exhaustive_enums.stderr | 28 ++++++++++++ 6 files changed, 148 insertions(+) create mode 100644 clippy_lints/src/exhaustive_enums.rs create mode 100644 tests/ui/exhaustive_enums.fixed create mode 100644 tests/ui/exhaustive_enums.rs create mode 100644 tests/ui/exhaustive_enums.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 04f042b2deb..4cf2125ea2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1938,6 +1938,7 @@ Released 2018-09-13 [`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op [`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence [`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision +[`exhaustive_enums`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums [`exit`]: https://rust-lang.github.io/rust-clippy/master/index.html#exit [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call [`expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_used diff --git a/clippy_lints/src/exhaustive_enums.rs b/clippy_lints/src/exhaustive_enums.rs new file mode 100644 index 00000000000..099171962d3 --- /dev/null +++ b/clippy_lints/src/exhaustive_enums.rs @@ -0,0 +1,68 @@ +use crate::utils::{snippet_opt, span_lint_and_help, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::ast::{Item, ItemKind}; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// **What it does:** Warns on any `enum`s that are not tagged `#[non_exhaustive]` + /// + /// **Why is this bad?** Exhaustive enums are typically fine, but a project which does + /// not wish to make a stability commitment around enums may wish to disable them by default. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// enum Foo { + /// Bar, + /// Baz + /// } + /// ``` + /// Use instead: + /// ```rust + /// #[non_exhaustive] + /// enum Foo { + /// Bar, + /// Baz + /// } /// ``` + pub EXHAUSTIVE_ENUMS, + restriction, + "default lint description" +} + +declare_lint_pass!(ExhaustiveEnums => [EXHAUSTIVE_ENUMS]); + +impl EarlyLintPass for ExhaustiveEnums { + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + if_chain! { + if let ItemKind::Enum(..) = item.kind; + if !item.attrs.iter().any(|a| a.has_name(sym::non_exhaustive)); + then { + if let Some(snippet) = snippet_opt(cx, item.span) { + span_lint_and_sugg( + cx, + EXHAUSTIVE_ENUMS, + item.span, + "enums should not be exhaustive", + "try adding #[non_exhaustive]", + format!("#[non_exhaustive]\n{}", snippet), + Applicability::MaybeIncorrect, + ); + } else { + span_lint_and_help( + cx, + EXHAUSTIVE_ENUMS, + item.span, + "enums should not be exhaustive", + None, + "try adding #[non_exhaustive]", + ); + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 53764bb7390..465ad3846ce 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -200,6 +200,7 @@ macro_rules! declare_clippy_lint { mod eta_reduction; mod eval_order_dependence; mod excessive_bools; +mod exhaustive_enums; mod exit; mod explicit_write; mod fallible_impl_from; @@ -611,6 +612,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &eval_order_dependence::EVAL_ORDER_DEPENDENCE, &excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS, &excessive_bools::STRUCT_EXCESSIVE_BOOLS, + &exhaustive_enums::EXHAUSTIVE_ENUMS, &exit::EXIT, &explicit_write::EXPLICIT_WRITE, &fallible_impl_from::FALLIBLE_IMPL_FROM, @@ -1096,6 +1098,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box eval_order_dependence::EvalOrderDependence); store.register_late_pass(|| box missing_doc::MissingDoc::new()); store.register_late_pass(|| box missing_inline::MissingInline); + store.register_early_pass(move || box exhaustive_enums::ExhaustiveEnums); store.register_late_pass(|| box if_let_some_result::OkIfLet); store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl); store.register_late_pass(|| box unused_io_amount::UnusedIoAmount); @@ -1246,6 +1249,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&create_dir::CREATE_DIR), LintId::of(&dbg_macro::DBG_MACRO), LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), + LintId::of(&exhaustive_enums::EXHAUSTIVE_ENUMS), LintId::of(&exit::EXIT), LintId::of(&float_literal::LOSSY_FLOAT_LITERAL), LintId::of(&implicit_return::IMPLICIT_RETURN), diff --git a/tests/ui/exhaustive_enums.fixed b/tests/ui/exhaustive_enums.fixed new file mode 100644 index 00000000000..2d5f0474bc0 --- /dev/null +++ b/tests/ui/exhaustive_enums.fixed @@ -0,0 +1,24 @@ +// run-rustfix + +#![deny(clippy::exhaustive_enums)] +#![allow(unused)] + +fn main() { + // nop +} + +#[non_exhaustive] +enum Exhaustive { + Foo, + Bar, + Baz, + Quux(String), +} + +#[non_exhaustive] +enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), +} diff --git a/tests/ui/exhaustive_enums.rs b/tests/ui/exhaustive_enums.rs new file mode 100644 index 00000000000..5c88454ae61 --- /dev/null +++ b/tests/ui/exhaustive_enums.rs @@ -0,0 +1,23 @@ +// run-rustfix + +#![deny(clippy::exhaustive_enums)] +#![allow(unused)] + +fn main() { + // nop +} + +enum Exhaustive { + Foo, + Bar, + Baz, + Quux(String), +} + +#[non_exhaustive] +enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), +} diff --git a/tests/ui/exhaustive_enums.stderr b/tests/ui/exhaustive_enums.stderr new file mode 100644 index 00000000000..ee5a1836267 --- /dev/null +++ b/tests/ui/exhaustive_enums.stderr @@ -0,0 +1,28 @@ +error: enums should not be exhaustive + --> $DIR/exhaustive_enums.rs:10:1 + | +LL | / enum Exhaustive { +LL | | Foo, +LL | | Bar, +LL | | Baz, +LL | | Quux(String), +LL | | } + | |_^ + | +note: the lint level is defined here + --> $DIR/exhaustive_enums.rs:3:9 + | +LL | #![deny(clippy::exhaustive_enums)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ +help: try adding #[non_exhaustive] + | +LL | #[non_exhaustive] +LL | enum Exhaustive { +LL | Foo, +LL | Bar, +LL | Baz, +LL | Quux(String), + ... + +error: aborting due to previous error + From dc93188805ac20fbccd7bd616a6b114680e9303a Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 13:31:15 -0800 Subject: [PATCH 2/9] Make exhaustive_enums a late pass --- clippy_lints/src/exhaustive_enums.rs | 8 ++++---- clippy_lints/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/exhaustive_enums.rs b/clippy_lints/src/exhaustive_enums.rs index 099171962d3..1391ebd9e31 100644 --- a/clippy_lints/src/exhaustive_enums.rs +++ b/clippy_lints/src/exhaustive_enums.rs @@ -1,8 +1,8 @@ use crate::utils::{snippet_opt, span_lint_and_help, span_lint_and_sugg}; use if_chain::if_chain; -use rustc_ast::ast::{Item, ItemKind}; +use rustc_hir::{Item, ItemKind}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -36,8 +36,8 @@ declare_lint_pass!(ExhaustiveEnums => [EXHAUSTIVE_ENUMS]); -impl EarlyLintPass for ExhaustiveEnums { - fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { +impl LateLintPass<'_> for ExhaustiveEnums { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if_chain! { if let ItemKind::Enum(..) = item.kind; if !item.attrs.iter().any(|a| a.has_name(sym::non_exhaustive)); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 465ad3846ce..becd9f333fd 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1098,7 +1098,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box eval_order_dependence::EvalOrderDependence); store.register_late_pass(|| box missing_doc::MissingDoc::new()); store.register_late_pass(|| box missing_inline::MissingInline); - store.register_early_pass(move || box exhaustive_enums::ExhaustiveEnums); + store.register_late_pass(move || box exhaustive_enums::ExhaustiveEnums); store.register_late_pass(|| box if_let_some_result::OkIfLet); store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl); store.register_late_pass(|| box unused_io_amount::UnusedIoAmount); From f6cb96ef07ac6197dac5be16adbe2b7950c82d99 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 13:34:44 -0800 Subject: [PATCH 3/9] Make exhaustive_enums only warn on exported items --- clippy_lints/src/exhaustive_enums.rs | 3 ++- tests/ui/exhaustive_enums.fixed | 22 ++++++++++++++++++++-- tests/ui/exhaustive_enums.rs | 22 ++++++++++++++++++++-- tests/ui/exhaustive_enums.stderr | 4 ++-- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/exhaustive_enums.rs b/clippy_lints/src/exhaustive_enums.rs index 1391ebd9e31..2e1c0728d2c 100644 --- a/clippy_lints/src/exhaustive_enums.rs +++ b/clippy_lints/src/exhaustive_enums.rs @@ -7,7 +7,7 @@ use rustc_span::sym; declare_clippy_lint! { - /// **What it does:** Warns on any `enum`s that are not tagged `#[non_exhaustive]` + /// **What it does:** Warns on any exported `enum`s that are not tagged `#[non_exhaustive]` /// /// **Why is this bad?** Exhaustive enums are typically fine, but a project which does /// not wish to make a stability commitment around enums may wish to disable them by default. @@ -40,6 +40,7 @@ impl LateLintPass<'_> for ExhaustiveEnums { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if_chain! { if let ItemKind::Enum(..) = item.kind; + if cx.access_levels.is_exported(item.hir_id); if !item.attrs.iter().any(|a| a.has_name(sym::non_exhaustive)); then { if let Some(snippet) = snippet_opt(cx, item.span) { diff --git a/tests/ui/exhaustive_enums.fixed b/tests/ui/exhaustive_enums.fixed index 2d5f0474bc0..71c4a251e3b 100644 --- a/tests/ui/exhaustive_enums.fixed +++ b/tests/ui/exhaustive_enums.fixed @@ -8,15 +8,33 @@ fn main() { } #[non_exhaustive] -enum Exhaustive { +pub enum Exhaustive { Foo, Bar, Baz, Quux(String), } +// no warning, already non_exhaustive #[non_exhaustive] -enum NonExhaustive { +pub enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), +} + +// no warning, private +enum ExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), +} + +// no warning, private +#[non_exhaustive] +enum NonExhaustivePrivate { Foo, Bar, Baz, diff --git a/tests/ui/exhaustive_enums.rs b/tests/ui/exhaustive_enums.rs index 5c88454ae61..45af6851dd1 100644 --- a/tests/ui/exhaustive_enums.rs +++ b/tests/ui/exhaustive_enums.rs @@ -7,15 +7,33 @@ fn main() { // nop } -enum Exhaustive { +pub enum Exhaustive { Foo, Bar, Baz, Quux(String), } +// no warning, already non_exhaustive #[non_exhaustive] -enum NonExhaustive { +pub enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), +} + +// no warning, private +enum ExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), +} + +// no warning, private +#[non_exhaustive] +enum NonExhaustivePrivate { Foo, Bar, Baz, diff --git a/tests/ui/exhaustive_enums.stderr b/tests/ui/exhaustive_enums.stderr index ee5a1836267..280c40b00aa 100644 --- a/tests/ui/exhaustive_enums.stderr +++ b/tests/ui/exhaustive_enums.stderr @@ -1,7 +1,7 @@ error: enums should not be exhaustive --> $DIR/exhaustive_enums.rs:10:1 | -LL | / enum Exhaustive { +LL | / pub enum Exhaustive { LL | | Foo, LL | | Bar, LL | | Baz, @@ -17,7 +17,7 @@ LL | #![deny(clippy::exhaustive_enums)] help: try adding #[non_exhaustive] | LL | #[non_exhaustive] -LL | enum Exhaustive { +LL | pub enum Exhaustive { LL | Foo, LL | Bar, LL | Baz, From 09d4d49299c6614a0ae956980e709a234d21a9ef Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 13:36:18 -0800 Subject: [PATCH 4/9] ExhaustiveEnums -> ExhaustiveItems --- .../src/{exhaustive_enums.rs => exhaustive_items.rs} | 4 ++-- clippy_lints/src/lib.rs | 8 ++++---- .../ui/{exhaustive_enums.fixed => exhaustive_items.fixed} | 0 tests/ui/{exhaustive_enums.rs => exhaustive_items.rs} | 0 .../{exhaustive_enums.stderr => exhaustive_items.stderr} | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) rename clippy_lints/src/{exhaustive_enums.rs => exhaustive_items.rs} (95%) rename tests/ui/{exhaustive_enums.fixed => exhaustive_items.fixed} (100%) rename tests/ui/{exhaustive_enums.rs => exhaustive_items.rs} (100%) rename tests/ui/{exhaustive_enums.stderr => exhaustive_items.stderr} (87%) diff --git a/clippy_lints/src/exhaustive_enums.rs b/clippy_lints/src/exhaustive_items.rs similarity index 95% rename from clippy_lints/src/exhaustive_enums.rs rename to clippy_lints/src/exhaustive_items.rs index 2e1c0728d2c..0fa6c4b589f 100644 --- a/clippy_lints/src/exhaustive_enums.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -34,9 +34,9 @@ "default lint description" } -declare_lint_pass!(ExhaustiveEnums => [EXHAUSTIVE_ENUMS]); +declare_lint_pass!(ExhaustiveItems => [EXHAUSTIVE_ENUMS]); -impl LateLintPass<'_> for ExhaustiveEnums { +impl LateLintPass<'_> for ExhaustiveItems { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if_chain! { if let ItemKind::Enum(..) = item.kind; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index becd9f333fd..a5deebf7d5f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -200,7 +200,7 @@ macro_rules! declare_clippy_lint { mod eta_reduction; mod eval_order_dependence; mod excessive_bools; -mod exhaustive_enums; +mod exhaustive_items; mod exit; mod explicit_write; mod fallible_impl_from; @@ -612,7 +612,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &eval_order_dependence::EVAL_ORDER_DEPENDENCE, &excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS, &excessive_bools::STRUCT_EXCESSIVE_BOOLS, - &exhaustive_enums::EXHAUSTIVE_ENUMS, + &exhaustive_items::EXHAUSTIVE_ENUMS, &exit::EXIT, &explicit_write::EXPLICIT_WRITE, &fallible_impl_from::FALLIBLE_IMPL_FROM, @@ -1098,7 +1098,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box eval_order_dependence::EvalOrderDependence); store.register_late_pass(|| box missing_doc::MissingDoc::new()); store.register_late_pass(|| box missing_inline::MissingInline); - store.register_late_pass(move || box exhaustive_enums::ExhaustiveEnums); + store.register_late_pass(move || box exhaustive_items::ExhaustiveItems); store.register_late_pass(|| box if_let_some_result::OkIfLet); store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl); store.register_late_pass(|| box unused_io_amount::UnusedIoAmount); @@ -1249,7 +1249,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&create_dir::CREATE_DIR), LintId::of(&dbg_macro::DBG_MACRO), LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), - LintId::of(&exhaustive_enums::EXHAUSTIVE_ENUMS), + LintId::of(&exhaustive_items::EXHAUSTIVE_ENUMS), LintId::of(&exit::EXIT), LintId::of(&float_literal::LOSSY_FLOAT_LITERAL), LintId::of(&implicit_return::IMPLICIT_RETURN), diff --git a/tests/ui/exhaustive_enums.fixed b/tests/ui/exhaustive_items.fixed similarity index 100% rename from tests/ui/exhaustive_enums.fixed rename to tests/ui/exhaustive_items.fixed diff --git a/tests/ui/exhaustive_enums.rs b/tests/ui/exhaustive_items.rs similarity index 100% rename from tests/ui/exhaustive_enums.rs rename to tests/ui/exhaustive_items.rs diff --git a/tests/ui/exhaustive_enums.stderr b/tests/ui/exhaustive_items.stderr similarity index 87% rename from tests/ui/exhaustive_enums.stderr rename to tests/ui/exhaustive_items.stderr index 280c40b00aa..d00d950efc7 100644 --- a/tests/ui/exhaustive_enums.stderr +++ b/tests/ui/exhaustive_items.stderr @@ -1,5 +1,5 @@ error: enums should not be exhaustive - --> $DIR/exhaustive_enums.rs:10:1 + --> $DIR/exhaustive_items.rs:10:1 | LL | / pub enum Exhaustive { LL | | Foo, @@ -10,7 +10,7 @@ LL | | } | |_^ | note: the lint level is defined here - --> $DIR/exhaustive_enums.rs:3:9 + --> $DIR/exhaustive_items.rs:3:9 | LL | #![deny(clippy::exhaustive_enums)] | ^^^^^^^^^^^^^^^^^^^^^^^^ From 8cb7e85006853e99e6ba2bf378c801fb53eee8fa Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 13:41:57 -0800 Subject: [PATCH 5/9] Add exhaustive_structs lint --- CHANGELOG.md | 1 + clippy_lints/src/exhaustive_items.rs | 54 ++++++++++++++--- clippy_lints/src/lib.rs | 2 + tests/ui/exhaustive_items.fixed | 86 +++++++++++++++++++--------- tests/ui/exhaustive_items.rs | 85 ++++++++++++++++++--------- tests/ui/exhaustive_items.stderr | 52 +++++++++++------ 6 files changed, 199 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cf2125ea2f..ccc7eb7e881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1939,6 +1939,7 @@ Released 2018-09-13 [`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence [`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision [`exhaustive_enums`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums +[`exhaustive_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_structs [`exit`]: https://rust-lang.github.io/rust-clippy/master/index.html#exit [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call [`expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_used diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index 0fa6c4b589f..d604ad0004f 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -1,7 +1,7 @@ use crate::utils::{snippet_opt, span_lint_and_help, span_lint_and_sugg}; use if_chain::if_chain; -use rustc_hir::{Item, ItemKind}; use rustc_errors::Applicability; +use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -10,7 +10,8 @@ /// **What it does:** Warns on any exported `enum`s that are not tagged `#[non_exhaustive]` /// /// **Why is this bad?** Exhaustive enums are typically fine, but a project which does - /// not wish to make a stability commitment around enums may wish to disable them by default. + /// not wish to make a stability commitment around exported enums may wish to + /// disable them by default. /// /// **Known problems:** None. /// @@ -28,25 +29,62 @@ /// enum Foo { /// Bar, /// Baz - /// } /// ``` + /// } + /// ``` pub EXHAUSTIVE_ENUMS, restriction, - "default lint description" + "detects exported enums that have not been marked #[non_exhaustive]" } -declare_lint_pass!(ExhaustiveItems => [EXHAUSTIVE_ENUMS]); +declare_clippy_lint! { + /// **What it does:** Warns on any exported `structs`s that are not tagged `#[non_exhaustive]` + /// + /// **Why is this bad?** Exhaustive structs are typically fine, but a project which does + /// not wish to make a stability commitment around exported structs may wish to + /// disable them by default. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// struct Foo { + /// bar: u8, + /// baz: String, + /// } + /// ``` + /// Use instead: + /// ```rust + /// #[non_exhaustive] + /// struct Foo { + /// bar: u8, + /// baz: String, + /// } + /// ``` + pub EXHAUSTIVE_STRUCTS, + restriction, + "detects exported structs that have not been marked #[non_exhaustive]" +} + +declare_lint_pass!(ExhaustiveItems => [EXHAUSTIVE_ENUMS, EXHAUSTIVE_STRUCTS]); impl LateLintPass<'_> for ExhaustiveItems { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if_chain! { - if let ItemKind::Enum(..) = item.kind; + if let ItemKind::Enum(..) | ItemKind::Struct(..) = item.kind; if cx.access_levels.is_exported(item.hir_id); if !item.attrs.iter().any(|a| a.has_name(sym::non_exhaustive)); then { + let lint = if let ItemKind::Enum(..) = item.kind { + EXHAUSTIVE_ENUMS + } else { + EXHAUSTIVE_STRUCTS + }; + if let Some(snippet) = snippet_opt(cx, item.span) { span_lint_and_sugg( cx, - EXHAUSTIVE_ENUMS, + lint, item.span, "enums should not be exhaustive", "try adding #[non_exhaustive]", @@ -56,7 +94,7 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { } else { span_lint_and_help( cx, - EXHAUSTIVE_ENUMS, + lint, item.span, "enums should not be exhaustive", None, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a5deebf7d5f..91c74026f64 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -613,6 +613,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS, &excessive_bools::STRUCT_EXCESSIVE_BOOLS, &exhaustive_items::EXHAUSTIVE_ENUMS, + &exhaustive_items::EXHAUSTIVE_STRUCTS, &exit::EXIT, &explicit_write::EXPLICIT_WRITE, &fallible_impl_from::FALLIBLE_IMPL_FROM, @@ -1250,6 +1251,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&dbg_macro::DBG_MACRO), LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), LintId::of(&exhaustive_items::EXHAUSTIVE_ENUMS), + LintId::of(&exhaustive_items::EXHAUSTIVE_STRUCTS), LintId::of(&exit::EXIT), LintId::of(&float_literal::LOSSY_FLOAT_LITERAL), LintId::of(&implicit_return::IMPLICIT_RETURN), diff --git a/tests/ui/exhaustive_items.fixed b/tests/ui/exhaustive_items.fixed index 71c4a251e3b..bc146c6afc5 100644 --- a/tests/ui/exhaustive_items.fixed +++ b/tests/ui/exhaustive_items.fixed @@ -1,42 +1,72 @@ // run-rustfix -#![deny(clippy::exhaustive_enums)] +#![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] #![allow(unused)] fn main() { // nop } -#[non_exhaustive] +pub mod enums { + #[non_exhaustive] pub enum Exhaustive { - Foo, - Bar, - Baz, - Quux(String), + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, already non_exhaustive + #[non_exhaustive] + pub enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, private + enum ExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, private + #[non_exhaustive] + enum NonExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), + } } -// no warning, already non_exhaustive -#[non_exhaustive] -pub enum NonExhaustive { - Foo, - Bar, - Baz, - Quux(String), -} +pub mod structs { + #[non_exhaustive] +pub struct Exhaustive { + foo: u8, + bar: String, + } -// no warning, private -enum ExhaustivePrivate { - Foo, - Bar, - Baz, - Quux(String), -} + // no warning, already non_exhaustive + #[non_exhaustive] + pub struct NonExhaustive { + foo: u8, + bar: String, + } -// no warning, private -#[non_exhaustive] -enum NonExhaustivePrivate { - Foo, - Bar, - Baz, - Quux(String), + // no warning, private + struct ExhaustivePrivate { + foo: u8, + bar: String, + } + + // no warning, private + #[non_exhaustive] + struct NonExhaustivePrivate { + foo: u8, + bar: String, + } } diff --git a/tests/ui/exhaustive_items.rs b/tests/ui/exhaustive_items.rs index 45af6851dd1..ed86b50be30 100644 --- a/tests/ui/exhaustive_items.rs +++ b/tests/ui/exhaustive_items.rs @@ -1,41 +1,70 @@ // run-rustfix -#![deny(clippy::exhaustive_enums)] +#![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] #![allow(unused)] fn main() { // nop } -pub enum Exhaustive { - Foo, - Bar, - Baz, - Quux(String), +pub mod enums { + pub enum Exhaustive { + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, already non_exhaustive + #[non_exhaustive] + pub enum NonExhaustive { + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, private + enum ExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), + } + + // no warning, private + #[non_exhaustive] + enum NonExhaustivePrivate { + Foo, + Bar, + Baz, + Quux(String), + } } -// no warning, already non_exhaustive -#[non_exhaustive] -pub enum NonExhaustive { - Foo, - Bar, - Baz, - Quux(String), -} +pub mod structs { + pub struct Exhaustive { + foo: u8, + bar: String, + } -// no warning, private -enum ExhaustivePrivate { - Foo, - Bar, - Baz, - Quux(String), -} + // no warning, already non_exhaustive + #[non_exhaustive] + pub struct NonExhaustive { + foo: u8, + bar: String, + } -// no warning, private -#[non_exhaustive] -enum NonExhaustivePrivate { - Foo, - Bar, - Baz, - Quux(String), + // no warning, private + struct ExhaustivePrivate { + foo: u8, + bar: String, + } + + // no warning, private + #[non_exhaustive] + struct NonExhaustivePrivate { + foo: u8, + bar: String, + } } diff --git a/tests/ui/exhaustive_items.stderr b/tests/ui/exhaustive_items.stderr index d00d950efc7..7e286e65949 100644 --- a/tests/ui/exhaustive_items.stderr +++ b/tests/ui/exhaustive_items.stderr @@ -1,28 +1,46 @@ error: enums should not be exhaustive - --> $DIR/exhaustive_items.rs:10:1 + --> $DIR/exhaustive_items.rs:11:5 | -LL | / pub enum Exhaustive { -LL | | Foo, -LL | | Bar, -LL | | Baz, -LL | | Quux(String), -LL | | } - | |_^ +LL | / pub enum Exhaustive { +LL | | Foo, +LL | | Bar, +LL | | Baz, +LL | | Quux(String), +LL | | } + | |_____^ | note: the lint level is defined here - --> $DIR/exhaustive_items.rs:3:9 + --> $DIR/exhaustive_items.rs:3:35 | -LL | #![deny(clippy::exhaustive_enums)] - | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try adding #[non_exhaustive] | -LL | #[non_exhaustive] +LL | #[non_exhaustive] LL | pub enum Exhaustive { -LL | Foo, -LL | Bar, -LL | Baz, -LL | Quux(String), +LL | Foo, +LL | Bar, +LL | Baz, +LL | Quux(String), ... -error: aborting due to previous error +error: enums should not be exhaustive + --> $DIR/exhaustive_items.rs:46:5 + | +LL | / pub struct Exhaustive { +LL | | foo: u8, +LL | | bar: String, +LL | | } + | |_____^ + | +help: try adding #[non_exhaustive] + | +LL | #[non_exhaustive] +LL | pub struct Exhaustive { +LL | foo: u8, +LL | bar: String, +LL | } + | + +error: aborting due to 2 previous errors From 752274eabdd9be991c9a4e11850890301b697032 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 21 Jan 2021 14:00:25 -0800 Subject: [PATCH 6/9] Fix indentation of suggestion --- clippy_lints/src/exhaustive_items.rs | 5 +++-- tests/ui/exhaustive_items.fixed | 4 ++-- tests/ui/exhaustive_items.stderr | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index d604ad0004f..bbcf9bfea27 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -1,4 +1,4 @@ -use crate::utils::{snippet_opt, span_lint_and_help, span_lint_and_sugg}; +use crate::utils::{indent_of, snippet_opt, span_lint_and_help, span_lint_and_sugg}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; @@ -82,13 +82,14 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { }; if let Some(snippet) = snippet_opt(cx, item.span) { + let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); span_lint_and_sugg( cx, lint, item.span, "enums should not be exhaustive", "try adding #[non_exhaustive]", - format!("#[non_exhaustive]\n{}", snippet), + format!("#[non_exhaustive]\n{}{}", indent, snippet), Applicability::MaybeIncorrect, ); } else { diff --git a/tests/ui/exhaustive_items.fixed b/tests/ui/exhaustive_items.fixed index bc146c6afc5..7e355d2a58b 100644 --- a/tests/ui/exhaustive_items.fixed +++ b/tests/ui/exhaustive_items.fixed @@ -9,7 +9,7 @@ fn main() { pub mod enums { #[non_exhaustive] -pub enum Exhaustive { + pub enum Exhaustive { Foo, Bar, Baz, @@ -45,7 +45,7 @@ pub enum Exhaustive { pub mod structs { #[non_exhaustive] -pub struct Exhaustive { + pub struct Exhaustive { foo: u8, bar: String, } diff --git a/tests/ui/exhaustive_items.stderr b/tests/ui/exhaustive_items.stderr index 7e286e65949..1716c8e2cb5 100644 --- a/tests/ui/exhaustive_items.stderr +++ b/tests/ui/exhaustive_items.stderr @@ -17,7 +17,7 @@ LL | #![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] help: try adding #[non_exhaustive] | LL | #[non_exhaustive] -LL | pub enum Exhaustive { +LL | pub enum Exhaustive { LL | Foo, LL | Bar, LL | Baz, @@ -36,7 +36,7 @@ LL | | } help: try adding #[non_exhaustive] | LL | #[non_exhaustive] -LL | pub struct Exhaustive { +LL | pub struct Exhaustive { LL | foo: u8, LL | bar: String, LL | } From 65d003a1128e9234730a66c79065afdabb31afa0 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 22 Jan 2021 12:08:46 -0800 Subject: [PATCH 7/9] Clean up suggestion span; clarify help message --- clippy_lints/src/exhaustive_items.rs | 27 ++++++++++++++++----------- tests/ui/exhaustive_items.stderr | 22 ++++++++++------------ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index bbcf9bfea27..4749e36238c 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -1,4 +1,4 @@ -use crate::utils::{indent_of, snippet_opt, span_lint_and_help, span_lint_and_sugg}; +use crate::utils::{indent_of, snippet_opt, span_lint_and_help, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; @@ -75,29 +75,34 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { if cx.access_levels.is_exported(item.hir_id); if !item.attrs.iter().any(|a| a.has_name(sym::non_exhaustive)); then { - let lint = if let ItemKind::Enum(..) = item.kind { - EXHAUSTIVE_ENUMS + let (lint, msg) = if let ItemKind::Enum(..) = item.kind { + (EXHAUSTIVE_ENUMS, "exported enums should not be exhaustive") } else { - EXHAUSTIVE_STRUCTS + (EXHAUSTIVE_STRUCTS, "exported structs should not be exhaustive") }; + let suggestion_span = item.span.until(item.ident.span); - if let Some(snippet) = snippet_opt(cx, item.span) { + if let Some(snippet) = snippet_opt(cx, suggestion_span) { let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); - span_lint_and_sugg( + span_lint_and_then( cx, lint, item.span, - "enums should not be exhaustive", - "try adding #[non_exhaustive]", - format!("#[non_exhaustive]\n{}{}", indent, snippet), - Applicability::MaybeIncorrect, + msg, + |diag| { + let sugg = format!("#[non_exhaustive]\n{}{}", indent, snippet); + diag.span_suggestion(suggestion_span, + "try adding #[non_exhaustive]", + sugg, + Applicability::MaybeIncorrect); + } ); } else { span_lint_and_help( cx, lint, item.span, - "enums should not be exhaustive", + msg, None, "try adding #[non_exhaustive]", ); diff --git a/tests/ui/exhaustive_items.stderr b/tests/ui/exhaustive_items.stderr index 1716c8e2cb5..a24e64b6705 100644 --- a/tests/ui/exhaustive_items.stderr +++ b/tests/ui/exhaustive_items.stderr @@ -1,4 +1,4 @@ -error: enums should not be exhaustive +error: exported enums should not be exhaustive --> $DIR/exhaustive_items.rs:11:5 | LL | / pub enum Exhaustive { @@ -10,21 +10,17 @@ LL | | } | |_____^ | note: the lint level is defined here - --> $DIR/exhaustive_items.rs:3:35 + --> $DIR/exhaustive_items.rs:3:9 | LL | #![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try adding #[non_exhaustive] | LL | #[non_exhaustive] LL | pub enum Exhaustive { -LL | Foo, -LL | Bar, -LL | Baz, -LL | Quux(String), - ... + | -error: enums should not be exhaustive +error: exported structs should not be exhaustive --> $DIR/exhaustive_items.rs:46:5 | LL | / pub struct Exhaustive { @@ -33,13 +29,15 @@ LL | | bar: String, LL | | } | |_____^ | +note: the lint level is defined here + --> $DIR/exhaustive_items.rs:3:35 + | +LL | #![deny(clippy::exhaustive_enums, clippy::exhaustive_structs)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try adding #[non_exhaustive] | LL | #[non_exhaustive] LL | pub struct Exhaustive { -LL | foo: u8, -LL | bar: String, -LL | } | error: aborting due to 2 previous errors From e0ae980fab38e61bf39380e69bbf0000e681a9f8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Jan 2021 14:35:57 -0800 Subject: [PATCH 8/9] Better suggestion span --- clippy_lints/src/exhaustive_items.rs | 43 +++++++++++----------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index 4749e36238c..32b1299efce 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -1,4 +1,4 @@ -use crate::utils::{indent_of, snippet_opt, span_lint_and_help, span_lint_and_then}; +use crate::utils::{indent_of, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; @@ -80,33 +80,22 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { } else { (EXHAUSTIVE_STRUCTS, "exported structs should not be exhaustive") }; - let suggestion_span = item.span.until(item.ident.span); + let suggestion_span = item.span.shrink_to_lo(); + let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); + span_lint_and_then( + cx, + lint, + item.span, + msg, + |diag| { + let sugg = format!("#[non_exhaustive]\n{}", indent); + diag.span_suggestion(suggestion_span, + "try adding #[non_exhaustive]", + sugg, + Applicability::MaybeIncorrect); + } + ); - if let Some(snippet) = snippet_opt(cx, suggestion_span) { - let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); - span_lint_and_then( - cx, - lint, - item.span, - msg, - |diag| { - let sugg = format!("#[non_exhaustive]\n{}{}", indent, snippet); - diag.span_suggestion(suggestion_span, - "try adding #[non_exhaustive]", - sugg, - Applicability::MaybeIncorrect); - } - ); - } else { - span_lint_and_help( - cx, - lint, - item.span, - msg, - None, - "try adding #[non_exhaustive]", - ); - } } } } From 3e3dff71357005556aba8d9a7893829f7510b079 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Jan 2021 14:39:03 -0800 Subject: [PATCH 9/9] Add test with attrs --- tests/ui/exhaustive_items.fixed | 10 ++++++++++ tests/ui/exhaustive_items.rs | 9 +++++++++ tests/ui/exhaustive_items.stderr | 21 +++++++++++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/ui/exhaustive_items.fixed b/tests/ui/exhaustive_items.fixed index 7e355d2a58b..8174a0175ab 100644 --- a/tests/ui/exhaustive_items.fixed +++ b/tests/ui/exhaustive_items.fixed @@ -16,6 +16,16 @@ pub mod enums { Quux(String), } + /// Some docs + #[repr(C)] + #[non_exhaustive] + pub enum ExhaustiveWithAttrs { + Foo, + Bar, + Baz, + Quux(String), + } + // no warning, already non_exhaustive #[non_exhaustive] pub enum NonExhaustive { diff --git a/tests/ui/exhaustive_items.rs b/tests/ui/exhaustive_items.rs index ed86b50be30..b476f09f8a0 100644 --- a/tests/ui/exhaustive_items.rs +++ b/tests/ui/exhaustive_items.rs @@ -15,6 +15,15 @@ pub enum Exhaustive { Quux(String), } + /// Some docs + #[repr(C)] + pub enum ExhaustiveWithAttrs { + Foo, + Bar, + Baz, + Quux(String), + } + // no warning, already non_exhaustive #[non_exhaustive] pub enum NonExhaustive { diff --git a/tests/ui/exhaustive_items.stderr b/tests/ui/exhaustive_items.stderr index a24e64b6705..7369fe75a4f 100644 --- a/tests/ui/exhaustive_items.stderr +++ b/tests/ui/exhaustive_items.stderr @@ -20,8 +20,25 @@ LL | #[non_exhaustive] LL | pub enum Exhaustive { | +error: exported enums should not be exhaustive + --> $DIR/exhaustive_items.rs:20:5 + | +LL | / pub enum ExhaustiveWithAttrs { +LL | | Foo, +LL | | Bar, +LL | | Baz, +LL | | Quux(String), +LL | | } + | |_____^ + | +help: try adding #[non_exhaustive] + | +LL | #[non_exhaustive] +LL | pub enum ExhaustiveWithAttrs { + | + error: exported structs should not be exhaustive - --> $DIR/exhaustive_items.rs:46:5 + --> $DIR/exhaustive_items.rs:55:5 | LL | / pub struct Exhaustive { LL | | foo: u8, @@ -40,5 +57,5 @@ LL | #[non_exhaustive] LL | pub struct Exhaustive { | -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors