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 - implement #[doc(codeblock_attr = ...)] #84597

Closed
wants to merge 2 commits into from
Closed
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
102 changes: 102 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,103 @@ impl CheckAttrVisitor<'tcx> {
}
}

fn check_doc_codeblock_attr_value(
&self,
meta: &NestedMetaItem,
doc_codeblock_attr: &str,
is_list: bool,
) -> bool {
let tcx = self.tcx;
let err_fn = move |span: Span, msg: &str| {
tcx.sess.span_err(
span,
&format!(
"`#[doc(codeblock_attr{})]` {}",
if is_list { "(\"...\")" } else { " = \"...\"" },
msg,
),
);
false
};
if doc_codeblock_attr.is_empty() {
return err_fn(
meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
"attribute cannot have empty value",
);
}
if let Some(c) = doc_codeblock_attr
.chars()
.find(|&c| c == ',' || c == '`' || c == '"' || c == '\'' || c.is_whitespace())
Comment on lines +548 to +550
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is used somewhere else in rustdoc - could you find it and factor out an "is_lang_string_separator" function? Getting it wrong means rustdoc can't fix it without breaking backwards compatibility.

{
self.tcx.sess.span_err(
meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
&format!(
"{:?} character isn't allowed in `#[doc(codeblock_attr{})]`",
c,
if is_list { "(\"...\")" } else { " = \"...\"" },
),
);
return false;
}
if doc_codeblock_attr.starts_with(' ') || doc_codeblock_attr.ends_with(' ') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check will never run, you already give an error above if the lang string contains whitespace.

return err_fn(
meta.name_value_literal_span().unwrap_or_else(|| meta.span()),
"cannot start or end with ' '",
);
}
true
}

fn check_doc_codeblock_attr(&self, meta: &NestedMetaItem) -> bool {
if let Some(values) = meta.meta_item_list() {
let mut errors = 0;
for v in values {
match v.literal() {
Some(l) => match l.kind {
LitKind::Str(s, _) => {
if !self.check_doc_codeblock_attr_value(v, &s.as_str(), true) {
errors += 1;
}
}
_ => {
self.tcx
.sess
.struct_span_err(
v.span(),
"`#[doc(codeblock_attr(\"a\"))]` expects string literals",
)
.emit();
errors += 1;
}
},
None => {
self.tcx
.sess
.struct_span_err(
v.span(),
"`#[doc(codeblock_attr(\"a\"))]` expects string literals",
)
.emit();
errors += 1;
}
}
}
errors == 0
} else if let Some(doc_codeblock_attr) = meta.value_str().map(|s| s.to_string()) {
self.check_doc_codeblock_attr_value(meta, &doc_codeblock_attr, false)
} else {
self.tcx
.sess
.struct_span_err(
meta.span(),
"doc codeblock_attr attribute expects a string `#[doc(codeblock_attr = \"a\")]` or a list of \
strings `#[doc(codeblock_attr(\"a\", \"b\"))]`",
)
.emit();
false
}
}

