Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustdoc: correct unclosed HTML tags as generics #93569

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![feature(rustc_private)]
#![feature(array_methods)]
#![feature(assert_matches)]
#![feature(bool_to_option)]
#![feature(box_patterns)]
#![feature(control_flow_enum)]
#![feature(box_syntax)]
Expand Down
74 changes: 65 additions & 9 deletions src/librustdoc/passes/html_tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn drop_tag(
tags: &mut Vec<(String, Range<usize>)>,
tag_name: String,
range: Range<usize>,
f: &impl Fn(&str, &Range<usize>),
f: &impl Fn(&str, &Range<usize>, bool),
) {
let tag_name_low = tag_name.to_lowercase();
if let Some(pos) = tags.iter().rposition(|(t, _)| t.to_lowercase() == tag_name_low) {
Expand All @@ -59,14 +59,42 @@ fn drop_tag(
// `tags` is used as a queue, meaning that everything after `pos` is included inside it.
// So `<h2><h3></h2>` will look like `["h2", "h3"]`. So when closing `h2`, we will still
// have `h3`, meaning the tag wasn't closed as it should have.
f(&format!("unclosed HTML tag `{}`", last_tag_name), &last_tag_span);
f(&format!("unclosed HTML tag `{}`", last_tag_name), &last_tag_span, true);
}
// Remove the `tag_name` that was originally closed
tags.pop();
} else {
// It can happen for example in this case: `<h2></script></h2>` (the `h2` tag isn't required
// but it helps for the visualization).
f(&format!("unopened HTML tag `{}`", tag_name), &range);
f(&format!("unopened HTML tag `{}`", tag_name), &range, false);
}
}

fn extract_path_backwards(text: &str, end_pos: usize) -> Option<usize> {
use rustc_lexer::{is_id_continue, is_id_start};
let mut current_pos = end_pos;
loop {
if current_pos >= 2 && text[..current_pos].ends_with("::") {
current_pos -= 2;
}
let new_pos = text[..current_pos]
.char_indices()
.rev()
.take_while(|(_, c)| is_id_start(*c) || is_id_continue(*c))
.reduce(|_accum, item| item)
.and_then(|(new_pos, c)| is_id_start(c).then_some(new_pos));
if let Some(new_pos) = new_pos {
if current_pos != new_pos {
current_pos = new_pos;
continue;
}
}
break;
}
if current_pos == end_pos {
return None;
} else {
return Some(current_pos);
}
}

Expand All @@ -76,7 +104,7 @@ fn extract_html_tag(
range: &Range<usize>,
start_pos: usize,
iter: &mut Peekable<CharIndices<'_>>,
f: &impl Fn(&str, &Range<usize>),
f: &impl Fn(&str, &Range<usize>, bool),
) {
let mut tag_name = String::new();
let mut is_closing = false;
Expand Down Expand Up @@ -140,7 +168,7 @@ fn extract_tags(
text: &str,
range: Range<usize>,
is_in_comment: &mut Option<Range<usize>>,
f: &impl Fn(&str, &Range<usize>),
f: &impl Fn(&str, &Range<usize>, bool),
) {
let mut iter = text.char_indices().peekable();

Expand Down Expand Up @@ -178,14 +206,42 @@ impl<'a, 'tcx> DocVisitor for InvalidHtmlTagsLinter<'a, 'tcx> {
};
let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() {
let report_diag = |msg: &str, range: &Range<usize>| {
let report_diag = |msg: &str, range: &Range<usize>, is_open_tag: bool| {
let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs)
{
Some(sp) => sp,
None => item.attr_span(tcx),
};
tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
lint.build(msg).emit()
use rustc_lint_defs::Applicability;
let mut diag = lint.build(msg);
// If a tag looks like `<this>`, it might actually be a generic.
// We don't try to detect stuff `<like, this>` because that's not valid HTML,
// and we don't try to detect stuff `<like this>` because that's not valid Rust.
if let Some(Some(generics_start)) = (is_open_tag
&& dox[..range.end].ends_with(">"))
.then(|| extract_path_backwards(&dox, range.start))
{
let generics_sp = match super::source_span_for_markdown_range(
tcx,
&dox,
&(generics_start..range.end),
&item.attrs,
) {
Some(sp) => sp,
None => item.attr_span(tcx),
};
// multipart form is chosen here because ``Vec<i32>`` would be confusing.
diag.multipart_suggestion(
"try marking as source code",
vec![
(generics_sp.shrink_to_lo(), String::from("`")),
(generics_sp.shrink_to_hi(), String::from("`")),
],
Applicability::MaybeIncorrect,
);
}
diag.emit()
});
};

Expand All @@ -210,11 +266,11 @@ impl<'a, 'tcx> DocVisitor for InvalidHtmlTagsLinter<'a, 'tcx> {
let t = t.to_lowercase();
!ALLOWED_UNCLOSED.contains(&t.as_str())
}) {
report_diag(&format!("unclosed HTML tag `{}`", tag), range);
report_diag(&format!("unclosed HTML tag `{}`", tag), range, true);
}

