From 56d3bc70080ec9ae4fca650df8708d0aae410f41 Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 8 Jul 2016 18:18:45 +0200 Subject: [PATCH] Handle `/**` and `~~~` in `DOC_MARKDOWN` --- clippy_lints/src/doc.rs | 126 ++++++++++++++++++++++++++++++++------ tests/compile-fail/doc.rs | 50 ++++++++++++++- 2 files changed, 156 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 4306204e5..92fcd804d 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -50,25 +50,50 @@ impl EarlyLintPass for Doc { } } +/// Cleanup documentation decoration (`///` and such). +/// +/// We can't use `syntax::attr::AttributeMethods::with_desugared_doc` or +/// `syntax::parse::lexer::comments::strip_doc_comment_decoration` because we need to keep track of +/// the span but this function is inspired from the later. +#[allow(cast_possible_truncation)] +pub fn strip_doc_comment_decoration((comment, span): (&str, Span)) -> Vec<(&str, Span)> { + // one-line comments lose their prefix + const ONELINERS: &'static [&'static str] = &["///!", "///", "//!", "//"]; + for prefix in ONELINERS { + if comment.starts_with(*prefix) { + return vec![( + &comment[prefix.len()..], + Span { lo: span.lo + BytePos(prefix.len() as u32), ..span } + )]; + } + } + + if comment.starts_with("/*") { + return comment[3..comment.len() - 2].lines().map(|line| { + let offset = line.as_ptr() as usize - comment.as_ptr() as usize; + debug_assert_eq!(offset as u32 as usize, offset); + + ( + line, + Span { + lo: span.lo + BytePos(offset as u32), + ..span + } + ) + }).collect(); + } + + panic!("not a doc-comment: {}", comment); +} + pub fn check_attrs<'a>(cx: &EarlyContext, valid_idents: &[String], attrs: &'a [ast::Attribute]) { let mut docs = vec![]; - let mut in_multiline = false; for attr in attrs { if attr.node.is_sugared_doc { if let ast::MetaItemKind::NameValue(_, ref doc) = attr.node.value.node { if let ast::LitKind::Str(ref doc, _) = doc.node { - // doc comments start with `///` or `//!` - let real_doc = &doc[3..]; - let mut span = attr.span; - span.lo = span.lo + BytePos(3); - - // check for multiline code blocks - if real_doc.trim_left().starts_with("```") { - in_multiline = !in_multiline; - } else if !in_multiline { - docs.push((real_doc, span)); - } + docs.extend_from_slice(&strip_doc_comment_decoration((doc, attr.span))); } } } @@ -135,11 +160,11 @@ fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) } #[allow(while_let_on_iterator)] // borrowck complains about for - fn jump_to(&mut self, n: char) -> Result<(), ()> { - while let Some((_, c)) = self.next() { + fn jump_to(&mut self, n: char) -> Result { + while let Some((new_line, c)) = self.next() { if c == n { self.advance_begin(); - return Ok(()); + return Ok(new_line); } } @@ -217,6 +242,54 @@ fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) pos: 0, }; + /// Check for fanced code block. + macro_rules! check_block { + ($parser:expr, $c:tt, $new_line:expr) => {{ + check_block!($parser, $c, $c, $new_line) + }}; + + ($parser:expr, $c:pat, $c_expr:expr, $new_line:expr) => {{ + fn check_block(parser: &mut Parser, new_line: bool) -> Result { + if new_line { + let mut lookup_parser = parser.clone(); + if let (Some((false, $c)), Some((false, $c))) = (lookup_parser.next(), lookup_parser.next()) { + *parser = lookup_parser; + // 3 or more ` or ~ open a code block to be closed with the same number of ` or ~ + let mut open_count = 3; + while let Some((false, $c)) = parser.next() { + open_count += 1; + } + + loop { + loop { + if try!(parser.jump_to($c_expr)) { + break; + } + } + + lookup_parser = parser.clone(); + if let (Some((false, $c)), Some((false, $c))) = (lookup_parser.next(), lookup_parser.next()) { + let mut close_count = 3; + while let Some((false, $c)) = lookup_parser.next() { + close_count += 1; + } + + if close_count == open_count { + *parser = lookup_parser; + return Ok(true); + } + } + } + } + } + + Ok(false) + } + + check_block(&mut $parser, $new_line) + }}; + } + loop { match parser.next() { Some((new_line, c)) => { @@ -225,7 +298,20 @@ fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) parser.next_line(); } '`' => { - try!(parser.jump_to('`')); + if try!(check_block!(parser, '`', new_line)) { + continue; + } + + try!(parser.jump_to('`')); // not a code block, just inline code + } + '~' => { + if try!(check_block!(parser, '~', new_line)) { + continue; + } + + // ~ does not introduce inline code, but two of them introduce + // strikethrough. Too bad for the consistency but we don't care about + // strikethrough. } '[' => { // Check for a reference definition `[foo]:` at the beginning of a line @@ -249,8 +335,12 @@ fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) parser.link = false; match parser.peek() { - Some('(') => try!(parser.jump_to(')')), - Some('[') => try!(parser.jump_to(']')), + Some('(') => { + try!(parser.jump_to(')')); + } + Some('[') => { + try!(parser.jump_to(']')); + } Some(_) => continue, None => return Err(()), } diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs index 415bcb2e6..84283a831 100755 --- a/tests/compile-fail/doc.rs +++ b/tests/compile-fail/doc.rs @@ -14,6 +14,8 @@ /// which should be reported only once despite being __doubly bad__. /// Here be ::is::a::global:path. //~^ ERROR: you should put `is::a::global:path` between ticks +/// That's not code ~NotInCodeBlock~. +//~^ ERROR: you should put `NotInCodeBlock` between ticks /// be_sure_we_got_to_the_end_of_it //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks fn foo_bar() { @@ -24,9 +26,14 @@ fn foo_bar() { /// foo_bar FOO_BAR /// _foo bar_ /// ``` +/// +/// ~~~rust +/// foo_bar FOO_BAR +/// _foo bar_ +/// ~~~ /// be_sure_we_got_to_the_end_of_it //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks -fn multiline_ticks() { +fn multiline_codeblock() { } /// This _is a test for @@ -106,7 +113,7 @@ fn test_unicode() { //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks fn main() { foo_bar(); - multiline_ticks(); + multiline_codeblock(); test_emphasis(); test_units(); } @@ -151,3 +158,42 @@ fn issue883() { /// bar](https://doc.rust-lang.org/stable/std/iter/trait.IteratorFooBar.html) fn multiline() { } + +/** E.g. serialization of an empty list: FooBar +``` +That's in a code block: `PackedNode` +``` + +And BarQuz too. +be_sure_we_got_to_the_end_of_it +*/ +//~^^^^^^^^ ERROR: you should put `FooBar` between ticks +//~^^^^ ERROR: you should put `BarQuz` between ticks +//~^^^^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn issue1073() { +} + +/** E.g. serialization of an empty list: FooBar +``` +That's in a code block: PackedNode +``` + +And BarQuz too. +be_sure_we_got_to_the_end_of_it +*/ +//~^^^^^^^^ ERROR: you should put `FooBar` between ticks +//~^^^^ ERROR: you should put `BarQuz` between ticks +//~^^^^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn issue1073_alt() { +} + +/// Test more than three quotes: +/// ```` +/// DoNotWarn +/// ``` +/// StillDont +/// ```` +/// be_sure_we_got_to_the_end_of_it +//~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn four_quotes() { +}