Auto merge of rust-lang#117094 - Nadrieril:warn-lint-on-arm, r=cjgillot

Warn users who set `non_exhaustive_omitted_patterns` lint level on a match arm

Before rust-lang#116734, the recommended usage of the [`non_exhaustive_omitted_patterns` lint](rust-lang#89554) was:
```rust
match Bar::A {
    Bar::A => {},
    #[warn(non_exhaustive_omitted_patterns)]
    _ => {},
}
```
After rust-lang#116734 this no longer makes sense, and recommended usage is now:
```rust
#[warn(non_exhaustive_omitted_patterns)]
match Bar::A {
    Bar::A => {},
    _ => {},
}
```

As you can guess, this silently breaks all uses of the lint that used the previous form. This is a problem in particular because `syn` recommends usage of this lint to its users in the old way. This PR emits a warning when the previous form is used so users can update.

r? `@cjgillot`

Author: bors

compiler/rustc_mir_build/messages.ftl

@@ -221,6 +221,11 @@ 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 lint level must be set on the whole match
+    .help = it no longer has any effect to set the lint level on an individual match arm
+    .label = remove this attribute
+    .suggestion = set the lint level on the whole match
+
 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}`

compiler/rustc_mir_build/src/errors.rs

@@ -789,6 +789,18 @@ 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 {
+    #[label]
+    pub lint_span: Span,
+    #[suggestion(code = "#[{lint_level}({lint_name})]\n", applicability = "maybe-incorrect")]
+    pub suggest_lint_on_match: Option<Span>,
+    pub lint_level: &'static str,
+    pub lint_name: &'static str,
+}
+
 #[derive(Subdiagnostic)]
 #[label(mir_build_uncovered)]
 pub(crate) struct Uncovered<'tcx> {

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -285,7 +285,11 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
         }
     }
 
-    fn new_cx(&self, refutability: RefutableFlag) -> MatchCheckCtxt<'p, 'tcx> {
+    fn new_cx(
+        &self,
+        refutability: RefutableFlag,
+        match_span: Option<Span>,
+    ) -> MatchCheckCtxt<'p, 'tcx> {
         let refutable = match refutability {
             Irrefutable => false,
             Refutable => true,
@@ -295,6 +299,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
             param_env: self.param_env,
             module: self.tcx.parent_module(self.lint_level).to_def_id(),
             pattern_arena: &self.pattern_arena,
+            match_span,
             refutable,
         }
     }
@@ -325,7 +330,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
         source: hir::MatchSource,
         expr_span: Span,
     ) {
-        let cx = self.new_cx(Refutable);
+        let cx = self.new_cx(Refutable, Some(expr_span));
 
         let mut tarms = Vec::with_capacity(arms.len());
         for &arm in arms {
@@ -448,7 +453,7 @@ impl<'thir, 'p, 'tcx> MatchVisitor<'thir, 'p, 'tcx> {
         pat: &Pat<'tcx>,
         refutability: RefutableFlag,
     ) -> Result<(MatchCheckCtxt<'p, 'tcx>, UsefulnessReport<'p, 'tcx>), ErrorGuaranteed> {
-        let cx = self.new_cx(refutability);
+        let cx = self.new_cx(refutability, None);
         let pat = self.lower_pattern(&cx, pat)?;
         let arms = [MatchArm { pat, hir_id: self.lint_level, has_guard: false }];
         let report = compute_match_usefulness(&cx, &arms, self.lint_level, pat.ty(), pat.span());

compiler/rustc_mir_build/src/thir/pattern/usefulness.rs

@@ -311,7 +311,10 @@ use super::deconstruct_pat::{
     Constructor, ConstructorSet, DeconstructedPat, IntRange, MaybeInfiniteInt, SplitConstructorSet,
     WitnessPat,
 };
-use crate::errors::{NonExhaustiveOmittedPattern, Overlap, OverlappingRangeEndpoints, Uncovered};
+use crate::errors::{
+    NonExhaustiveOmittedPattern, NonExhaustiveOmittedPatternLintOnArm, Overlap,
+    OverlappingRangeEndpoints, Uncovered,
+};
 
 use rustc_data_structures::captures::Captures;
 
@@ -337,6 +340,8 @@ pub(crate) struct MatchCheckCtxt<'p, 'tcx> {
     pub(crate) module: DefId,
     pub(crate) param_env: ty::ParamEnv<'tcx>,
     pub(crate) pattern_arena: &'p TypedArena<DeconstructedPat<'p, 'tcx>>,
+    /// The span of the whole match, if applicable.
+    pub(crate) match_span: Option<Span>,
     /// Only produce `NON_EXHAUSTIVE_OMITTED_PATTERNS` lint on refutable patterns.
     pub(crate) refutable: bool,
 }
@@ -1149,28 +1154,50 @@ 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 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 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 {
+                let (lint_level, lint_level_source) =
+                    cx.tcx.lint_level_at_node(NON_EXHAUSTIVE_OMITTED_PATTERNS, arm.hir_id);
+                if !matches!(lint_level, rustc_session::lint::Level::Allow) {
+                    let decorator = NonExhaustiveOmittedPatternLintOnArm {
+                        lint_span: lint_level_source.span(),
+                        suggest_lint_on_match: cx.match_span.map(|span| span.shrink_to_lo()),
+                        lint_level: lint_level.as_str(),
+                        lint_name: "non_exhaustive_omitted_patterns",
+                    };
+
+                    use rustc_errors::DecorateLint;
+                    let mut err = cx.tcx.sess.struct_span_warn(arm.pat.span(), "");
+                    err.set_primary_message(decorator.msg());
+                    decorator.decorate_lint(&mut err);
+                    err.emit();
+                }
+            }
         }
     }
 

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(),
     }
 }

tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.lint.stderr

@@ -0,0 +1,75 @@
+error: some variants are not matched explicitly
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:15:11
+   |
+LL |     match val {
+   |           ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
+   |
+   = help: ensure that all variants are matched explicitly by adding the suggested match arms
+   = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:14:12
+   |
+LL |     #[deny(non_exhaustive_omitted_patterns)]
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: some variants are not matched explicitly
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:23:11
+   |
+LL |     match val {
+   |           ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
+   |
+   = help: ensure that all variants are matched explicitly by adding the suggested match arms
+   = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:22:27
+   |
+LL |     #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: the lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
+   |
+LL |         #[deny(non_exhaustive_omitted_patterns)]
+   |                ------------------------------- remove this attribute
+LL |         _ => {}
+   |         ^
+   |
+   = help: it no longer has any effect to set the lint level on an individual match arm
+help: set the lint level on the whole match
+   |
+LL +     #[deny(non_exhaustive_omitted_patterns)]
+LL |     match val {
+   |
+
+warning: the lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:42:9
+   |
+LL |         #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
+   |                               ------------------------------- remove this attribute
+LL |         _ => {}
+   |         ^
+   |
+   = help: it no longer has any effect to set the lint level on an individual match arm
+help: set the lint level on the whole match
+   |
+LL +     #[deny(non_exhaustive_omitted_patterns)]
+LL |     match val {
+   |
+
+warning: the lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:50:9
+   |
+LL |         #[cfg_attr(lint, warn(non_exhaustive_omitted_patterns))]
+   |                               ------------------------------- remove this attribute
+LL |         _ => {}
+   |         ^
+   |
+   = help: it no longer has any effect to set the lint level on an individual match arm
+help: set the lint level on the whole match
+   |
+LL +     #[warn(non_exhaustive_omitted_patterns)]
+LL |     match val {
+   |
+
+error: aborting due to 2 previous errors; 3 warnings emitted
+

tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.normal.stderr

@@ -0,0 +1,31 @@
+error: some variants are not matched explicitly
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:15:11
+   |
+LL |     match val {
+   |           ^^^ pattern `NonExhaustiveEnum::Struct { .. }` not covered
+   |
+   = help: ensure that all variants are matched explicitly by adding the suggested match arms
+   = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found
+note: the lint level is defined here
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:14:12
+   |
+LL |     #[deny(non_exhaustive_omitted_patterns)]
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: the lint level must be set on the whole match
+  --> $DIR/omitted-patterns-dont-lint-on-arm.rs:34:9
+   |
+LL |         #[deny(non_exhaustive_omitted_patterns)]
+   |                ------------------------------- remove this attribute
+LL |         _ => {}
+   |         ^
+   |
+   = help: it no longer has any effect to set the lint level on an individual match arm
+help: set the lint level on the whole match
+   |
+LL +     #[deny(non_exhaustive_omitted_patterns)]
+LL |     match val {
+   |
+
+error: aborting due to previous error; 1 warning emitted
+

tests/ui/rfcs/rfc-2008-non-exhaustive/omitted-patterns-dont-lint-on-arm.rs

@@ -0,0 +1,53 @@
+// revisions: normal lint
+// Test that putting the lint level on a match arm emits a warning, as this was previously
+// meaningful and is no longer.
+#![feature(non_exhaustive_omitted_patterns_lint)]
+
+// aux-build:enums.rs
+extern crate enums;
+
+use enums::NonExhaustiveEnum;
+
+fn main() {
+    let val = NonExhaustiveEnum::Unit;
+
+    #[deny(non_exhaustive_omitted_patterns)]
+    match val {
+        //~^ ERROR some variants are not matched explicitly
+        NonExhaustiveEnum::Unit => {}
+        NonExhaustiveEnum::Tuple(_) => {}
+        _ => {}
+    }
+
+    #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
+    match val {
+        //[lint]~^ ERROR some variants are not matched explicitly
+        NonExhaustiveEnum::Unit => {}
+        NonExhaustiveEnum::Tuple(_) => {}
+        _ => {}
+    }
+
+    match val {
+        NonExhaustiveEnum::Unit => {}
+        NonExhaustiveEnum::Tuple(_) => {}
+        #[deny(non_exhaustive_omitted_patterns)]
+        _ => {}
+    }
+    //~^^ WARN lint level must be set on the whole match
+
+    match val {
+        NonExhaustiveEnum::Unit => {}
+        NonExhaustiveEnum::Tuple(_) => {}
+        #[cfg_attr(lint, deny(non_exhaustive_omitted_patterns))]
+        _ => {}
+    }
+    //[lint]~^^ WARN 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
+}