Warn when lint level is set on a match arm

rust-lang · Oct 23, 2023 · d53f545 · d53f545
1 parent 5cf6929
commit d53f545
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 29 deletions.
diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl
@@ -221,6 +221,9 @@ mir_build_non_exhaustive_omitted_pattern = some variants are not matched explici
     .help = ensure that all variants are matched explicitly by adding the suggested match arms
     .note = the matched value is of type `{$scrut_ty}` and the `non_exhaustive_omitted_patterns` attribute was found
 
+mir_build_non_exhaustive_omitted_pattern_lint_on_arm = the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
+    .help = it used to make sense to set the lint level on an individual match arm, but that is no longer the case
+
 mir_build_non_exhaustive_patterns_type_not_empty = non-exhaustive patterns: type `{$ty}` is non-empty
     .def_note = `{$peeled_ty}` defined here
     .type_note = the matched value is of type `{$ty}`

diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs
@@ -794,6 +794,11 @@ pub(crate) struct NonExhaustiveOmittedPattern<'tcx> {
     pub uncovered: Uncovered<'tcx>,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(mir_build_non_exhaustive_omitted_pattern_lint_on_arm)]
+#[help]
+pub(crate) struct NonExhaustiveOmittedPatternLintOnArm;
+
 #[derive(Subdiagnostic)]
 #[label(mir_build_uncovered)]
 pub(crate) struct Uncovered<'tcx> {

diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
@@ -308,7 +308,7 @@
 use self::ArmType::*;
 use self::Usefulness::*;
 use super::deconstruct_pat::{Constructor, ConstructorSet, DeconstructedPat, WitnessPat};
-use crate::errors::{NonExhaustiveOmittedPattern, Uncovered};
+use crate::errors::{NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Uncovered};
 
 use rustc_data_structures::captures::Captures;
 
@@ -1021,30 +1021,47 @@ pub(crate) fn compute_match_usefulness<'p, 'tcx>(
 
     // Run the non_exhaustive_omitted_patterns lint. Only run on refutable patterns to avoid hitting
     // `if let`s. Only run if the match is exhaustive otherwise the error is redundant.
-    if cx.refutable
-        && non_exhaustiveness_witnesses.is_empty()
-        && !matches!(
+    if cx.refutable && non_exhaustiveness_witnesses.is_empty() {
+        if !matches!(
             cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, lint_root).0,
             rustc_session::lint::Level::Allow
-        )
-    {
-        let pat_column = arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
-        let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);
-
-        if !witnesses.is_empty() {
-            // Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
-            // is not exhaustive enough.
-            //
-            // NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
-            cx.tcx.emit_spanned_lint(
-                NON_EXHAUSTIVE_OMITTED_PATTERNS,
-                lint_root,
-                scrut_span,
-                NonExhaustiveOmittedPattern {
-                    scrut_ty,
-                    uncovered: Uncovered::new(scrut_span, cx, witnesses),
-                },
-            );
+        ) {
+            let pat_column =
+                arms.iter().flat_map(|arm| arm.pat.flatten_or_pat()).collect::<Vec<_>>();
+            let witnesses = collect_nonexhaustive_missing_variants(cx, &pat_column);
+
+            if !witnesses.is_empty() {
+                // Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns`
+                // is not exhaustive enough.
+                //
+                // NB: The partner lint for structs lives in `compiler/rustc_hir_analysis/src/check/pat.rs`.
+                cx.tcx.emit_spanned_lint(
+                    NON_EXHAUSTIVE_OMITTED_PATTERNS,
+                    lint_root,
+                    scrut_span,
+                    NonExhaustiveOmittedPattern {
+                        scrut_ty,
+                        uncovered: Uncovered::new(scrut_span, cx, witnesses),
+                    },
+                );
+            }
+        } else {
+            // We used to allow putting the `#[allow(non_exhaustive_omitted_patterns)]` on a match
+            // arm. This no longer makes sense so we warn users, to avoid silently breaking their
+            // usage of the lint.
+            for arm in arms {
+                if !matches!(
+                    cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, arm.hir_id).0,
+                    rustc_session::lint::Level::Allow
+                ) {
+                    cx.tcx.emit_spanned_lint(
+                        NON_EXHAUSTIVE_OMITTED_PATTERNS,
+                        arm.hir_id,
+                        arm.pat.span(),
+                        NonExhaustiveOmittedPatternLintOnArm,
+                    );
+                }
+            }
         }
     }
 

diff --git a/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs b/src/tools/clippy/tests/ui/match_same_arms_non_exhaustive.rs
@@ -9,12 +9,12 @@ fn repeat() -> ! {
 }
 
 pub fn f(x: Ordering) {
+    #[deny(non_exhaustive_omitted_patterns)]
     match x {
         Ordering::Relaxed => println!("relaxed"),
         Ordering::Release => println!("release"),
         Ordering::Acquire => println!("acquire"),
         Ordering::AcqRel | Ordering::SeqCst => repeat(),
-        #[deny(non_exhaustive_omitted_patterns)]
         _ => repeat(),
     }
 }

diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr
@@ -26,5 +26,44 @@ note: the lint level is defined here
 LL |     #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
+   |
+LL |         _ => {}
+   |         ^
+   |
+   = help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:33:16
+   |
+LL |         #[deny(non_exhaustive_omitted_patterns)]
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:41:9
+   |
+LL |         _ => {}
+   |         ^
+   |
+   = help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:40:31
+   |
+LL |         #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:48:9
+   |
+LL |         _ => {}
+   |         ^
+   |
+   = help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:47:31
+   |
+LL |         #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
+   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors; 1 warning emitted
 
diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr
@@ -12,5 +12,18 @@ note: the lint level is defined here
 LL |     #[deny(non_exhaustive_omitted_patterns)]
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: the `non_exhaustive_omitted_pattern` lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
+   |
+LL |         _ => {}
+   |         ^
+   |
+   = help: it used to make sense to set the lint level on an individual match arm, but that is no longer the case
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:33:16
+   |
+LL |         #[deny(non_exhaustive_omitted_patterns)]
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
 
diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs
@@ -31,20 +31,20 @@ fn main() {
         NonExhaustiveEnum::Unit => {}
         NonExhaustiveEnum::Tuple(_) => {}
         #[deny(non_exhaustive_omitted_patterns)]
-        _ => {}
+        _ => {} //~ ERROR lint level must be set on the whole match
     }
 
     match val {
         NonExhaustiveEnum::Unit => {}
         NonExhaustiveEnum::Tuple(_) => {}
         #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
-        _ => {}
+        _ => {} //[lint]~ ERROR lint level must be set on the whole match
     }
 
     match val {
         NonExhaustiveEnum::Unit => {}
         NonExhaustiveEnum::Tuple(_) => {}
         #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
-        _ => {}
+        _ => {} //[lint]~ WARN lint level must be set on the whole match
     }
 }