about summary refs log tree commit diff
path: root/compiler/rustc_lint/src/non_fmt_panic.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
committerbors <bors@rust-lang.org>2022-10-01 10:44:25 +0000
commit744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch)
tree1721987352b5f0a8548fc46984821d974b661934 /compiler/rustc_lint/src/non_fmt_panic.rs
parent277bb6653b55475b5fbce6309e9852fa2100dabe (diff)
parentb5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff)
downloadrust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.tar.gz
rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.zip
Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=estebank
Move lint level source explanation to the bottom

So, uhhhhh

r? `@estebank`

## User-facing change

"note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic:
```diff
-   = note: `#[warn(unsupported_calling_conventions)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #87678 <https://github.com/rust-lang/rust/issues/87678>
+   = note: `#[warn(unsupported_calling_conventions)]` on by default
```

Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO.

## Developer-facing change

`struct_span_lint` and similar methods have a different signature.

Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)`
After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>`

The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion.

Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere.

## Subtle problem

By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings...

I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate.

## P.S.

I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢
Diffstat (limited to 'compiler/rustc_lint/src/non_fmt_panic.rs')
-rw-r--r--compiler/rustc_lint/src/non_fmt_panic.rs104
1 files changed, 55 insertions, 49 deletions
diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
index cdad2d2e8f9..9d2a23f2b5f 100644
--- a/compiler/rustc_lint/src/non_fmt_panic.rs
+++ b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -119,21 +119,19 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
         arg_span = expn.call_site;
     }
 
-    cx.struct_span_lint(NON_FMT_PANICS, arg_span, |lint| {
-        let mut l = lint.build(fluent::lint::non_fmt_panic);
-        l.set_arg("name", symbol);
-        l.note(fluent::lint::note);
-        l.note(fluent::lint::more_info_note);
+    cx.struct_span_lint(NON_FMT_PANICS, arg_span, fluent::lint::non_fmt_panic, |lint| {
+        lint.set_arg("name", symbol);
+        lint.note(fluent::lint::note);
+        lint.note(fluent::lint::more_info_note);
         if !is_arg_inside_call(arg_span, span) {
             // No clue where this argument is coming from.
-            l.emit();
-            return;
+            return lint;
         }
         if arg_macro.map_or(false, |id| cx.tcx.is_diagnostic_item(sym::format_macro, id)) {
             // A case of `panic!(format!(..))`.
-            l.note(fluent::lint::supports_fmt_note);
+            lint.note(fluent::lint::supports_fmt_note);
             if let Some((open, close, _)) = find_delimiters(cx, arg_span) {
-                l.multipart_suggestion(
+                lint.multipart_suggestion(
                     fluent::lint::supports_fmt_suggestion,
                     vec![
                         (arg_span.until(open.shrink_to_hi()), "".into()),
@@ -180,15 +178,15 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
             };
 
             if suggest_display {
-                l.span_suggestion_verbose(
+                lint.span_suggestion_verbose(
                     arg_span.shrink_to_lo(),
                     fluent::lint::display_suggestion,
                     "\"{}\", ",
                     fmt_applicability,
                 );
             } else if suggest_debug {
-                l.set_arg("ty", ty);
-                l.span_suggestion_verbose(
+                lint.set_arg("ty", ty);
+                lint.span_suggestion_verbose(
                     arg_span.shrink_to_lo(),
                     fluent::lint::debug_suggestion,
                     "\"{:?}\", ",
@@ -198,8 +196,8 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
 
             if suggest_panic_any {
                 if let Some((open, close, del)) = find_delimiters(cx, span) {
-                    l.set_arg("already_suggested", suggest_display || suggest_debug);
-                    l.multipart_suggestion(
+                    lint.set_arg("already_suggested", suggest_display || suggest_debug);
+                    lint.multipart_suggestion(
                         fluent::lint::panic_suggestion,
                         if del == '(' {
                             vec![(span.until(open), "std::panic::panic_any".into())]
@@ -214,7 +212,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
                 }
             }
         }
-        l.emit();
+        lint
     });
 }
 
@@ -258,26 +256,30 @@ fn check_panic_str<'tcx>(
                 .map(|span| fmt_span.from_inner(InnerSpan::new(span.start, span.end)))
                 .collect(),
         };
-        cx.struct_span_lint(NON_FMT_PANICS, arg_spans, |lint| {
-            let mut l = lint.build(fluent::lint::non_fmt_panic_unused);
-            l.set_arg("count", n_arguments);
-            l.note(fluent::lint::note);
-            if is_arg_inside_call(arg.span, span) {
-                l.span_suggestion(
-                    arg.span.shrink_to_hi(),
-                    fluent::lint::add_args_suggestion,
-                    ", ...",
-                    Applicability::HasPlaceholders,
-                );
-                l.span_suggestion(
-                    arg.span.shrink_to_lo(),
-                    fluent::lint::add_fmt_suggestion,
-                    "\"{}\", ",
-                    Applicability::MachineApplicable,
-                );
-            }
-            l.emit();
-        });
+        cx.struct_span_lint(
+            NON_FMT_PANICS,
+            arg_spans,
+            fluent::lint::non_fmt_panic_unused,
+            |lint| {
+                lint.set_arg("count", n_arguments);
+                lint.note(fluent::lint::note);
+                if is_arg_inside_call(arg.span, span) {
+                    lint.span_suggestion(
+                        arg.span.shrink_to_hi(),
+                        fluent::lint::add_args_suggestion,
+                        ", ...",
+                        Applicability::HasPlaceholders,
+                    );
+                    lint.span_suggestion(
+                        arg.span.shrink_to_lo(),
+                        fluent::lint::add_fmt_suggestion,
+                        "\"{}\", ",
+                        Applicability::MachineApplicable,
+                    );
+                }
+                lint
+            },
+        );
     } else {
         let brace_spans: Option<Vec<_>> =
             snippet.filter(|s| s.starts_with('"') || s.starts_with("r#")).map(|s| {
@@ -287,20 +289,24 @@ fn check_panic_str<'tcx>(
                     .collect()
             });
         let count = brace_spans.as_ref().map(|v| v.len()).unwrap_or(/* any number >1 */ 2);
-        cx.struct_span_lint(NON_FMT_PANICS, brace_spans.unwrap_or_else(|| vec![span]), |lint| {
-            let mut l = lint.build(fluent::lint::non_fmt_panic_braces);
-            l.set_arg("count", count);
-            l.note(fluent::lint::note);
-            if is_arg_inside_call(arg.span, span) {
-                l.span_suggestion(
-                    arg.span.shrink_to_lo(),
-                    fluent::lint::suggestion,
-                    "\"{}\", ",
-                    Applicability::MachineApplicable,
-                );
-            }
-            l.emit();
-        });
+        cx.struct_span_lint(
+            NON_FMT_PANICS,
+            brace_spans.unwrap_or_else(|| vec![span]),
+            fluent::lint::non_fmt_panic_braces,
+            |lint| {
+                lint.set_arg("count", count);
+                lint.note(fluent::lint::note);
+                if is_arg_inside_call(arg.span, span) {
+                    lint.span_suggestion(
+                        arg.span.shrink_to_lo(),
+                        fluent::lint::suggestion,
+                        "\"{}\", ",
+                        Applicability::MachineApplicable,
+                    );
+                }
+                lint
+            },
+        );
     }
 }