diff options
author | bors <bors@rust-lang.org> | 2021-02-26 00:17:22 +0000 |
---|---|---|
committer | bors <bors@rust-lang.org> | 2021-02-26 00:17:22 +0000 |
commit | d95d30486180387a875b14633aea4e4dd8f960da (patch) | |
tree | 00fe29d185cb558b445fe81eec66bbbf9f8820f6 | |
parent | c0a54cc4eb6111cac9ad75cc439f75b79698b4a7 (diff) | |
parent | 66f4883308d999c8b405fdfd442562b8600a462d (diff) | |
download | rust-d95d30486180387a875b14633aea4e4dd8f960da.tar.gz |
Auto merge of #78429 - casey:doctest-attribute-splitting, r=jyn514
[librustdoc] Only split lang string on `,`, ` `, and `\t`
Split markdown lang strings into tokens on `,`.
The previous behavior was to split lang strings into tokens on any
character that wasn't a `_`, `_`, or alphanumeric.
This is a potentially breaking change, so please scrutinize! See discussion in #78344.
I noticed some test cases that made me wonder if there might have been some reason for the original behavior:
```
t("{.no_run .example}", false, true, Ignore::None, true, false, false, false, v(), None);
t("{.sh .should_panic}", true, false, Ignore::None, false, false, false, false, v(), None);
t("{.example .rust}", false, false, Ignore::None, true, false, false, false, v(), None);
t("{.test_harness .rust}", false, false, Ignore::None, true, true, false, false, v(), None);
```
It seemed pretty peculiar to specifically test lang strings in braces, with all the tokens prefixed by `.`.
I did some digging, and it looks like the test cases were added way back in [this commit from 2014](https://github.com/rust-lang/rust/commit/3fef7a74ca9a) by `@skade.`
It looks like they were added just to make sure that the splitting was permissive, and aren't testing that those strings in particular are accepted.
Closes https://github.com/rust-lang/rust/issues/78344.
-rw-r--r-- | compiler/rustc_error_codes/src/error_codes/E0761.md | 2 | ||||
-rw-r--r-- | compiler/rustc_mir/src/dataflow/framework/mod.rs | 4 | ||||
-rw-r--r-- | library/core/src/option.rs | 8 | ||||
-rw-r--r-- | library/core/src/result.rs | 12 | ||||
-rw-r--r-- | src/librustdoc/html/markdown.rs | 32 | ||||
-rw-r--r-- | src/librustdoc/html/markdown/tests.rs | 46 |
6 files changed, 84 insertions, 20 deletions
diff --git a/compiler/rustc_error_codes/src/error_codes/E0761.md b/compiler/rustc_error_codes/src/error_codes/E0761.md index e112674fbcc..760c5897698 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0761.md +++ b/compiler/rustc_error_codes/src/error_codes/E0761.md @@ -2,7 +2,7 @@ Multiple candidate files were found for an out-of-line module. Erroneous code example: -```ignore (multiple source files required for compile_fail) +```ignore (Multiple source files are required for compile_fail.) // file: ambiguous_module/mod.rs fn foo() {} diff --git a/compiler/rustc_mir/src/dataflow/framework/mod.rs b/compiler/rustc_mir/src/dataflow/framework/mod.rs index 524ad0af1a7..3f7808c2090 100644 --- a/compiler/rustc_mir/src/dataflow/framework/mod.rs +++ b/compiler/rustc_mir/src/dataflow/framework/mod.rs @@ -10,7 +10,7 @@ //! fixpoint solution to your dataflow problem, or implement the `ResultsVisitor` interface and use //! `visit_results`. The following example uses the `ResultsCursor` approach. //! -//! ```ignore(cross-crate-imports) +//! ```ignore (cross-crate-imports) //! use rustc_mir::dataflow::Analysis; // Makes `into_engine` available. //! //! fn do_my_analysis(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>) { @@ -211,7 +211,7 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { /// default impl and the one for all `A: GenKillAnalysis` will do the right thing. /// Its purpose is to enable method chaining like so: /// - /// ```ignore(cross-crate-imports) + /// ```ignore (cross-crate-imports) /// let results = MyAnalysis::new(tcx, body) /// .into_engine(tcx, body, def_id) /// .iterate_to_fixpoint() diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 14e4e4da3b9..bcd2b207c4f 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -336,7 +336,7 @@ impl<T> Option<T> { /// assert_eq!(x.expect("fruits are healthy"), "value"); /// ``` /// - /// ```{.should_panic} + /// ```should_panic /// let x: Option<&str> = None; /// x.expect("fruits are healthy"); // panics with `fruits are healthy` /// ``` @@ -372,7 +372,7 @@ impl<T> Option<T> { /// assert_eq!(x.unwrap(), "air"); /// ``` /// - /// ```{.should_panic} + /// ```should_panic /// let x: Option<&str> = None; /// assert_eq!(x.unwrap(), "air"); // fails /// ``` @@ -1114,7 +1114,7 @@ impl<T: fmt::Debug> Option<T> { /// } /// ``` /// - /// ```{.should_panic} + /// ```should_panic /// #![feature(option_expect_none)] /// /// use std::collections::HashMap; @@ -1156,7 +1156,7 @@ impl<T: fmt::Debug> Option<T> { /// } /// ``` /// - /// ```{.should_panic} + /// ```should_panic /// #![feature(option_unwrap_none)] /// /// use std::collections::HashMap; diff --git a/library/core/src/result.rs b/library/core/src/result.rs index d8747f8b8d6..c7121c7ee59 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -112,7 +112,7 @@ //! assert success with [`expect`]. This will panic if the //! write fails, providing a marginally useful message indicating why: //! -//! ```{.no_run} +//! ```no_run //! use std::fs::File; //! use std::io::prelude::*; //! @@ -122,7 +122,7 @@ //! //! You might also simply assert success: //! -//! ```{.no_run} +//! ```no_run //! # use std::fs::File; //! # use std::io::prelude::*; //! # let mut file = File::create("valuable_data.txt").unwrap(); @@ -984,7 +984,7 @@ impl<T, E: fmt::Debug> Result<T, E> { /// /// Basic usage: /// - /// ```{.should_panic} + /// ```should_panic /// let x: Result<u32, &str> = Err("emergency failure"); /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` /// ``` @@ -1024,7 +1024,7 @@ impl<T, E: fmt::Debug> Result<T, E> { /// assert_eq!(x.unwrap(), 2); /// ``` /// - /// ```{.should_panic} + /// ```should_panic /// let x: Result<u32, &str> = Err("emergency failure"); /// x.unwrap(); // panics with `emergency failure` /// ``` @@ -1052,7 +1052,7 @@ impl<T: fmt::Debug, E> Result<T, E> { /// /// Basic usage: /// - /// ```{.should_panic} + /// ```should_panic /// let x: Result<u32, &str> = Ok(10); /// x.expect_err("Testing expect_err"); // panics with `Testing expect_err: 10` /// ``` @@ -1075,7 +1075,7 @@ impl<T: fmt::Debug, E> Result<T, E> { /// /// # Examples /// - /// ```{.should_panic} + /// ```should_panic /// let x: Result<u32, &str> = Ok(2); /// x.unwrap_err(); // panics with `2` /// ``` diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index cb11f22d0d8..9a054e29dd3 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -780,6 +780,31 @@ impl LangString { Self::parse(string, allow_error_code_check, enable_per_target_ignores, None) } + fn tokens(string: &str) -> impl Iterator<Item = &str> { + // Pandoc, which Rust once used for generating documentation, + // expects lang strings to be surrounded by `{}` and for each token + // to be proceeded by a `.`. Since some of these lang strings are still + // loose in the wild, we strip a pair of surrounding `{}` from the lang + // string and a leading `.` from each token. + + let string = string.trim(); + + let first = string.chars().next(); + let last = string.chars().last(); + + let string = if first == Some('{') && last == Some('}') { + &string[1..string.len() - 1] + } else { + string + }; + + string + .split(|c| c == ',' || c == ' ' || c == '\t') + .map(str::trim) + .map(|token| if token.chars().next() == Some('.') { &token[1..] } else { token }) + .filter(|token| !token.is_empty()) + } + fn parse( string: &str, allow_error_code_check: ErrorCodes, @@ -793,11 +818,11 @@ impl LangString { let mut ignores = vec![]; data.original = string.to_owned(); - let tokens = string.split(|c: char| !(c == '_' || c == '-' || c.is_alphanumeric())); + + let tokens = Self::tokens(string).collect::<Vec<&str>>(); for token in tokens { - match token.trim() { - "" => {} + match token { "should_panic" => { data.should_panic = true; seen_rust_tags = !seen_other_tags; @@ -894,6 +919,7 @@ impl LangString { _ => seen_other_tags = true, } } + // ignore-foo overrides ignore if !ignores.is_empty() { data.ignore = Ignore::Some(ignores); diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs index 6b2cfe68575..59ca841715c 100644 --- a/src/librustdoc/html/markdown/tests.rs +++ b/src/librustdoc/html/markdown/tests.rs @@ -58,6 +58,9 @@ fn test_lang_string_parse() { t(Default::default()); t(LangString { original: "rust".into(), ..Default::default() }); + t(LangString { original: ".rust".into(), ..Default::default() }); + t(LangString { original: "{rust}".into(), ..Default::default() }); + t(LangString { original: "{.rust}".into(), ..Default::default() }); t(LangString { original: "sh".into(), rust: false, ..Default::default() }); t(LangString { original: "ignore".into(), ignore: Ignore::All, ..Default::default() }); t(LangString { @@ -75,16 +78,16 @@ fn test_lang_string_parse() { ..Default::default() }); t(LangString { original: "allow_fail".into(), allow_fail: true, ..Default::default() }); - t(LangString { original: "{.no_run .example}".into(), no_run: true, ..Default::default() }); + t(LangString { original: "no_run,example".into(), no_run: true, ..Default::default() }); t(LangString { - original: "{.sh .should_panic}".into(), + original: "sh,should_panic".into(), should_panic: true, rust: false, ..Default::default() }); - t(LangString { original: "{.example .rust}".into(), ..Default::default() }); + t(LangString { original: "example,rust".into(), ..Default::default() }); t(LangString { - original: "{.test_harness .rust}".into(), + original: "test_harness,.rust".into(), test_harness: true, ..Default::default() }); @@ -101,6 +104,18 @@ fn test_lang_string_parse() { ..Default::default() }); t(LangString { + original: "text,no_run, ".into(), + no_run: true, + rust: false, + ..Default::default() + }); + t(LangString { + original: "text,no_run,".into(), + no_run: true, + rust: false, + ..Default::default() + }); + t(LangString { original: "edition2015".into(), edition: Some(Edition::Edition2015), ..Default::default() @@ -113,6 +128,29 @@ fn test_lang_string_parse() { } #[test] +fn test_lang_string_tokenizer() { + fn case(lang_string: &str, want: &[&str]) { + let have = LangString::tokens(lang_string).collect::<Vec<&str>>(); + assert_eq!(have, want, "Unexpected lang string split for `{}`", lang_string); + } + + case("", &[]); + case("foo", &["foo"]); + case("foo,bar", &["foo", "bar"]); + case(".foo,.bar", &["foo", "bar"]); + case("{.foo,.bar}", &["foo", "bar"]); + case(" {.foo,.bar} ", &["foo", "bar"]); + case("foo bar", &["foo", "bar"]); + case("foo\tbar", &["foo", "bar"]); + case("foo\t, bar", &["foo", "bar"]); + case(" foo , bar ", &["foo", "bar"]); + case(",,foo,,bar,,", &["foo", "bar"]); + case("foo=bar", &["foo=bar"]); + case("a-b-c", &["a-b-c"]); + case("a_b_c", &["a_b_c"]); +} + +#[test] fn test_header() { fn t(input: &str, expect: &str) { let mut map = IdMap::new(); |