Skip to content

Commit

Permalink
New rustdoc lint to respect -Dwarnings correctly
Browse files Browse the repository at this point in the history
This adds a new lint to `rustc` that is used in rustdoc when a code
block is empty or cannot be parsed as valid Rust code.

Previously this was unconditionally a warning. As such some
documentation comments were (unknowingly) abusing this to pass despite
the `-Dwarnings` used when compiling `rustc`, this should not be the
case anymore.
  • Loading branch information
poliorcetics authored and jyn514 committed May 18, 2021
1 parent 3e99439 commit b574c67
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 40 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/borrow_check/region_infer/mod.rs
Expand Up @@ -1241,7 +1241,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// it. However, it works pretty well in practice. In particular,
/// this is needed to deal with projection outlives bounds like
///
/// ```ignore (internal compiler representation so lifetime syntax is invalid)
/// ```text (internal compiler representation so lifetime syntax is invalid)
/// <T as Foo<'0>>::Item: '1
/// ```
///
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/opaque_types.rs
Expand Up @@ -46,6 +46,7 @@ pub struct OpaqueTypeDecl<'tcx> {
/// type Foo = impl Baz;
/// fn bar() -> Foo {
/// // ^^^ This is the span we are looking for!
/// }
/// ```
///
/// In cases where the fn returns `(impl Trait, impl Trait)` or
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/upvar.rs
Expand Up @@ -323,7 +323,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
///
/// InferBorrowKind results in a structure like this:
///
/// ```
/// ```text
/// {
/// Place(base: hir_id_s, projections: [], ....) -> {
/// capture_kind_expr: hir_id_L5,
Expand All @@ -348,7 +348,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// ```
///
/// After the min capture analysis, we get:
/// ```
/// ```text
/// {
/// hir_id_s -> [
/// Place(base: hir_id_s, projections: [], ....) -> {
Expand Down
37 changes: 37 additions & 0 deletions src/doc/rustdoc/src/lints.md
Expand Up @@ -294,6 +294,43 @@ warning: unclosed HTML tag `h1`
warning: 2 warnings emitted
```

## invalid_rust_codeblock

This lint **warns by default**. It detects Rust code blocks in documentation
examples that are invalid (e.g. empty, not parsable as Rust). For example:

```rust
/// Empty code blocks (with and without the `rust` marker):
///
/// ```rust
/// ```
///
/// Unclosed code blocks (with and without the `rust` marker):
///
/// ```rust
fn main() {}
```

Which will give:

```text
warning: Rust code block is empty
--> src/lib.rs:3:5
|
3 | /// ```rust
| _____^
4 | | /// ```
| |_______^
|
= note: `#[warn(rustdoc::invalid_rust_codeblock)]` on by default

