about summary refs log tree commit diff
path: root/src/librustdoc/passes/check_code_block_syntax.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 /src/librustdoc/passes/check_code_block_syntax.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 'src/librustdoc/passes/check_code_block_syntax.rs')
-rw-r--r--src/librustdoc/passes/check_code_block_syntax.rs85
1 files changed, 41 insertions, 44 deletions
diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs
index 381ac7a5dee..23f87838544 100644
--- a/src/librustdoc/passes/check_code_block_syntax.rs
+++ b/src/librustdoc/passes/check_code_block_syntax.rs
@@ -2,7 +2,7 @@
 use rustc_data_structures::sync::{Lock, Lrc};
 use rustc_errors::{
     emitter::Emitter, translation::Translate, Applicability, Diagnostic, Handler,
-    LazyFallbackBundle, LintDiagnosticBuilder,
+    LazyFallbackBundle,
 };
 use rustc_parse::parse_stream_from_source_str;
 use rustc_session::parse::ParseSess;
@@ -97,48 +97,10 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
             None => (item.attr_span(self.cx.tcx), false),
         };
 
-        // lambda that will use the lint to start a new diagnostic and add
-        // a suggestion to it when needed.
-        let diag_builder = |lint: LintDiagnosticBuilder<'_, ()>| {
-            let explanation = if is_ignore {
-                "`ignore` code blocks require valid Rust code for syntax highlighting; \
-                    mark blocks that do not contain Rust code as text"
-            } else {
-                "mark blocks that do not contain Rust code as text"
-            };
-            let msg = if buffer.has_errors {
-                "could not parse code block as Rust code"
-            } else {
-                "Rust code block is empty"
-            };
-            let mut diag = lint.build(msg);
-
-            if precise_span {
-                if is_ignore {
-                    // giving an accurate suggestion is hard because `ignore` might not have come first in the list.
-                    // just give a `help` instead.
-                    diag.span_help(
-                        sp.from_inner(InnerSpan::new(0, 3)),
-                        &format!("{}: ```text", explanation),
-                    );
-                } else if empty_block {
-                    diag.span_suggestion(
-                        sp.from_inner(InnerSpan::new(0, 3)).shrink_to_hi(),
-                        explanation,
-                        "text",
-                        Applicability::MachineApplicable,
-                    );
-                }
-            } else if empty_block || is_ignore {
-                diag.help(&format!("{}: ```text", explanation));
-            }
-
-            // FIXME(#67563): Provide more context for these errors by displaying the spans inline.
-            for message in buffer.messages.iter() {
-                diag.note(message);
-            }
-
-            diag.emit();
+        let msg = if buffer.has_errors {
+            "could not parse code block as Rust code"
+        } else {
+            "Rust code block is empty"
         };
 
         // Finally build and emit the completed diagnostic.
@@ -148,7 +110,42 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
             crate::lint::INVALID_RUST_CODEBLOCKS,
             hir_id,
             sp,
-            diag_builder,
+            msg,
+            |lint| {
+                let explanation = if is_ignore {
+                    "`ignore` code blocks require valid Rust code for syntax highlighting; \
+                    mark blocks that do not contain Rust code as text"
+                } else {
+                    "mark blocks that do not contain Rust code as text"
+                };
+
+                if precise_span {
+                    if is_ignore {
+                        // giving an accurate suggestion is hard because `ignore` might not have come first in the list.
+                        // just give a `help` instead.
+                        lint.span_help(
+                            sp.from_inner(InnerSpan::new(0, 3)),
+                            &format!("{}: ```text", explanation),
+                        );
+                    } else if empty_block {
+                        lint.span_suggestion(
+                            sp.from_inner(InnerSpan::new(0, 3)).shrink_to_hi(),
+                            explanation,
+                            "text",
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                } else if empty_block || is_ignore {
+                    lint.help(&format!("{}: ```text", explanation));
+                }
+
+                // FIXME(#67563): Provide more context for these errors by displaying the spans inline.
+                for message in buffer.messages.iter() {
+                    lint.note(message);
+                }
+
+                lint
+            },
         );
     }
 }