if let Some(range) = is_in_comment {
report_diag("Unclosed HTML comment", &range);
report_diag("Unclosed HTML comment", &range, false);
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/test/rustdoc-ui/suggestions/html-as-generics-no-suggestions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![deny(rustdoc::invalid_html_tags)]

/// This Vec<32> thing!
// Numbers aren't valid HTML tags, so no error.
pub struct ConstGeneric;

/// This Vec<i32, i32> thing!
// HTML tags cannot contain commas, so no error.
pub struct MultipleGenerics;

/// This Vec<i32 class="test"> thing!
//~^ERROR unclosed HTML tag `i32`
// HTML attributes shouldn't be treated as Rust syntax, so no suggestions.
pub struct TagWithAttributes;

/// This Vec<i32></i32> thing!
// There should be no error, and no suggestion, since the tags are balanced.
pub struct DoNotWarnOnMatchingTags;

/// This Vec</i32> thing!
//~^ERROR unopened HTML tag `i32`
// This should produce an error, but no suggestion.
pub struct EndTagsAreNotValidRustSyntax;

/// This 123<i32> thing!
//~^ERROR unclosed HTML tag `i32`
// This should produce an error, but no suggestion.
pub struct NumbersAreNotPaths;

/// This Vec:<i32> thing!
//~^ERROR unclosed HTML tag `i32`
// This should produce an error, but no suggestion.
pub struct InvalidTurbofish;

/// This [link](https://rust-lang.org)<i32> thing!
//~^ERROR unclosed HTML tag `i32`
// This should produce an error, but no suggestion.
pub struct BareTurbofish;
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error: unclosed HTML tag `i32`
--> $DIR/html-as-generics-no-suggestions.rs:11:13
|
LL | /// This Vec<i32 class="test"> thing!
| ^^^^
|
note: the lint level is defined here
--> $DIR/html-as-generics-no-suggestions.rs:1:9
|
LL | #![deny(rustdoc::invalid_html_tags)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unopened HTML tag `i32`
--> $DIR/html-as-generics-no-suggestions.rs:20:13
|
LL | /// This Vec</i32> thing!
| ^^^^^^

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics-no-suggestions.rs:25:13
|
LL | /// This 123<i32> thing!
| ^^^^^

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics-no-suggestions.rs:30:14
|
LL | /// This Vec:<i32> thing!
| ^^^^^

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics-no-suggestions.rs:35:39
|
LL | /// This [link](https://rust-lang.org)<i32> thing!
| ^^^^^

error: aborting due to 5 previous errors

32 changes: 32 additions & 0 deletions src/test/rustdoc-ui/suggestions/html-as-generics.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// run-rustfix
#![deny(rustdoc::invalid_html_tags)]

/// This `Vec<i32>` thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Generic;

/// This `vec::Vec<i32>` thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct GenericPath;

/// This `i32<i32>` thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct PathsCanContainTrailingNumbers;

/// This `Vec::<i32>` thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Turbofish;

/// This [link](https://rust-lang.org)`::<i32>` thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct BareTurbofish;

/// This <span>`Vec::<i32>`</span> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Nested;
32 changes: 32 additions & 0 deletions src/test/rustdoc-ui/suggestions/html-as-generics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// run-rustfix
#![deny(rustdoc::invalid_html_tags)]

/// This Vec<i32> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Generic;

/// This vec::Vec<i32> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct GenericPath;

/// This i32<i32> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct PathsCanContainTrailingNumbers;

/// This Vec::<i32> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Turbofish;

/// This [link](https://rust-lang.org)::<i32> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct BareTurbofish;

/// This <span>Vec::<i32></span> thing!
//~^ERROR unclosed HTML tag `i32`
//~|HELP try marking as source
pub struct Nested;
73 changes: 73 additions & 0 deletions src/test/rustdoc-ui/suggestions/html-as-generics.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:4:13
|
LL | /// This Vec<i32> thing!
| ^^^^^
|
note: the lint level is defined here
--> $DIR/html-as-generics.rs:2:9
|
LL | #![deny(rustdoc::invalid_html_tags)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: try marking as source code
|
LL | /// This `Vec<i32>` thing!
| + +

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:9:18
|
LL | /// This vec::Vec<i32> thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This `vec::Vec<i32>` thing!
| + +

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:14:13
|
LL | /// This i32<i32> thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This `i32<i32>` thing!
| + +

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:19:15
|
LL | /// This Vec::<i32> thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This `Vec::<i32>` thing!
| + +

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:24:41
|
LL | /// This [link](https://rust-lang.org)::<i32> thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This [link](https://rust-lang.org)`::<i32>` thing!
| + +

error: unclosed HTML tag `i32`
--> $DIR/html-as-generics.rs:29:21
|
LL | /// This <span>Vec::<i32></span> thing!
| ^^^^^
|
help: try marking as source code
|
LL | /// This <span>`Vec::<i32>`</span> thing!
| + +

error: aborting due to 6 previous errors