warning: Rust code block is empty
--> src/lib.rs:8:5
|
8 | /// ```rust
| ^^^^^^^
```
## bare_urls
This lint is **warn-by-default**. It detects URLs which are not links.
Expand Down
13 changes: 13 additions & 0 deletions src/librustdoc/lint.rs
Expand Up @@ -157,13 +157,26 @@ declare_rustdoc_lint! {
"detects URLs that are not hyperlinks"
}

declare_rustdoc_lint! {
/// The `invalid_rust_codeblock` lint detects Rust code blocks in
/// documentation examples that are invalid (e.g. empty, not parsable as
/// Rust code). This is a `rustdoc` only lint, see the documentation in the
/// [rustdoc book].
///
/// [rustdoc book]: ../../../rustdoc/lints.html#invalid_rust_codeblock
INVALID_RUST_CODEBLOCK,
Warn,
"codeblock could not be parsed as valid Rust or is empty"
}

crate static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
vec![
BROKEN_INTRA_DOC_LINKS,
PRIVATE_INTRA_DOC_LINKS,
MISSING_DOC_CODE_EXAMPLES,
PRIVATE_DOC_TESTS,
INVALID_CODEBLOCK_ATTRIBUTES,
INVALID_RUST_CODEBLOCK,
INVALID_HTML_TAGS,
BARE_URLS,
MISSING_CRATE_LEVEL_DOCS,
Expand Down
92 changes: 59 additions & 33 deletions src/librustdoc/passes/check_code_block_syntax.rs
@@ -1,5 +1,6 @@
use rustc_data_structures::sync::{Lock, Lrc};
use rustc_errors::{emitter::Emitter, Applicability, Diagnostic, Handler};
use rustc_middle::lint::LintDiagnosticBuilder;
use rustc_parse::parse_stream_from_source_str;
use rustc_session::parse::ParseSess;
use rustc_span::source_map::{FilePathMapping, SourceMap};
Expand Down Expand Up @@ -47,50 +48,65 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
.unwrap_or(false);
let buffer = buffer.borrow();

if buffer.has_errors || is_empty {
let mut diag = if let Some(sp) = super::source_span_for_markdown_range(
self.cx.tcx,
&dox,
&code_block.range,
&item.attrs,
) {
let (warning_message, suggest_using_text) = if buffer.has_errors {
("could not parse code block as Rust code", true)
if !(buffer.has_errors || is_empty) {
// No errors in a non-empty program.
return;
}

let local_id = match item.def_id.as_local() {
Some(id) => id,
// We don't need to check the syntax for other crates so returning
// without doing anything should not be a problem.
None => return,
};

let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_id);
let suggest_using_text = code_block.syntax.is_none() && code_block.is_fenced;
let is_ignore = code_block.is_ignore;

// The span and whether it is precise or not.
let (sp, precise_span) = match super::source_span_for_markdown_range(
self.cx.tcx,
&dox,
&code_block.range,
&item.attrs,
) {
Some(sp) => (sp, true),
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 mut diag = if precise_span {
let msg = if buffer.has_errors {
"could not parse code block as Rust code"
} else {
("Rust code block is empty", false)
"Rust code block is empty"
};

let mut diag = self.cx.sess().struct_span_warn(sp, warning_message);
let mut diag = lint.build(msg);

if suggest_using_text {
let extended_msg = 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 code_block.syntax.is_none() && code_block.is_fenced {
let sp = sp.from_inner(InnerSpan::new(0, 3));
diag.span_suggestion(
sp,
"mark blocks that do not contain Rust code as text",
sp.from_inner(InnerSpan::new(0, 3)),
extended_msg,
String::from("```text"),
Applicability::MachineApplicable,
);
} else if suggest_using_text && code_block.is_ignore {
let sp = sp.from_inner(InnerSpan::new(0, 3));
diag.span_suggestion(
sp,
"`ignore` code blocks require valid Rust code for syntax highlighting. \
Mark blocks that do not contain Rust code as text",
String::from("```text,"),
Applicability::MachineApplicable,
);
}

diag
} else {
// We couldn't calculate the span of the markdown block that had the error, so our
// diagnostics are going to be a bit lacking.
let mut diag = self.cx.sess().struct_span_warn(
item.attr_span(self.cx.tcx),
"doc comment contains an invalid Rust code block",
);

if code_block.syntax.is_none() && code_block.is_fenced {
let mut diag = lint.build("doc comment contains an invalid Rust code block");
if suggest_using_text {
diag.help("mark blocks that do not contain Rust code as text: ```text");
}

Expand All @@ -103,7 +119,17 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
}

diag.emit();
}
};

// Finally build and emit the completed diagnostic.
// All points of divergence have been handled earlier so this can be
// done the same way whether the span is precise or not.
self.cx.tcx.struct_span_lint_hir(
crate::lint::INVALID_RUST_CODEBLOCK,
hir_id,
sp,
diag_builder,
);
}
}

Expand Down
5 changes: 1 addition & 4 deletions src/test/rustdoc-ui/ignore-block-help.stderr
Expand Up @@ -7,11 +7,8 @@ LL | | /// let heart = '❤️';
LL | | /// ```
| |_______^
|
= note: `#[warn(invalid_rust_codeblock)]` on by default
= note: error from rustc: character literal may only contain one codepoint
help: `ignore` code blocks require valid Rust code for syntax highlighting. Mark blocks that do not contain Rust code as text
|
LL | /// ```text,ignore (to-prevent-tidy-error)
| ^^^^^^^^

warning: 1 warning emitted

1 change: 1 addition & 0 deletions src/test/rustdoc-ui/invalid-syntax.stderr
Expand Up @@ -7,6 +7,7 @@ LL | | /// \__________pkt->size___________/ \_result->size_/ \__pkt->si
LL | | /// ```
| |_______^
|
= note: `#[warn(invalid_rust_codeblock)]` on by default
= note: error from rustc: unknown start of token: \
= note: error from rustc: unknown start of token: \
= note: error from rustc: unknown start of token: \
Expand Down

0 comments on commit b574c67

Please sign in to comment.