fn check_doc_keyword(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool {
let doc_keyword = meta.value_str().map(|s| s.to_string()).unwrap_or_else(String::new);
if doc_keyword.is_empty() {
Expand Down Expand Up @@ -600,6 +697,10 @@ impl CheckAttrVisitor<'tcx> {
is_valid = false
}

sym::codeblock_attr if !self.check_doc_codeblock_attr(&meta) => {
is_valid = false
}

sym::keyword
if !self.check_attr_crate_level(&meta, hir_id, "keyword")
|| !self.check_doc_keyword(&meta, hir_id) =>
Expand Down Expand Up @@ -627,6 +728,7 @@ impl CheckAttrVisitor<'tcx> {
// passes: deprecated
// plugins: removed, but rustdoc warns about it itself
sym::alias
| sym::codeblock_attr
| sym::cfg
| sym::hidden
| sym::html_favicon_url
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ symbols! {
cmp,
cmpxchg16b_target_feature,
cmse_nonsecure_entry,
codeblock_attr,
coerce_unsized,
cold,
column,
Expand Down
21 changes: 21 additions & 0 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,27 @@ impl Attributes {
}
aliases.into_iter().collect::<Vec<String>>().into()
}

crate fn get_codeblock_attrs(&self) -> Box<[String]> {
let mut codeblocks = FxHashSet::default();

for attr in self.other_attrs.lists(sym::doc).filter(|a| a.has_name(sym::codeblock_attr)) {
if let Some(values) = attr.meta_item_list() {
for l in values {
match l.literal().unwrap().kind {
ast::LitKind::Str(s, _) => {
codeblocks.insert(s.as_str().to_string());
}
_ => unreachable!(),
}
}
} else {
codeblocks.insert(attr.value_str().map(|s| s.to_string()).unwrap());
}
}

codeblocks.into_iter().collect::<Vec<String>>().into()
}
}

impl PartialEq for Attributes {
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,8 +1115,10 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
.map(|span| span.ctxt().outer_expn().expansion_cause().unwrap_or(span))
.unwrap_or(DUMMY_SP);
self.collector.set_position(span);
let syntax_override = attrs.get_codeblock_attrs();
markdown::find_testable_code(
&doc,
&syntax_override,
self.collector,
self.codes,
self.collector.enable_per_target_ignores,
Expand Down
120 changes: 55 additions & 65 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ impl<'a, I: Iterator<Item = SpannedEvent<'a>>> Iterator for Footnotes<'a, I> {

crate fn find_testable_code<T: doctest::Tester>(
doc: &str,
syntax_override: &[String],
tests: &mut T,
error_codes: ErrorCodes,
enable_per_target_ignores: bool,
Expand All @@ -634,24 +635,24 @@ crate fn find_testable_code<T: doctest::Tester>(
let mut prev_offset = 0;
let mut nb_lines = 0;
let mut register_header = None;

let syntax_override =
if syntax_override.is_empty() { None } else { Some(syntax_override.join(",")) };
while let Some((event, offset)) = parser.next() {
match event {
Event::Start(Tag::CodeBlock(kind)) => {
let block_info = match kind {
CodeBlockKind::Fenced(ref lang) => {
if lang.is_empty() {
Default::default()
} else {
LangString::parse(
lang,
error_codes,
enable_per_target_ignores,
extra_info,
)
}
}
CodeBlockKind::Indented => Default::default(),
let lang = match &kind {
CodeBlockKind::Fenced(lang) => syntax_override
.as_deref()
.or_else(|| if lang.is_empty() { None } else { Some(&lang) }),
CodeBlockKind::Indented => syntax_override.as_deref(),
};
let block_info = lang
.map(|lang| {
LangString::parse(lang, error_codes, enable_per_target_ignores, extra_info)
})
.unwrap_or_default();

if !block_info.rust {
continue;
}
Expand Down Expand Up @@ -1248,7 +1249,11 @@ crate struct RustCodeBlock {

/// Returns a range of bytes for each code block in the markdown that is tagged as `rust` or
/// untagged (and assumed to be rust).
crate fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<RustCodeBlock> {
crate fn rust_code_blocks(
md: &str,
syntax_override: &[String],
extra_info: &ExtraInfo<'_>,
) -> Vec<RustCodeBlock> {
let mut code_blocks = vec![];

if md.is_empty() {
Expand All @@ -1257,74 +1262,59 @@ crate fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<RustCodeB

let mut p = Parser::new_ext(md, opts()).into_offset_iter();

let syntax_override =
if syntax_override.is_empty() { None } else { Some(syntax_override.join(",")) };

while let Some((event, offset)) = p.next() {
if let Event::Start(Tag::CodeBlock(syntax)) = event {
let (syntax, code_start, code_end, range, is_fenced, is_ignore) = match syntax {
CodeBlockKind::Fenced(syntax) => {
let syntax = syntax.as_ref();
let lang_string = if syntax.is_empty() {
Default::default()
} else {
LangString::parse(&*syntax, ErrorCodes::Yes, false, Some(extra_info))
};
if !lang_string.rust {
continue;
}
let is_ignore = lang_string.ignore != Ignore::None;
let syntax = if syntax.is_empty() { None } else { Some(syntax.to_owned()) };
let (code_start, mut code_end) = match p.next() {
Some((Event::Text(_), offset)) => (offset.start, offset.end),
Some((_, sub_offset)) => {
let code = Range { start: sub_offset.start, end: sub_offset.start };
code_blocks.push(RustCodeBlock {
is_fenced: true,
range: offset,
code,
syntax,
is_ignore,
});
continue;
let block_syntax = match &syntax {
CodeBlockKind::Fenced(syntax) => syntax_override
.as_deref()
.or_else(|| if syntax.is_empty() { None } else { Some(&syntax) }),
CodeBlockKind::Indented => syntax_override.as_deref(),
};

let lang_string = block_syntax
.map(|syntax| LangString::parse(syntax, ErrorCodes::Yes, false, Some(extra_info)))
.unwrap_or_default();
if !lang_string.rust {
continue;
}
let is_ignore = lang_string.ignore != Ignore::None;

let (code, range, is_fenced) = match &syntax {
CodeBlockKind::Fenced(_syntax) => {
let code = match p.next() {
Some((Event::Text(_), offset)) => {
let mut code = offset.clone();
while let Some((Event::Text(_), offset)) = p.next() {
code.end = offset.end;
}
code
}
None => {
let code = Range { start: offset.end, end: offset.end };
code_blocks.push(RustCodeBlock {
is_fenced: true,
range: offset,
code,
syntax,
is_ignore,
});
continue;
Some((_, sub_offset)) => {
Range { start: sub_offset.start, end: sub_offset.start }
}
None => Range { start: offset.end, end: offset.end },
};
while let Some((Event::Text(_), offset)) = p.next() {
code_end = offset.end;
}
(syntax, code_start, code_end, offset, true, is_ignore)
(code, offset, true)
}
CodeBlockKind::Indented => {
// The ending of the offset goes too far sometime so we reduce it by one in
// these cases.
if offset.end > offset.start && md.get(offset.end..=offset.end) == Some(&"\n") {
(
None,
offset.start,
offset.end,
Range { start: offset.start, end: offset.end - 1 },
false,
false,
)
(offset.clone(), Range { start: offset.start, end: offset.end - 1 }, false)
} else {
(None, offset.start, offset.end, offset, false, false)
(offset.clone(), offset, false)
}
}
};

code_blocks.push(RustCodeBlock {
is_fenced,
range,
code: Range { start: code_start, end: code_end },
syntax,
code,
syntax: block_syntax.map(str::to_string),
is_ignore,
});
}
Expand Down
9 changes: 8 additions & 1 deletion src/librustdoc/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,14 @@ crate fn test(mut options: Options) -> Result<(), String> {
collector.set_position(DUMMY_SP);
let codes = ErrorCodes::from(options.render_options.unstable_features.is_nightly_build());

find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None);
find_testable_code(
&input_str,
&[],
&mut collector,
codes,
options.enable_per_target_ignores,
None,
);

options.test_args.insert(0, "rustdoctest".to_string());
testing::test_main(
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,11 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> {
_ => {
let has_docs = !i.attrs.doc_strings.is_empty();
let mut tests = Tests { found_tests: 0 };
let syntax_override = i.attrs.get_codeblock_attrs();

find_testable_code(
&i.attrs.collapsed_doc_value().unwrap_or_default(),
&syntax_override,
&mut tests,
ErrorCodes::No,
false,
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/passes/check_code_block_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ impl<'a, 'tcx> DocFolder for SyntaxChecker<'a, 'tcx> {
if let Some(dox) = &item.attrs.collapsed_doc_value() {
let sp = item.attr_span(self.cx.tcx);
let extra = crate::html::markdown::ExtraInfo::new_did(self.cx.tcx, item.def_id, sp);
for code_block in markdown::rust_code_blocks(&dox, &extra) {
let syntax_override = item.attrs.get_codeblock_attrs();
for code_block in markdown::rust_code_blocks(&dox, &syntax_override, &extra) {
self.check_rust_syntax(&item, &dox, code_block);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/passes/doc_test_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) {
};

let mut tests = Tests { found_tests: 0 };
let syntax_override = item.attrs.get_codeblock_attrs();

find_testable_code(&dox, &mut tests, ErrorCodes::No, false, None);
find_testable_code(&dox, &syntax_override, &mut tests, ErrorCodes::No, false, None);

if tests.found_tests == 0 && cx.tcx.sess.is_nightly_build() {
if should_have_doc_example(cx, &item) {
Expand Down
Loading