diff options
| author | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
| commit | 744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch) | |
| tree | 1721987352b5f0a8548fc46984821d974b661934 /src/librustdoc/passes/check_code_block_syntax.rs | |
| parent | 277bb6653b55475b5fbce6309e9852fa2100dabe (diff) | |
| parent | b5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff) | |
| download | rust-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.rs | 85 |
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 + }, ); } } |
