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

Turn custom code classes in docs into warning #115914

Merged
Show file tree
Hide file tree
Changes from 2 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/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
def_id.to_def_id(),
span_of_fragments(&attrs.doc_strings).unwrap_or(sp),
)),
self.tcx.features().custom_code_classes_in_docs,
);
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/externalfiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ impl ExternalHtml {
edition,
playground,
heading_offset: HeadingOffset::H2,
// For external files, it'll be disabled until the feature is enabled by default.
custom_code_classes_in_docs: false,
}
.into_string()
);
Expand All @@ -61,6 +63,8 @@ impl ExternalHtml {
edition,
playground,
heading_offset: HeadingOffset::H2,
// For external files, it'll be disabled until the feature is enabled by default.
custom_code_classes_in_docs: false,
}
.into_string()
);
Expand Down
95 changes: 79 additions & 16 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
//! edition: Edition::Edition2015,
//! playground: &None,
//! heading_offset: HeadingOffset::H2,
//! custom_code_classes_in_docs: true,
//! };
//! let html = md.into_string();
//! // ... something using html
Expand Down Expand Up @@ -95,6 +96,8 @@ pub struct Markdown<'a> {
/// Offset at which we render headings.
/// E.g. if `heading_offset: HeadingOffset::H2`, then `# something` renders an `<h2>`.
pub heading_offset: HeadingOffset,
/// `true` if the `custom_code_classes_in_docs` feature is enabled.
pub custom_code_classes_in_docs: bool,
}
/// A struct like `Markdown` that renders the markdown with a table of contents.
pub(crate) struct MarkdownWithToc<'a> {
Expand All @@ -103,6 +106,8 @@ pub(crate) struct MarkdownWithToc<'a> {
pub(crate) error_codes: ErrorCodes,
pub(crate) edition: Edition,
pub(crate) playground: &'a Option<Playground>,
/// `true` if the `custom_code_classes_in_docs` feature is enabled.
pub(crate) custom_code_classes_in_docs: bool,
}
/// A tuple struct like `Markdown` that renders the markdown escaping HTML tags
/// and includes no paragraph tags.
Expand Down Expand Up @@ -203,6 +208,7 @@ struct CodeBlocks<'p, 'a, I: Iterator<Item = Event<'a>>> {
// Information about the playground if a URL has been specified, containing an
// optional crate name and the URL.
playground: &'p Option<Playground>,
custom_code_classes_in_docs: bool,
}

impl<'p, 'a, I: Iterator<Item = Event<'a>>> CodeBlocks<'p, 'a, I> {
Expand All @@ -211,8 +217,15 @@ impl<'p, 'a, I: Iterator<Item = Event<'a>>> CodeBlocks<'p, 'a, I> {
error_codes: ErrorCodes,
edition: Edition,
playground: &'p Option<Playground>,
custom_code_classes_in_docs: bool,
) -> Self {
CodeBlocks { inner: iter, check_error_codes: error_codes, edition, playground }
CodeBlocks {
inner: iter,
check_error_codes: error_codes,
edition,
playground,
custom_code_classes_in_docs,
}
}
}

