From 365644e9e63bc4aa100ad4d84e2212f843a912e4 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 2 May 2016 14:36:33 +0200 Subject: [PATCH] doc markdown lint's span shows the line instead of the item --- src/doc.rs | 49 ++++++++++++++------------------------- tests/compile-fail/doc.rs | 34 +++++++++++++-------------- 2 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/doc.rs b/src/doc.rs index 54d5366729e2..6a2b3c27ba95 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -1,7 +1,6 @@ use rustc::lint::*; -use std::borrow::Cow; use syntax::ast; -use syntax::codemap::Span; +use syntax::codemap::{Span, BytePos}; use utils::span_lint; /// **What it does:** This lint checks for the presence of `_`, `::` or camel-case words outside @@ -43,48 +42,36 @@ impl LintPass for Doc { impl EarlyLintPass for Doc { fn check_crate(&mut self, cx: &EarlyContext, krate: &ast::Crate) { - check_attrs(cx, &self.valid_idents, &krate.attrs, krate.span); + check_attrs(cx, &self.valid_idents, &krate.attrs); } fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) { - check_attrs(cx, &self.valid_idents, &item.attrs, item.span); + check_attrs(cx, &self.valid_idents, &item.attrs); } } -/// Collect all doc attributes. Multiple `///` are represented in different attributes. `rustdoc` -/// has a pass to merge them, but we probably don’t want to invoke that here. -fn collect_doc(attrs: &[ast::Attribute]) -> (Cow, Option) { - fn doc_and_span(attr: &ast::Attribute) -> Option<(&str, Span)> { +pub fn check_attrs<'a>(cx: &EarlyContext, valid_idents: &[String], attrs: &'a [ast::Attribute]) { + 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 { - return Some((&doc[..], attr.span)); + // 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; + } + if !in_multiline { + check_doc(cx, valid_idents, real_doc, span); + } } } } - - None } - let doc_and_span: fn(_) -> _ = doc_and_span; - - let mut doc_attrs = attrs.iter().filter_map(doc_and_span); - - let count = doc_attrs.clone().take(2).count(); - - match count { - 0 => ("".into(), None), - 1 => { - let (doc, span) = doc_attrs.next().unwrap_or_else(|| unreachable!()); - (doc.into(), Some(span)) - } - _ => (doc_attrs.map(|s| format!("{}\n", s.0)).collect::().into(), None), - } -} - -pub fn check_attrs<'a>(cx: &EarlyContext, valid_idents: &[String], attrs: &'a [ast::Attribute], default_span: Span) { - let (doc, span) = collect_doc(attrs); - let span = span.unwrap_or(default_span); - check_doc(cx, valid_idents, &doc, span); } macro_rules! jump_to { diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs index 16e460e50556..7a150ba378dc 100755 --- a/tests/compile-fail/doc.rs +++ b/tests/compile-fail/doc.rs @@ -7,14 +7,14 @@ #![deny(doc_markdown)] /// The foo_bar function does _nothing_. See also foo::bar. (note the dot there) -/// Markdown is _weird_. I mean _really weird_. This \_ is ok. So is `_`. But not Foo::some_fun -/// which should be reported only once despite being __doubly bad__. -/// be_sure_we_got_to_the_end_of_it -fn foo_bar() { //~^ ERROR: you should put `foo_bar` between ticks //~| ERROR: you should put `foo::bar` between ticks -//~| ERROR: you should put `Foo::some_fun` between ticks -//~| ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +/// Markdown is _weird_. I mean _really weird_. This \_ is ok. So is `_`. But not Foo::some_fun +//~^ ERROR: you should put `Foo::some_fun` between ticks +/// which should be reported only once despite being __doubly bad__. +/// 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() { } /// That one tests multiline ticks. @@ -23,16 +23,16 @@ fn foo_bar() { /// _foo bar_ /// ``` /// be_sure_we_got_to_the_end_of_it -fn multiline_ticks() { //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn multiline_ticks() { } /// This _is a test for /// multiline /// emphasis_. /// be_sure_we_got_to_the_end_of_it -fn test_emphasis() { //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn test_emphasis() { } /// This tests units. See also #835. @@ -45,8 +45,8 @@ fn test_emphasis() { /// 32kB 32MB 32GB 32TB 32PB 32EB /// 32kb 32Mb 32Gb 32Tb 32Pb 32Eb /// be_sure_we_got_to_the_end_of_it -fn test_units() { //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks +fn test_units() { } /// This one checks we don’t try to split unicode codepoints @@ -55,11 +55,15 @@ fn test_units() { /// `💣` /// `❤️` /// ß_foo +//~^ ERROR: you should put `ß_foo` between ticks /// ℝ_foo +//~^ ERROR: you should put `ℝ_foo` between ticks /// 💣_foo /// ❤️_foo /// foo_ß +//~^ ERROR: you should put `foo_ß` between ticks /// foo_ℝ +//~^ ERROR: you should put `foo_ℝ` between ticks /// foo_💣 /// foo_❤️ /// [ßdummy textß][foo_ß] @@ -75,18 +79,16 @@ fn test_units() { /// [foo_💣]: dummy text /// [foo_❤️]: dummy text /// 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 test_unicode() { -//~^ ERROR: you should put `ß_foo` between ticks -//~| ERROR: you should put `ℝ_foo` between ticks -//~| ERROR: you should put `foo_ß` between ticks -//~| ERROR: you should put `foo_ℝ` between ticks -//~| ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks } /// This test has [a link_with_underscores][chunked-example] inside it. See #823. +//~^ ERROR: you should put `link_with_underscores` between ticks /// See also [the issue tracker](https://github.com/Manishearth/rust-clippy/search?q=doc_markdown&type=Issues) /// on GitHub (which is a camel-cased word, but is OK). And here is another [inline link][inline_link]. /// It can also be [inline_link2]. +//~^ ERROR: you should put `inline_link2` between ticks /// /// [chunked-example]: https://en.wikipedia.org/wiki/Chunked_transfer_encoding#Example /// [inline_link]: https://foobar @@ -98,10 +100,8 @@ fn test_unicode() { /// expression of the type `_ m c` (where `` /// is one of {`&`, '|'} and `` is one of {`!=`, `>=`, `>` , /// 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 main() { -//~^ ERROR: you should put `inline_link2` between ticks -//~| ERROR: you should put `link_with_underscores` between ticks -//~| ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks foo_bar(); multiline_ticks(); test_emphasis();