From 0af5a6be0531a1ad038bd150239847cb4acc5026 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Jul 2018 16:12:16 -0600 Subject: [PATCH 01/10] Pull out nightly checking to edges Parsing the code block's LangString (```foo) previously checked itself to see if we were on nightly; that isn't the right place to do so. Move that check slightly outwards to better abstract LangString. (This is also an optimization as we avoid the costly environment variable load of RUSTC_BOOTSTRAP). --- src/librustdoc/html/markdown.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 8d85adfb3d0..d01745f4a46 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -129,12 +129,14 @@ thread_local!(pub static PLAYGROUND: RefCell, String)>> = /// Adds syntax highlighting and playground Run buttons to rust code blocks. struct CodeBlocks<'a, I: Iterator>> { inner: I, + check_error_codes: bool, } impl<'a, I: Iterator>> CodeBlocks<'a, I> { fn new(iter: I) -> Self { CodeBlocks { inner: iter, + check_error_codes: UnstableFeatures::from_environment().is_nightly_build(), } } } @@ -147,7 +149,7 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { let compile_fail; let ignore; if let Some(Event::Start(Tag::CodeBlock(lang))) = event { - let parse_result = LangString::parse(&lang); + let parse_result = LangString::parse(&lang, self.check_error_codes); if !parse_result.rust { return Some(Event::Start(Tag::CodeBlock(lang))); } @@ -471,6 +473,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp sess: Option<&session::Session>) { tests.set_position(position); + let is_nightly = UnstableFeatures::from_environment().is_nightly_build(); let mut parser = Parser::new(doc); let mut prev_offset = 0; let mut nb_lines = 0; @@ -481,7 +484,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp let block_info = if s.is_empty() { LangString::all_false() } else { - LangString::parse(&*s) + LangString::parse(&*s, is_nightly) }; if !block_info.rust { continue @@ -569,14 +572,10 @@ impl LangString { } } - fn parse(string: &str) -> LangString { + fn parse(string: &str, allow_error_code_check: bool) -> LangString { let mut seen_rust_tags = false; let mut seen_other_tags = false; let mut data = LangString::all_false(); - let mut allow_error_code_check = false; - if UnstableFeatures::from_environment().is_nightly_build() { - allow_error_code_check = true; - } data.original = string.to_owned(); let tokens = string.split(|c: char| @@ -842,7 +841,7 @@ mod tests { fn t(s: &str, should_panic: bool, no_run: bool, ignore: bool, rust: bool, test_harness: bool, compile_fail: bool, allow_fail: bool, error_codes: Vec) { - assert_eq!(LangString::parse(s), LangString { + assert_eq!(LangString::parse(s, true), LangString { should_panic, no_run, ignore, From ad40e4517f1969e60eac9fcac2affa75bb2e715e Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Jul 2018 16:30:02 -0600 Subject: [PATCH 02/10] Provide warnings for invalid code blocks in markdown files Previously we would only warn on Rust code but we can also do so when testing markdown (the diag::Handler is available). --- src/librustdoc/html/markdown.rs | 8 +++----- src/librustdoc/markdown.rs | 2 +- src/librustdoc/test.rs | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index d01745f4a46..0774ce34718 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -27,7 +27,6 @@ #![allow(non_camel_case_types)] -use rustc::session; use std::cell::RefCell; use std::collections::{HashMap, VecDeque}; use std::default::Default; @@ -37,6 +36,7 @@ use std::ops::Range; use std::str; use syntax::feature_gate::UnstableFeatures; use syntax::codemap::Span; +use errors; use html::render::derive_id; use html::toc::TocBuilder; @@ -470,7 +470,7 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { } pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Span, - sess: Option<&session::Session>) { + handler: &errors::Handler) { tests.set_position(position); let is_nightly = UnstableFeatures::from_environment().is_nightly_build(); @@ -521,9 +521,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp line, filename, block_info.allow_fail); prev_offset = offset; } else { - if let Some(ref sess) = sess { - sess.span_warn(position, "invalid start of a new code block"); - } + handler.span_warn(position, "invalid start of a new code block"); break; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 36a8fc94dba..82489e9fbdb 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -156,7 +156,7 @@ pub fn test(input: &str, cfgs: Vec, libs: SearchPaths, externs: Externs, true, opts, maybe_sysroot, None, Some(PathBuf::from(input)), linker, edition); - find_testable_code(&input_str, &mut collector, DUMMY_SP, None); + find_testable_code(&input_str, &mut collector, DUMMY_SP, diag); test_args.insert(0, "rustdoctest".to_string()); testing::test_main(&test_args, collector.tests, testing::Options::new().display_output(display_warnings)); diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 2966b9e9819..a6667e17728 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -692,7 +692,7 @@ impl<'a, 'hir> HirCollector<'a, 'hir> { markdown::find_testable_code(&doc, self.collector, attrs.span.unwrap_or(DUMMY_SP), - Some(self.sess)); + self.sess.diagnostic()); } nested(self); From de5cebdba58770ab555476bc4cdf23d89bd0c3ea Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Jul 2018 16:54:30 -0600 Subject: [PATCH 03/10] Provide test configuration through struct This is far more sound than passing many different arguments of the same type. --- src/librustdoc/html/markdown.rs | 25 ++++++++++--------------- src/librustdoc/test.rs | 24 +++++++++++------------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 0774ce34718..c73e6d4b355 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -513,12 +513,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp let text = lines.collect::>>().join("\n"); nb_lines += doc[prev_offset..offset].lines().count(); let line = tests.get_line() + (nb_lines - 1); - let filename = tests.get_filename(); - tests.add_test(text.to_owned(), - block_info.should_panic, block_info.no_run, - block_info.ignore, block_info.test_harness, - block_info.compile_fail, block_info.error_codes, - line, filename, block_info.allow_fail); + tests.add_test(text, block_info, line); prev_offset = offset; } else { handler.span_warn(position, "invalid start of a new code block"); @@ -543,16 +538,16 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp } #[derive(Eq, PartialEq, Clone, Debug)] -struct LangString { +pub struct LangString { original: String, - should_panic: bool, - no_run: bool, - ignore: bool, - rust: bool, - test_harness: bool, - compile_fail: bool, - error_codes: Vec, - allow_fail: bool, + pub should_panic: bool, + pub no_run: bool, + pub ignore: bool, + pub rust: bool, + pub test_harness: bool, + pub compile_fail: bool, + pub error_codes: Vec, + pub allow_fail: bool, } impl LangString { diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index a6667e17728..2885ce615d2 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -42,7 +42,7 @@ use errors; use errors::emitter::ColorConfig; use clean::Attributes; -use html::markdown; +use html::markdown::{self, LangString}; #[derive(Clone, Default)] pub struct TestOptions { @@ -533,10 +533,8 @@ impl Collector { format!("{} - {} (line {})", filename, self.names.join("::"), line) } - pub fn add_test(&mut self, test: String, - should_panic: bool, no_run: bool, should_ignore: bool, - as_test_harness: bool, compile_fail: bool, error_codes: Vec, - line: usize, filename: FileName, allow_fail: bool) { + pub fn add_test(&mut self, test: String, config: LangString, line: usize) { + let filename = self.get_filename(); let name = self.generate_name(line, &filename); let cfgs = self.cfgs.clone(); let libs = self.libs.clone(); @@ -551,10 +549,10 @@ impl Collector { self.tests.push(testing::TestDescAndFn { desc: testing::TestDesc { name: testing::DynTestName(name.clone()), - ignore: should_ignore, + ignore: config.ignore, // compiler failures are test failures should_panic: testing::ShouldPanic::No, - allow_fail, + allow_fail: config.allow_fail, }, testfn: testing::DynTestFn(box move || { let panic = io::set_panic(None); @@ -572,11 +570,11 @@ impl Collector { libs, cg, externs, - should_panic, - no_run, - as_test_harness, - compile_fail, - error_codes, + config.should_panic, + config.no_run, + config.test_harness, + config.compile_fail, + config.error_codes, &opts, maybe_sysroot, linker, @@ -604,7 +602,7 @@ impl Collector { self.position = position; } - pub fn get_filename(&self) -> FileName { + fn get_filename(&self) -> FileName { if let Some(ref codemap) = self.codemap { let filename = codemap.span_to_filename(self.position); if let FileName::Real(ref filename) = filename { From 03e34f8f816c2594238585bdca39716f4050cb69 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Jul 2018 17:15:25 -0600 Subject: [PATCH 04/10] Remove dependency on error handling from find_testable_code --- src/librustdoc/html/markdown.rs | 21 +++++++++++++-------- src/librustdoc/markdown.rs | 6 +++++- src/librustdoc/test.rs | 10 ++++++---- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index c73e6d4b355..54fd041fb00 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -34,10 +34,8 @@ use std::fmt::{self, Write}; use std::borrow::Cow; use std::ops::Range; use std::str; -use syntax::feature_gate::UnstableFeatures; -use syntax::codemap::Span; -use errors; +use syntax::feature_gate::UnstableFeatures; use html::render::derive_id; use html::toc::TocBuilder; use html::highlight; @@ -469,10 +467,17 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { } } -pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Span, - handler: &errors::Handler) { - tests.set_position(position); +pub struct TestableCodeError(()); +impl fmt::Display for TestableCodeError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "invalid start of a new code block") + } +} + +pub fn find_testable_code( + doc: &str, tests: &mut test::Collector +) -> Result<(), TestableCodeError> { let is_nightly = UnstableFeatures::from_environment().is_nightly_build(); let mut parser = Parser::new(doc); let mut prev_offset = 0; @@ -516,8 +521,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp tests.add_test(text, block_info, line); prev_offset = offset; } else { - handler.span_warn(position, "invalid start of a new code block"); - break; + return Err(TestableCodeError(())); } } Event::Start(Tag::Header(level)) => { @@ -535,6 +539,7 @@ pub fn find_testable_code(doc: &str, tests: &mut ::test::Collector, position: Sp _ => {} } } + Ok(()) } #[derive(Eq, PartialEq, Clone, Debug)] diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 82489e9fbdb..4d1eae1470f 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -156,7 +156,11 @@ pub fn test(input: &str, cfgs: Vec, libs: SearchPaths, externs: Externs, true, opts, maybe_sysroot, None, Some(PathBuf::from(input)), linker, edition); - find_testable_code(&input_str, &mut collector, DUMMY_SP, diag); + collector.set_position(DUMMY_SP); + let res = find_testable_code(&input_str, &mut collector); + if let Err(err) = res { + diag.span_warn(DUMMY_SP, &err.to_string()); + } test_args.insert(0, "rustdoctest".to_string()); testing::test_main(&test_args, collector.tests, testing::Options::new().display_output(display_warnings)); diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 2885ce615d2..9723b452800 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -687,10 +687,12 @@ impl<'a, 'hir> HirCollector<'a, 'hir> { // the collapse-docs pass won't combine sugared/raw doc attributes, or included files with // anything else, this will combine them for us if let Some(doc) = attrs.collapsed_doc_value() { - markdown::find_testable_code(&doc, - self.collector, - attrs.span.unwrap_or(DUMMY_SP), - self.sess.diagnostic()); + self.collector.set_position(attrs.span.unwrap_or(DUMMY_SP)); + let res = markdown::find_testable_code(&doc, self.collector); + if let Err(err) = res { + self.sess.diagnostic().span_warn(attrs.span.unwrap_or(DUMMY_SP), + &err.to_string()); + } } nested(self); From 01d95558e62f40f958618292afd4abae2fb3c236 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 22 Jul 2018 11:10:19 -0600 Subject: [PATCH 05/10] Further extract error code switch Removes dependency on UnstableFeatures from markdown rendering --- src/librustdoc/externalfiles.rs | 8 ++- src/librustdoc/html/markdown.rs | 81 ++++++++++++++++--------- src/librustdoc/html/render.rs | 40 ++++++------ src/librustdoc/markdown.rs | 11 ++-- src/librustdoc/test.rs | 10 +-- src/tools/error_index_generator/main.rs | 4 +- 6 files changed, 96 insertions(+), 58 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 10b6c9850ae..67502ab8e15 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -12,7 +12,8 @@ use std::fs; use std::path::Path; use std::str; use errors; -use html::markdown::Markdown; +use syntax::feature_gate::UnstableFeatures; +use html::markdown::{ErrorCodes, Markdown}; #[derive(Clone)] pub struct ExternalHtml { @@ -31,6 +32,7 @@ impl ExternalHtml { pub fn load(in_header: &[String], before_content: &[String], after_content: &[String], md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler) -> Option { + let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); load_external_files(in_header, diag) .and_then(|ih| load_external_files(before_content, diag) @@ -38,7 +40,7 @@ impl ExternalHtml { ) .and_then(|(ih, bc)| load_external_files(md_before_content, diag) - .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[])))) + .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], codes)))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +48,7 @@ impl ExternalHtml { ) .and_then(|(ih, bc, ac)| load_external_files(md_after_content, diag) - .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[])))) + .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], codes)))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 54fd041fb00..f65211f97be 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -18,10 +18,10 @@ //! ``` //! #![feature(rustc_private)] //! -//! use rustdoc::html::markdown::Markdown; +//! use rustdoc::html::markdown::{Markdown, ErrorCodes}; //! //! let s = "My *markdown* _text_"; -//! let html = format!("{}", Markdown(s, &[])); +//! let html = format!("{}", Markdown(s, &[], ErrorCodes::Yes)); //! // ... something using html //! ``` @@ -35,7 +35,6 @@ use std::borrow::Cow; use std::ops::Range; use std::str; -use syntax::feature_gate::UnstableFeatures; use html::render::derive_id; use html::toc::TocBuilder; use html::highlight; @@ -48,15 +47,37 @@ use pulldown_cmark::{Options, OPTION_ENABLE_FOOTNOTES, OPTION_ENABLE_TABLES}; /// formatted, this struct will emit the HTML corresponding to the rendered /// version of the contained markdown string. /// The second parameter is a list of link replacements -pub struct Markdown<'a>(pub &'a str, pub &'a [(String, String)]); +pub struct Markdown<'a>(pub &'a str, pub &'a [(String, String)], pub ErrorCodes); /// A unit struct like `Markdown`, that renders the markdown with a /// table of contents. -pub struct MarkdownWithToc<'a>(pub &'a str); +pub struct MarkdownWithToc<'a>(pub &'a str, pub ErrorCodes); /// A unit struct like `Markdown`, that renders the markdown escaping HTML tags. -pub struct MarkdownHtml<'a>(pub &'a str); +pub struct MarkdownHtml<'a>(pub &'a str, pub ErrorCodes); /// A unit struct like `Markdown`, that renders only the first paragraph. pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); +#[derive(Copy, Clone, PartialEq, Debug)] +pub enum ErrorCodes { + Yes, + No, +} + +impl ErrorCodes { + pub fn from(b: bool) -> Self { + match b { + true => ErrorCodes::Yes, + false => ErrorCodes::No, + } + } + + pub fn as_bool(self) -> bool { + match self { + ErrorCodes::Yes => true, + ErrorCodes::No => false, + } + } +} + /// Controls whether a line will be hidden or shown in HTML output. /// /// All lines are used in documentation tests. @@ -127,14 +148,14 @@ thread_local!(pub static PLAYGROUND: RefCell, String)>> = /// Adds syntax highlighting and playground Run buttons to rust code blocks. struct CodeBlocks<'a, I: Iterator>> { inner: I, - check_error_codes: bool, + check_error_codes: ErrorCodes, } impl<'a, I: Iterator>> CodeBlocks<'a, I> { - fn new(iter: I) -> Self { + fn new(iter: I, error_codes: ErrorCodes) -> Self { CodeBlocks { inner: iter, - check_error_codes: UnstableFeatures::from_environment().is_nightly_build(), + check_error_codes: error_codes, } } } @@ -476,9 +497,8 @@ impl fmt::Display for TestableCodeError { } pub fn find_testable_code( - doc: &str, tests: &mut test::Collector + doc: &str, tests: &mut test::Collector, error_codes: ErrorCodes, ) -> Result<(), TestableCodeError> { - let is_nightly = UnstableFeatures::from_environment().is_nightly_build(); let mut parser = Parser::new(doc); let mut prev_offset = 0; let mut nb_lines = 0; @@ -489,7 +509,7 @@ pub fn find_testable_code( let block_info = if s.is_empty() { LangString::all_false() } else { - LangString::parse(&*s, is_nightly) + LangString::parse(&*s, error_codes) }; if !block_info.rust { continue @@ -570,7 +590,8 @@ impl LangString { } } - fn parse(string: &str, allow_error_code_check: bool) -> LangString { + fn parse(string: &str, allow_error_code_check: ErrorCodes) -> LangString { + let allow_error_code_check = allow_error_code_check.as_bool(); let mut seen_rust_tags = false; let mut seen_other_tags = false; let mut data = LangString::all_false(); @@ -620,7 +641,7 @@ impl LangString { impl<'a> fmt::Display for Markdown<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let Markdown(md, links) = *self; + let Markdown(md, links, codes) = *self; // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -645,7 +666,7 @@ impl<'a> fmt::Display for Markdown<'a> { CodeBlocks::new( LinkReplacer::new( HeadingLinks::new(p, None), - links)))); + links), codes))); fmt.write_str(&s) } @@ -653,7 +674,7 @@ impl<'a> fmt::Display for Markdown<'a> { impl<'a> fmt::Display for MarkdownWithToc<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let MarkdownWithToc(md) = *self; + let MarkdownWithToc(md, codes) = *self; let mut opts = Options::empty(); opts.insert(OPTION_ENABLE_TABLES); @@ -665,8 +686,12 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { let mut toc = TocBuilder::new(); - html::push_html(&mut s, - Footnotes::new(CodeBlocks::new(HeadingLinks::new(p, Some(&mut toc))))); + { + let p = HeadingLinks::new(p, Some(&mut toc)); + let p = CodeBlocks::new(p, codes); + let p = Footnotes::new(p); + html::push_html(&mut s, p); + } write!(fmt, "", toc.into_toc())?; @@ -676,7 +701,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { impl<'a> fmt::Display for MarkdownHtml<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let MarkdownHtml(md) = *self; + let MarkdownHtml(md, codes) = *self; // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -694,8 +719,10 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); - html::push_html(&mut s, - Footnotes::new(CodeBlocks::new(HeadingLinks::new(p, None)))); + let p = HeadingLinks::new(p, None); + let p = CodeBlocks::new(p, codes); + let p = Footnotes::new(p); + html::push_html(&mut s, p); fmt.write_str(&s) } @@ -830,7 +857,7 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { #[cfg(test)] mod tests { - use super::{LangString, Markdown, MarkdownHtml}; + use super::{ErrorCodes, LangString, Markdown, MarkdownHtml}; use super::plain_summary_line; use html::render::reset_ids; @@ -839,7 +866,7 @@ mod tests { fn t(s: &str, should_panic: bool, no_run: bool, ignore: bool, rust: bool, test_harness: bool, compile_fail: bool, allow_fail: bool, error_codes: Vec) { - assert_eq!(LangString::parse(s, true), LangString { + assert_eq!(LangString::parse(s, ErrorCodes::Yes), LangString { should_panic, no_run, ignore, @@ -878,14 +905,14 @@ mod tests { #[test] fn issue_17736() { let markdown = "# title"; - Markdown(markdown, &[]).to_string(); + Markdown(markdown, &[], ErrorCodes::Yes).to_string(); reset_ids(true); } #[test] fn test_header() { fn t(input: &str, expect: &str) { - let output = Markdown(input, &[]).to_string(); + let output = Markdown(input, &[], ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); reset_ids(true); } @@ -907,7 +934,7 @@ mod tests { #[test] fn test_header_ids_multiple_blocks() { fn t(input: &str, expect: &str) { - let output = Markdown(input, &[]).to_string(); + let output = Markdown(input, &[], ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); } @@ -948,7 +975,7 @@ mod tests { #[test] fn test_markdown_html_escape() { fn t(input: &str, expect: &str) { - let output = MarkdownHtml(input).to_string(); + let output = MarkdownHtml(input, ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 24a9bf416e4..b9b058cb548 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -56,6 +56,7 @@ use externalfiles::ExternalHtml; use serialize::json::{ToJson, Json, as_json}; use syntax::ast; use syntax::codemap::FileName; +use syntax::feature_gate::UnstableFeatures; use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId}; use rustc::middle::privacy::AccessLevels; use rustc::middle::stability; @@ -72,7 +73,7 @@ use html::format::{GenericBounds, WhereClause, href, AbiSpace}; use html::format::{VisSpace, Method, UnsafetySpace, MutableSpace}; use html::format::fmt_impl_for_trait_page; use html::item_type::ItemType; -use html::markdown::{self, Markdown, MarkdownHtml, MarkdownSummaryLine}; +use html::markdown::{self, Markdown, MarkdownHtml, MarkdownSummaryLine, ErrorCodes}; use html::{highlight, layout}; use minifier; @@ -99,6 +100,7 @@ pub struct Context { /// real location of an item. This is used to allow external links to /// publicly reused items to redirect to the right location. pub render_redirect_pages: bool, + pub codes: ErrorCodes, pub shared: Arc, } @@ -581,6 +583,7 @@ pub fn run(mut krate: clean::Crate, current: Vec::new(), dst, render_redirect_pages: false, + codes: ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()), shared: Arc::new(scx), }; @@ -2221,13 +2224,14 @@ fn document(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item) -> fmt::Re fn render_markdown(w: &mut fmt::Formatter, md_text: &str, links: Vec<(String, String)>, - prefix: &str,) + prefix: &str, + codes: ErrorCodes) -> fmt::Result { - write!(w, "
{}{}
", prefix, Markdown(md_text, &links)) + write!(w, "
{}{}
", prefix, Markdown(md_text, &links, codes)) } fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLink, - prefix: &str) -> fmt::Result { + prefix: &str, codes: ErrorCodes) -> fmt::Result { if let Some(s) = item.doc_value() { let markdown = if s.contains('\n') { format!("{} [Read more]({})", @@ -2235,7 +2239,7 @@ fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLin } else { plain_summary_line(Some(s)).to_string() }; - render_markdown(w, &markdown, item.links(), prefix)?; + render_markdown(w, &markdown, item.links(), prefix, codes)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -2261,7 +2265,7 @@ fn document_full(w: &mut fmt::Formatter, item: &clean::Item, cx: &Context, prefix: &str) -> fmt::Result { if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) { debug!("Doc block: =====\n{}\n=====", s); - render_markdown(w, &*s, item.links(), prefix)?; + render_markdown(w, &*s, item.links(), prefix, cx.codes)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -2508,6 +2512,7 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec { let mut stability = vec![]; + let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); if let Some(stab) = item.stability.as_ref() { let deprecated_reason = if show_reason && !stab.deprecated_reason.is_empty() { @@ -2521,14 +2526,11 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec{}", text)) }; @@ -2559,7 +2561,9 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec{}", unstable_extra, - MarkdownHtml(&stab.unstable_reason)); + MarkdownHtml( + &stab.unstable_reason, + error_codes)); stability.push(format!("
{}
", text)); } @@ -2582,11 +2586,11 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec{}", text)) } @@ -3811,7 +3815,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi write!(w, "")?; if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { write!(w, "
{}
", - Markdown(&*dox, &i.impl_item.links()))?; + Markdown(&*dox, &i.impl_item.links(), cx.codes))?; } } @@ -3897,7 +3901,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi } else if show_def_docs { // In case the item isn't documented, // provide short documentation from the trait. - document_short(w, it, link, &prefix)?; + document_short(w, it, link, &prefix, cx.codes)?; } } } else { @@ -3909,7 +3913,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi } else { document_stability(w, cx, item)?; if show_def_docs { - document_short(w, item, link, &prefix)?; + document_short(w, item, link, &prefix, cx.codes)?; } } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 4d1eae1470f..97a3b2c1ea0 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -19,6 +19,7 @@ use testing; use rustc::session::search_paths::SearchPaths; use rustc::session::config::{Externs, CodegenOptions}; use syntax::codemap::DUMMY_SP; +use syntax::feature_gate::UnstableFeatures; use syntax::edition::Edition; use externalfiles::{ExternalHtml, LoadStringError, load_string}; @@ -26,7 +27,7 @@ use externalfiles::{ExternalHtml, LoadStringError, load_string}; use html::render::reset_ids; use html::escape::Escape; use html::markdown; -use html::markdown::{Markdown, MarkdownWithToc, find_testable_code}; +use html::markdown::{ErrorCodes, Markdown, MarkdownWithToc, find_testable_code}; use test::{TestOptions, Collector}; /// Separate any lines at the start of the file that begin with `# ` or `%`. @@ -88,10 +89,11 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches, reset_ids(false); + let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); let text = if include_toc { - MarkdownWithToc(text).to_string() + MarkdownWithToc(text, error_codes).to_string() } else { - Markdown(text, &[]).to_string() + Markdown(text, &[], error_codes).to_string() }; let err = write!( @@ -157,7 +159,8 @@ pub fn test(input: &str, cfgs: Vec, libs: SearchPaths, externs: Externs, Some(PathBuf::from(input)), linker, edition); collector.set_position(DUMMY_SP); - let res = find_testable_code(&input_str, &mut collector); + let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); + let res = find_testable_code(&input_str, &mut collector, codes); if let Err(err) = res { diag.span_warn(DUMMY_SP, &err.to_string()); } diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 9723b452800..650a2408aa6 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -42,7 +42,7 @@ use errors; use errors::emitter::ColorConfig; use clean::Attributes; -use html::markdown::{self, LangString}; +use html::markdown::{self, ErrorCodes, LangString}; #[derive(Clone, Default)] pub struct TestOptions { @@ -145,7 +145,8 @@ pub fn run(input_path: &Path, let mut hir_collector = HirCollector { sess: &sess, collector: &mut collector, - map: &map + map: &map, + codes: ErrorCodes::from(sess.opts.unstable_features.is_nightly_build()), }; hir_collector.visit_testable("".to_string(), &krate.attrs, |this| { intravisit::walk_crate(this, krate); @@ -662,7 +663,8 @@ impl Collector { struct HirCollector<'a, 'hir: 'a> { sess: &'a session::Session, collector: &'a mut Collector, - map: &'a hir::map::Map<'hir> + map: &'a hir::map::Map<'hir>, + codes: ErrorCodes, } impl<'a, 'hir> HirCollector<'a, 'hir> { @@ -688,7 +690,7 @@ impl<'a, 'hir> HirCollector<'a, 'hir> { // anything else, this will combine them for us if let Some(doc) = attrs.collapsed_doc_value() { self.collector.set_position(attrs.span.unwrap_or(DUMMY_SP)); - let res = markdown::find_testable_code(&doc, self.collector); + let res = markdown::find_testable_code(&doc, self.collector, self.codes); if let Err(err) = res { self.sess.diagnostic().span_warn(attrs.span.unwrap_or(DUMMY_SP), &err.to_string()); diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index e72f90554d3..79c5be125b3 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -24,7 +24,7 @@ use std::path::PathBuf; use syntax::diagnostics::metadata::{get_metadata_dir, ErrorMetadataMap, ErrorMetadata}; -use rustdoc::html::markdown::{Markdown, PLAYGROUND}; +use rustdoc::html::markdown::{Markdown, ErrorCodes, PLAYGROUND}; use rustc_serialize::json; enum OutputFormat { @@ -100,7 +100,7 @@ impl Formatter for HTMLFormatter { // Description rendered as markdown. match info.description { - Some(ref desc) => write!(output, "{}", Markdown(desc, &[]))?, + Some(ref desc) => write!(output, "{}", Markdown(desc, &[], ErrorCodes::Yes))?, None => write!(output, "

No description.

\n")?, } From 2216db9de78f2da623bea80594f1d9a7b54ddb54 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 22 Jul 2018 07:39:52 -0600 Subject: [PATCH 06/10] Format code for easier editing --- src/librustdoc/html/markdown.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index f65211f97be..8f897387564 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -661,12 +661,11 @@ impl<'a> fmt::Display for Markdown<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); - html::push_html(&mut s, - Footnotes::new( - CodeBlocks::new( - LinkReplacer::new( - HeadingLinks::new(p, None), - links), codes))); + let p = HeadingLinks::new(p, None); + let p = LinkReplacer::new(p, links); + let p = CodeBlocks::new(p, codes); + let p = Footnotes::new(p); + html::push_html(&mut s, p); fmt.write_str(&s) } From 7bea518d3a792d9c8df508b0af83ceb857ce75b7 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 22 Jul 2018 07:25:00 -0600 Subject: [PATCH 07/10] Remove global derive_id and reset_ids functions Previously these functions relied on TLS but we can instead thread the relevant state through explicitly. --- src/librustdoc/externalfiles.rs | 12 +- src/librustdoc/html/markdown.rs | 154 ++++++++++++++++-------- src/librustdoc/html/render.rs | 147 +++++++++------------- src/librustdoc/lib.rs | 6 +- src/librustdoc/markdown.rs | 11 +- src/tools/error_index_generator/main.rs | 13 +- 6 files changed, 190 insertions(+), 153 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 67502ab8e15..9631ea059cc 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -13,7 +13,8 @@ use std::path::Path; use std::str; use errors; use syntax::feature_gate::UnstableFeatures; -use html::markdown::{ErrorCodes, Markdown}; +use html::markdown::{IdMap, ErrorCodes, Markdown}; +use std::cell::RefCell; #[derive(Clone)] pub struct ExternalHtml { @@ -30,7 +31,8 @@ pub struct ExternalHtml { impl ExternalHtml { pub fn load(in_header: &[String], before_content: &[String], after_content: &[String], - md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler) + md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler, + id_map: &mut IdMap) -> Option { let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); load_external_files(in_header, diag) @@ -40,7 +42,8 @@ impl ExternalHtml { ) .and_then(|(ih, bc)| load_external_files(md_before_content, diag) - .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], codes)))) + .map(|m_bc| (ih, + format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), codes)))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -48,7 +51,8 @@ impl ExternalHtml { ) .and_then(|(ih, bc, ac)| load_external_files(md_after_content, diag) - .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], codes)))) + .map(|m_ac| (ih, bc, + format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), codes)))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 8f897387564..706671f21ef 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -18,10 +18,12 @@ //! ``` //! #![feature(rustc_private)] //! -//! use rustdoc::html::markdown::{Markdown, ErrorCodes}; +//! use rustdoc::html::markdown::{IdMap, Markdown, ErrorCodes}; +//! use std::cell::RefCell; //! //! let s = "My *markdown* _text_"; -//! let html = format!("{}", Markdown(s, &[], ErrorCodes::Yes)); +//! let mut id_map = IdMap::new(); +//! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), ErrorCodes::Yes)); //! // ... something using html //! ``` @@ -35,7 +37,6 @@ use std::borrow::Cow; use std::ops::Range; use std::str; -use html::render::derive_id; use html::toc::TocBuilder; use html::highlight; use test; @@ -47,12 +48,13 @@ use pulldown_cmark::{Options, OPTION_ENABLE_FOOTNOTES, OPTION_ENABLE_TABLES}; /// formatted, this struct will emit the HTML corresponding to the rendered /// version of the contained markdown string. /// The second parameter is a list of link replacements -pub struct Markdown<'a>(pub &'a str, pub &'a [(String, String)], pub ErrorCodes); +pub struct Markdown<'a>( + pub &'a str, pub &'a [(String, String)], pub RefCell<&'a mut IdMap>, pub ErrorCodes); /// A unit struct like `Markdown`, that renders the markdown with a /// table of contents. -pub struct MarkdownWithToc<'a>(pub &'a str, pub ErrorCodes); +pub struct MarkdownWithToc<'a>(pub &'a str, pub RefCell<&'a mut IdMap>, pub ErrorCodes); /// A unit struct like `Markdown`, that renders the markdown escaping HTML tags. -pub struct MarkdownHtml<'a>(pub &'a str, pub ErrorCodes); +pub struct MarkdownHtml<'a>(pub &'a str, pub RefCell<&'a mut IdMap>, pub ErrorCodes); /// A unit struct like `Markdown`, that renders only the first paragraph. pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); @@ -287,23 +289,25 @@ impl<'a, 'b, I: Iterator>> Iterator for LinkReplacer<'a, 'b, I> } /// Make headings links with anchor ids and build up TOC. -struct HeadingLinks<'a, 'b, I: Iterator>> { +struct HeadingLinks<'a, 'b, 'ids, I: Iterator>> { inner: I, toc: Option<&'b mut TocBuilder>, buf: VecDeque>, + id_map: &'ids mut IdMap, } -impl<'a, 'b, I: Iterator>> HeadingLinks<'a, 'b, I> { - fn new(iter: I, toc: Option<&'b mut TocBuilder>) -> Self { +impl<'a, 'b, 'ids, I: Iterator>> HeadingLinks<'a, 'b, 'ids, I> { + fn new(iter: I, toc: Option<&'b mut TocBuilder>, ids: &'ids mut IdMap) -> Self { HeadingLinks { inner: iter, toc, buf: VecDeque::new(), + id_map: ids, } } } -impl<'a, 'b, I: Iterator>> Iterator for HeadingLinks<'a, 'b, I> { +impl<'a, 'b, 'ids, I: Iterator>> Iterator for HeadingLinks<'a, 'b, 'ids, I> { type Item = Event<'a>; fn next(&mut self) -> Option { @@ -322,7 +326,7 @@ impl<'a, 'b, I: Iterator>> Iterator for HeadingLinks<'a, 'b, I> } self.buf.push_back(event); } - let id = derive_id(id); + let id = self.id_map.derive(id); if let Some(ref mut builder) = self.toc { let mut html_header = String::new(); @@ -641,7 +645,8 @@ impl LangString { impl<'a> fmt::Display for Markdown<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let Markdown(md, links, codes) = *self; + let Markdown(md, links, ref ids, codes) = *self; + let mut ids = ids.borrow_mut(); // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -661,7 +666,7 @@ impl<'a> fmt::Display for Markdown<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); - let p = HeadingLinks::new(p, None); + let p = HeadingLinks::new(p, None, &mut ids); let p = LinkReplacer::new(p, links); let p = CodeBlocks::new(p, codes); let p = Footnotes::new(p); @@ -673,7 +678,8 @@ impl<'a> fmt::Display for Markdown<'a> { impl<'a> fmt::Display for MarkdownWithToc<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let MarkdownWithToc(md, codes) = *self; + let MarkdownWithToc(md, ref ids, codes) = *self; + let mut ids = ids.borrow_mut(); let mut opts = Options::empty(); opts.insert(OPTION_ENABLE_TABLES); @@ -686,7 +692,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { let mut toc = TocBuilder::new(); { - let p = HeadingLinks::new(p, Some(&mut toc)); + let p = HeadingLinks::new(p, Some(&mut toc), &mut ids); let p = CodeBlocks::new(p, codes); let p = Footnotes::new(p); html::push_html(&mut s, p); @@ -700,7 +706,8 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { impl<'a> fmt::Display for MarkdownHtml<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let MarkdownHtml(md, codes) = *self; + let MarkdownHtml(md, ref ids, codes) = *self; + let mut ids = ids.borrow_mut(); // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -718,7 +725,7 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); - let p = HeadingLinks::new(p, None); + let p = HeadingLinks::new(p, None, &mut ids); let p = CodeBlocks::new(p, codes); let p = Footnotes::new(p); html::push_html(&mut s, p); @@ -835,7 +842,10 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { let p = Parser::new_with_broken_link_callback(md, opts, Some(&push)); - let iter = Footnotes::new(HeadingLinks::new(p, None)); + // There's no need to thread an IdMap through to here because + // the IDs generated aren't going to be emitted anywhere. + let mut ids = IdMap::new(); + let iter = Footnotes::new(HeadingLinks::new(p, None, &mut ids)); for ev in iter { if let Event::Start(Tag::Link(dest, _)) = ev { @@ -854,11 +864,67 @@ pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { links } +#[derive(Default)] +pub struct IdMap { + map: HashMap, +} + +impl IdMap { + pub fn new() -> Self { + IdMap::default() + } + + pub fn populate>(&mut self, ids: I) { + for id in ids { + let _ = self.derive(id); + } + } + + pub fn reset(&mut self) { + self.map = HashMap::new(); + } + + pub fn derive(&mut self, candidate: String) -> String { + let id = match self.map.get_mut(&candidate) { + None => candidate, + Some(a) => { + let id = format!("{}-{}", candidate, *a); + *a += 1; + id + } + }; + + self.map.insert(id.clone(), 1); + id + } +} + +#[cfg(test)] +#[test] +fn test_unique_id() { + let input = ["foo", "examples", "examples", "method.into_iter","examples", + "method.into_iter", "foo", "main", "search", "methods", + "examples", "method.into_iter", "assoc_type.Item", "assoc_type.Item"]; + let expected = ["foo", "examples", "examples-1", "method.into_iter", "examples-2", + "method.into_iter-1", "foo-1", "main", "search", "methods", + "examples-3", "method.into_iter-2", "assoc_type.Item", "assoc_type.Item-1"]; + + let map = RefCell::new(IdMap::new()); + let test = || { + let mut map = map.borrow_mut(); + let actual: Vec = input.iter().map(|s| map.derive(s.to_string())).collect(); + assert_eq!(&actual[..], expected); + }; + test(); + map.borrow_mut().reset(); + test(); +} + #[cfg(test)] mod tests { - use super::{ErrorCodes, LangString, Markdown, MarkdownHtml}; + use super::{ErrorCodes, LangString, Markdown, MarkdownHtml, IdMap}; use super::plain_summary_line; - use html::render::reset_ids; + use std::cell::RefCell; #[test] fn test_lang_string_parse() { @@ -901,19 +967,12 @@ mod tests { t("text,no_run", false, true, false, false, false, false, false, v()); } - #[test] - fn issue_17736() { - let markdown = "# title"; - Markdown(markdown, &[], ErrorCodes::Yes).to_string(); - reset_ids(true); - } - #[test] fn test_header() { fn t(input: &str, expect: &str) { - let output = Markdown(input, &[], ErrorCodes::Yes).to_string(); + let mut map = IdMap::new(); + let output = Markdown(input, &[], RefCell::new(&mut map), ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); - reset_ids(true); } t("# Foo bar", "

\ @@ -932,28 +991,24 @@ mod tests { #[test] fn test_header_ids_multiple_blocks() { - fn t(input: &str, expect: &str) { - let output = Markdown(input, &[], ErrorCodes::Yes).to_string(); + let mut map = IdMap::new(); + fn t(map: &mut IdMap, input: &str, expect: &str) { + let output = Markdown(input, &[], RefCell::new(map), ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); } - let test = || { - t("# Example", "

\ - Example

"); - t("# Panics", "

\ - Panics

"); - t("# Example", "

\ - Example

"); - t("# Main", "

\ - Main

"); - t("# Example", "

\ - Example

"); - t("# Panics", "

\ - Panics

"); - }; - test(); - reset_ids(true); - test(); + t(&mut map, "# Example", "

\ + Example

"); + t(&mut map, "# Panics", "

\ + Panics

"); + t(&mut map, "# Example", "

\ + Example

"); + t(&mut map, "# Main", "

\ + Main

"); + t(&mut map, "# Example", "

\ + Example

"); + t(&mut map, "# Panics", "

\ + Panics

"); } #[test] @@ -974,7 +1029,8 @@ mod tests { #[test] fn test_markdown_html_escape() { fn t(input: &str, expect: &str) { - let output = MarkdownHtml(input, ErrorCodes::Yes).to_string(); + let mut idmap = IdMap::new(); + let output = MarkdownHtml(input, RefCell::new(&mut idmap), ErrorCodes::Yes).to_string(); assert_eq!(output, expect, "original: {}", input); } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b9b058cb548..c1375fd37fd 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -50,6 +50,7 @@ use std::mem; use std::path::{PathBuf, Path, Component}; use std::str; use std::sync::Arc; +use std::rc::Rc; use externalfiles::ExternalHtml; @@ -73,7 +74,7 @@ use html::format::{GenericBounds, WhereClause, href, AbiSpace}; use html::format::{VisSpace, Method, UnsafetySpace, MutableSpace}; use html::format::fmt_impl_for_trait_page; use html::item_type::ItemType; -use html::markdown::{self, Markdown, MarkdownHtml, MarkdownSummaryLine, ErrorCodes}; +use html::markdown::{self, Markdown, MarkdownHtml, MarkdownSummaryLine, ErrorCodes, IdMap}; use html::{highlight, layout}; use minifier; @@ -89,7 +90,7 @@ pub type NameDoc = (String, Option); /// easily cloned because it is cloned per work-job (about once per item in the /// rustdoc tree). #[derive(Clone)] -pub struct Context { +struct Context { /// Current hierarchy of components leading down to what's currently being /// rendered pub current: Vec, @@ -101,10 +102,12 @@ pub struct Context { /// publicly reused items to redirect to the right location. pub render_redirect_pages: bool, pub codes: ErrorCodes, + /// The map used to ensure all generated 'id=' attributes are unique. + id_map: Rc>, pub shared: Arc, } -pub struct SharedContext { +struct SharedContext { /// The path to the crate root source minus the file name. /// Used for simplifying paths to the highlighted source code files. pub src_root: PathBuf, @@ -452,9 +455,8 @@ impl ToJson for IndexItemFunctionType { thread_local!(static CACHE_KEY: RefCell> = Default::default()); thread_local!(pub static CURRENT_LOCATION_KEY: RefCell> = RefCell::new(Vec::new())); -thread_local!(pub static USED_ID_MAP: RefCell> = RefCell::new(init_ids())); -fn init_ids() -> FxHashMap { +pub fn initial_ids() -> Vec { [ "main", "search", @@ -472,36 +474,7 @@ fn init_ids() -> FxHashMap { "methods", "deref-methods", "implementations", - ].into_iter().map(|id| (String::from(*id), 1)).collect() -} - -/// This method resets the local table of used ID attributes. This is typically -/// used at the beginning of rendering an entire HTML page to reset from the -/// previous state (if any). -pub fn reset_ids(embedded: bool) { - USED_ID_MAP.with(|s| { - *s.borrow_mut() = if embedded { - init_ids() - } else { - FxHashMap() - }; - }); -} - -pub fn derive_id(candidate: String) -> String { - USED_ID_MAP.with(|map| { - let id = match map.borrow_mut().get_mut(&candidate) { - None => candidate, - Some(a) => { - let id = format!("{}-{}", candidate, *a); - *a += 1; - id - } - }; - - map.borrow_mut().insert(id.clone(), 1); - id - }) + ].into_iter().map(|id| (String::from(*id))).collect() } /// Generates the documentation for `crate` into the directory `dst` @@ -515,7 +488,8 @@ pub fn run(mut krate: clean::Crate, renderinfo: RenderInfo, sort_modules_alphabetically: bool, themes: Vec, - enable_minification: bool) -> Result<(), Error> { + enable_minification: bool, + id_map: IdMap) -> Result<(), Error> { let src_root = match krate.src { FileName::Real(ref p) => match p.parent() { Some(p) => p.to_path_buf(), @@ -584,6 +558,7 @@ pub fn run(mut krate: clean::Crate, dst, render_redirect_pages: false, codes: ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()), + id_map: Rc::new(RefCell::new(id_map)), shared: Arc::new(scx), }; @@ -1711,6 +1686,11 @@ impl<'a> fmt::Display for Settings<'a> { } impl Context { + fn derive_id(&self, id: String) -> String { + let mut map = self.id_map.borrow_mut(); + map.derive(id) + } + /// String representation of how to get back to the root path of the 'doc/' /// folder in terms of a relative URL. fn root_path(&self) -> String { @@ -1865,7 +1845,10 @@ impl Context { resource_suffix: &self.shared.resource_suffix, }; - reset_ids(true); + { + self.id_map.borrow_mut().reset(); + self.id_map.borrow_mut().populate(initial_ids()); + } if !self.render_redirect_pages { layout::render(writer, &self.shared.layout, &page, @@ -2222,16 +2205,18 @@ fn document(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item) -> fmt::Re /// Render md_text as markdown. fn render_markdown(w: &mut fmt::Formatter, + cx: &Context, md_text: &str, links: Vec<(String, String)>, - prefix: &str, - codes: ErrorCodes) + prefix: &str) -> fmt::Result { - write!(w, "
{}{}
", prefix, Markdown(md_text, &links, codes)) + let mut ids = cx.id_map.borrow_mut(); + write!(w, "
{}{}
", + prefix, Markdown(md_text, &links, RefCell::new(&mut ids), cx.codes)) } -fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLink, - prefix: &str, codes: ErrorCodes) -> fmt::Result { +fn document_short(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item, link: AssocItemLink, + prefix: &str) -> fmt::Result { if let Some(s) = item.doc_value() { let markdown = if s.contains('\n') { format!("{} [Read more]({})", @@ -2239,7 +2224,7 @@ fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLin } else { plain_summary_line(Some(s)).to_string() }; - render_markdown(w, &markdown, item.links(), prefix, codes)?; + render_markdown(w, cx, &markdown, item.links(), prefix)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -2265,7 +2250,7 @@ fn document_full(w: &mut fmt::Formatter, item: &clean::Item, cx: &Context, prefix: &str) -> fmt::Result { if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) { debug!("Doc block: =====\n{}\n=====", s); - render_markdown(w, &*s, item.links(), prefix, cx.codes)?; + render_markdown(w, cx, &*s, item.links(), prefix)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -2431,7 +2416,7 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, let (short, name) = item_ty_to_strs(&myty.unwrap()); write!(w, "

\ {name}

\n", - id = derive_id(short.to_owned()), name = name)?; + id = cx.derive_id(short.to_owned()), name = name)?; } match myitem.inner { @@ -2526,7 +2511,8 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec Vec", unstable_extra)); } else { + let mut ids = cx.id_map.borrow_mut(); let text = format!("🔬 \ This is a nightly-only experimental API. {}\ {}", unstable_extra, MarkdownHtml( &stab.unstable_reason, + RefCell::new(&mut ids), error_codes)); stability.push(format!("
{}
", text)); @@ -2583,14 +2571,15 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec{}", text)) } @@ -2831,8 +2820,8 @@ fn item_trait( -> fmt::Result { let name = m.name.as_ref().unwrap(); let item_type = m.type_(); - let id = derive_id(format!("{}.{}", item_type, name)); - let ns_id = derive_id(format!("{}.{}", name, item_type.name_space())); + let id = cx.derive_id(format!("{}.{}", item_type, name)); + let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space())); write!(w, "{extra}

\

")?; if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { + let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", - Markdown(&*dox, &i.impl_item.links(), cx.codes))?; + Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), cx.codes))?; } } @@ -3836,8 +3826,8 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi clean::TyMethodItem(clean::TyMethod{ ref decl, .. }) => { // Only render when the method is not static or we allow static methods if render_method_item { - let id = derive_id(format!("{}.{}", item_type, name)); - let ns_id = derive_id(format!("{}.{}", name, item_type.name_space())); + let id = cx.derive_id(format!("{}.{}", item_type, name)); + let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space())); write!(w, "

", id, item_type)?; write!(w, "{}", spotlight_decl(decl)?)?; write!(w, "