Expand Down Expand Up @@ -242,8 +255,12 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {

let parse_result = match kind {
CodeBlockKind::Fenced(ref lang) => {
let parse_result =
LangString::parse_without_check(lang, self.check_error_codes, false);
let parse_result = LangString::parse_without_check(
lang,
self.check_error_codes,
false,
self.custom_code_classes_in_docs,
);
if !parse_result.rust {
let added_classes = parse_result.added_classes;
let lang_string = if let Some(lang) = parse_result.unknown.first() {
Expand Down Expand Up @@ -725,8 +742,17 @@ pub(crate) fn find_testable_code<T: doctest::Tester>(
error_codes: ErrorCodes,
enable_per_target_ignores: bool,
extra_info: Option<&ExtraInfo<'_>>,
custom_code_classes_in_docs: bool,
) {
find_codes(doc, tests, error_codes, enable_per_target_ignores, extra_info, false)
find_codes(
doc,
tests,
error_codes,
enable_per_target_ignores,
extra_info,
false,
custom_code_classes_in_docs,
)
}

pub(crate) fn find_codes<T: doctest::Tester>(
Expand All @@ -736,6 +762,7 @@ pub(crate) fn find_codes<T: doctest::Tester>(
enable_per_target_ignores: bool,
extra_info: Option<&ExtraInfo<'_>>,
include_non_rust: bool,
custom_code_classes_in_docs: bool,
) {
let mut parser = Parser::new(doc).into_offset_iter();
let mut prev_offset = 0;
Expand All @@ -754,6 +781,7 @@ pub(crate) fn find_codes<T: doctest::Tester>(
error_codes,
enable_per_target_ignores,
extra_info,
custom_code_classes_in_docs,
)
}
}
Expand Down Expand Up @@ -1153,15 +1181,23 @@ impl LangString {
string: &str,
allow_error_code_check: ErrorCodes,
enable_per_target_ignores: bool,
custom_code_classes_in_docs: bool,
) -> Self {
Self::parse(string, allow_error_code_check, enable_per_target_ignores, None)
Self::parse(
string,
allow_error_code_check,
enable_per_target_ignores,
None,
custom_code_classes_in_docs,
)
}

fn parse(
string: &str,
allow_error_code_check: ErrorCodes,
enable_per_target_ignores: bool,
extra: Option<&ExtraInfo<'_>>,
custom_code_classes_in_docs: bool,
) -> Self {
let allow_error_code_check = allow_error_code_check.as_bool();
let mut seen_rust_tags = false;
Expand Down Expand Up @@ -1197,7 +1233,11 @@ impl LangString {
seen_rust_tags = true;
}
LangStringToken::LangToken("custom") => {
seen_custom_tag = true;
if custom_code_classes_in_docs {
seen_custom_tag = true;
} else {
seen_other_tags = true;
}
}
LangStringToken::LangToken("test_harness") => {
data.test_harness = true;
Expand Down Expand Up @@ -1268,11 +1308,16 @@ impl LangString {
data.unknown.push(x.to_owned());
}
LangStringToken::KeyValueAttribute(key, value) => {
if key == "class" {
data.added_classes.push(value.to_owned());
} else if let Some(extra) = extra {
extra
.error_invalid_codeblock_attr(format!("unsupported attribute `{key}`"));
if custom_code_classes_in_docs {
if key == "class" {
data.added_classes.push(value.to_owned());
} else if let Some(extra) = extra {
extra.error_invalid_codeblock_attr(format!(
"unsupported attribute `{key}`"
));
}
} else {
seen_other_tags = true;
}
}
LangStringToken::ClassAttribute(class) => {
Expand Down Expand Up @@ -1302,6 +1347,7 @@ impl Markdown<'_> {
edition,
playground,
heading_offset,
custom_code_classes_in_docs,
} = self;

// This is actually common enough to special-case
Expand All @@ -1324,7 +1370,7 @@ impl Markdown<'_> {
let p = Footnotes::new(p);
let p = LinkReplacer::new(p.map(|(ev, _)| ev), links);
let p = TableWrapper::new(p);
let p = CodeBlocks::new(p, codes, edition, playground);
let p = CodeBlocks::new(p, codes, edition, playground, custom_code_classes_in_docs);
html::push_html(&mut s, p);

s
Expand All @@ -1333,7 +1379,14 @@ impl Markdown<'_> {

impl MarkdownWithToc<'_> {
pub(crate) fn into_string(self) -> String {
let MarkdownWithToc { content: md, ids, error_codes: codes, edition, playground } = self;
let MarkdownWithToc {
content: md,
ids,
error_codes: codes,
edition,
playground,
custom_code_classes_in_docs,
} = self;

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

Expand All @@ -1345,7 +1398,7 @@ impl MarkdownWithToc<'_> {
let p = HeadingLinks::new(p, Some(&mut toc), ids, HeadingOffset::H1);
let p = Footnotes::new(p);
let p = TableWrapper::new(p.map(|(ev, _)| ev));
let p = CodeBlocks::new(p, codes, edition, playground);
let p = CodeBlocks::new(p, codes, edition, playground, custom_code_classes_in_docs);
html::push_html(&mut s, p);
}

Expand Down Expand Up @@ -1786,7 +1839,11 @@ pub(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).
pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<RustCodeBlock> {
pub(crate) fn rust_code_blocks(
md: &str,
extra_info: &ExtraInfo<'_>,
custom_code_classes_in_docs: bool,
) -> Vec<RustCodeBlock> {
let mut code_blocks = vec![];

if md.is_empty() {
Expand All @@ -1803,7 +1860,13 @@ pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<Rust
let lang_string = if syntax.is_empty() {
Default::default()
} else {
LangString::parse(&*syntax, ErrorCodes::Yes, false, Some(extra_info))
LangString::parse(
&*syntax,
ErrorCodes::Yes,
false,
Some(extra_info),
custom_code_classes_in_docs,
)
};
if !lang_string.rust {
continue;
Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/html/markdown/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn test_unique_id() {
fn test_lang_string_parse() {
fn t(lg: LangString) {
let s = &lg.original;
assert_eq!(LangString::parse(s, ErrorCodes::Yes, true, None), lg)
assert_eq!(LangString::parse(s, ErrorCodes::Yes, true, None, true), lg)
}

t(Default::default());
Expand Down Expand Up @@ -290,6 +290,7 @@ fn test_header() {
edition: DEFAULT_EDITION,
playground: &None,
heading_offset: HeadingOffset::H2,
custom_code_classes_in_docs: true,
}
.into_string();
assert_eq!(output, expect, "original: {}", input);
Expand Down Expand Up @@ -329,6 +330,7 @@ fn test_header_ids_multiple_blocks() {
edition: DEFAULT_EDITION,
playground: &None,
heading_offset: HeadingOffset::H2,
custom_code_classes_in_docs: true,
}
.into_string();
assert_eq!(output, expect, "original: {}", input);
Expand Down Expand Up @@ -433,7 +435,7 @@ fn test_find_testable_code_line() {
}
}
let mut lines = Vec::<usize>::new();
find_testable_code(input, &mut lines, ErrorCodes::No, false, None);
find_testable_code(input, &mut lines, ErrorCodes::No, false, None, true);
assert_eq!(lines, expect);
}

Expand All @@ -458,6 +460,7 @@ fn test_ascii_with_prepending_hashtag() {
edition: DEFAULT_EDITION,
playground: &None,
heading_offset: HeadingOffset::H2,
custom_code_classes_in_docs: true,
}
.into_string();
assert_eq!(output, expect, "original: {}", input);
Expand Down
9 changes: 7 additions & 2 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ fn scrape_examples_help(shared: &SharedContext<'_>) -> String {
error_codes: shared.codes,
edition: shared.edition(),
playground: &shared.playground,
heading_offset: HeadingOffset::H1
heading_offset: HeadingOffset::H1,
custom_code_classes_in_docs: false,
}
.into_string()
)
Expand Down Expand Up @@ -437,6 +438,7 @@ fn render_markdown<'a, 'cx: 'a>(
heading_offset: HeadingOffset,
) -> impl fmt::Display + 'a + Captures<'cx> {
display_fn(move |f| {
let custom_code_classes_in_docs = cx.tcx().features().custom_code_classes_in_docs;
write!(
f,
"<div class=\"docblock\">{}</div>",
Expand All @@ -448,6 +450,7 @@ fn render_markdown<'a, 'cx: 'a>(
edition: cx.shared.edition(),
playground: &cx.shared.playground,
heading_offset,
custom_code_classes_in_docs,
}
.into_string()
)
Expand Down Expand Up @@ -1778,6 +1781,7 @@ fn render_impl(
</div>",
);
}
let custom_code_classes_in_docs = cx.tcx().features().custom_code_classes_in_docs;
write!(
w,
"<div class=\"docblock\">{}</div>",
Expand All @@ -1788,7 +1792,8 @@ fn render_impl(
error_codes: cx.shared.codes,
edition: cx.shared.edition(),
playground: &cx.shared.playground,
heading_offset: HeadingOffset::H4
heading_offset: HeadingOffset::H4,
custom_code_classes_in_docs,
}
.into_string()
);
Expand Down
14 changes: 13 additions & 1 deletion src/librustdoc/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ pub(crate) fn render<P: AsRef<Path>>(
error_codes,
edition,
playground: &playground,
// For markdown files, it'll be disabled until the feature is enabled by default.
custom_code_classes_in_docs: false,
}
.into_string()
} else {
Expand All @@ -91,6 +93,8 @@ pub(crate) fn render<P: AsRef<Path>>(
edition,
playground: &playground,
heading_offset: HeadingOffset::H1,
// For markdown files, it'll be disabled until the feature is enabled by default.
custom_code_classes_in_docs: false,
}
.into_string()
};
Expand Down Expand Up @@ -154,7 +158,15 @@ pub(crate) fn test(options: Options) -> Result<(), String> {
collector.set_position(DUMMY_SP);
let codes = ErrorCodes::from(options.unstable_features.is_nightly_build());

find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None);
// For markdown files, it'll be disabled until the feature is enabled by default.
Manishearth marked this conversation as resolved.
Show resolved Hide resolved
find_testable_code(
&input_str,
&mut collector,
codes,
options.enable_per_target_ignores,
None,
false,
);

crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests);
Ok(())
Expand Down
9 changes: 8 additions & 1 deletion src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,14 @@ impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
let has_docs = !i.attrs.doc_strings.is_empty();
let mut tests = Tests { found_tests: 0 };

find_testable_code(&i.doc_value(), &mut tests, ErrorCodes::No, false, None);
find_testable_code(
&i.doc_value(),
&mut tests,
ErrorCodes::No,
false,
None,
self.ctx.tcx.features().custom_code_classes_in_docs,
);

let has_doc_example = tests.found_tests != 0;
let hir_id = DocContext::as_local_hir_id(self.ctx.tcx, i.item_id).unwrap();
Expand Down
Loading