Improve the DOC_MARKDOWN lint

`_` can be used for emphasize text. `::` is equality as bad outside
ticks.

diff --git a/README.md b/README.md
index e4d18c28bfecc9aa71dfe91c6d12ae73a910349a..264bc6f010bbc1fe348cd7a5cbefb2c56c7bc678 100644 (file)
--- a/README.md
+++ b/README.md
@@ -43,7 +43,7 @@ name
 [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity)                       | warn    | finds functions that should be split up into multiple functions
 [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver)                               | warn    | `Warn` on `#[deprecated(since = "x")]` where x is not semver
 [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq)                             | warn    | deriving `Hash` but implementing `PartialEq` explicitly
-[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown)                                         | warn    | checks for the presence of the `_` character outside ticks in documentation
+[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown)                                         | warn    | checks for the presence of `_`, `::` or camel-case outside ticks in documentation
 [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref)                                                 | warn    | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value
 [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument)       | warn    | Function arguments having names which only differ by an underscore
 [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop)                                             | warn    | empty `loop {}` detected

diff --git a/src/doc.rs b/src/doc.rs
index 610aa34ab2c2ef9968e61a247719406f2f1c9975..a4a0448c102e7a660e7486d5e04560268463be11 100644 (file)
--- a/src/doc.rs
+++ b/src/doc.rs
@@ -4,22 +4,23 @@
 use syntax::codemap::Span;
 use utils::span_lint;
 
-/// **What it does:** This lint checks for the presence of the `_` character outside ticks in
-/// documentation.
+/// **What it does:** This lint checks for the presence of `_`, `::` or camel-case words outside
+/// ticks in documentation.
 ///
-/// **Why is this bad?** *Rustdoc* supports markdown formatting, the `_` character probably
+/// **Why is this bad?** *Rustdoc* supports markdown formatting, `_`, `::` and camel-case probably
 /// indicates some code which should be included between ticks.
 ///
-/// **Known problems:** Lots of bad docs won’t be fixed, the lint only checks for `_`.
+/// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks for is limited.
 ///
 /// **Examples:**
 /// ```rust
-/// /// Do something with the foo_bar parameter.
+/// /// Do something with the foo_bar parameter. See also that::other::module::foo.
+/// // ^ `foo_bar` and `that::other::module::foo` should be ticked.
 /// fn doit(foo_bar) { .. }
 /// ```
 declare_lint! {
     pub DOC_MARKDOWN, Warn,
-    "checks for the presence of the `_` character outside ticks in documentation"
+    "checks for the presence of `_`, `::` or camel-case outside ticks in documentation"
 }
 
 #[derive(Copy,Clone)]
@@ -71,10 +72,21 @@ fn doc_and_span(attr: &ast::Attribute) -> Option<(&str, Span)> {
     }
 }
 
-fn check_attrs<'a>(cx: &EarlyContext, attrs: &'a [ast::Attribute], default_span: Span) {
+pub fn check_attrs<'a>(cx: &EarlyContext, attrs: &'a [ast::Attribute], default_span: Span) {
     let (doc, span) = collect_doc(attrs);
     let span = span.unwrap_or(default_span);
 
+    // In markdown, `_` can be used to emphasize something, or, is a raw `_` depending on context.
+    // There really is no markdown specification that would disambiguate this properly. This is
+    // what GitHub and Rustdoc do:
+    //
+    // foo_bar test_quz    → foo_bar test_quz
+    // foo_bar_baz         → foo_bar_baz (note that the “official” spec says this should be emphasized)
+    // _foo bar_ test_quz_ → <em>foo bar</em> test_quz_
+    // \_foo bar\_         → _foo bar_
+    // (_baz_)             → (<em>baz</em>)
+    // foo _ bar _ baz     → foo _ bar _ baz
+
     let mut in_ticks = false;
     for word in doc.split_whitespace() {
         let ticks = word.bytes().filter(|&b| b == b'`').count();
@@ -106,7 +118,16 @@ fn is_camel_case(s: &str) -> bool {
         s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0
     }
 
-    if word.contains('_') || is_camel_case(word) {
+    fn has_underscore(s: &str) -> bool {
+        s != "_" && !s.contains("\\_") && s.contains('_')
+    }
+
+    // Trim punctuation as in `some comment (see foo::bar).`
+    //                                                   ^^
+    // Or even as `_foo bar_` which is emphasized.
+    let word = word.trim_matches(|c: char| !c.is_alphanumeric());
+
+    if has_underscore(word) || word.contains("::") || is_camel_case(word) {
         span_lint(cx, DOC_MARKDOWN, span, &format!("you should put `{}` between ticks in the documentation", word));
     }
 }

diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs
index 9eb949a3b68b951d88e91ce2f11a749b5d8b45d1..35b5857d93725460c3e41e6447f6da800860ef35 100755 (executable)
--- a/tests/compile-fail/doc.rs
+++ b/tests/compile-fail/doc.rs
@@ -6,9 +6,13 @@
 
 #![deny(doc_markdown)]
 
-/// The foo_bar function does nothing.
-//~^ ERROR: you should put `foo_bar` between ticks
+/// 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__.
 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
 }
 
 /// That one tests multiline ticks.