Add a lint for bad documentation formatting

diff --git a/README.md b/README.md
index 95ba6b08539a1306e444459b01a32a143f8e85a8..e4d18c28bfecc9aa71dfe91c6d12ae73a910349a 100644 (file)
--- a/README.md
+++ b/README.md
@@ -43,6 +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
 [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
new file mode 100644 (file)
index 0000000..610aa34
--- /dev/null
+++ b/src/doc.rs
@@ -0,0 +1,112 @@
+use rustc::lint::*;
+use std::borrow::Cow;
+use syntax::ast;
+use syntax::codemap::Span;
+use utils::span_lint;
+
+/// **What it does:** This lint checks for the presence of the `_` character outside ticks in
+/// documentation.
+///
+/// **Why is this bad?** *Rustdoc* supports markdown formatting, the `_` character 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 `_`.
+///
+/// **Examples:**
+/// ```rust
+/// /// Do something with the foo_bar parameter.
+/// fn doit(foo_bar) { .. }
+/// ```
+declare_lint! {
+    pub DOC_MARKDOWN, Warn,
+    "checks for the presence of the `_` character outside ticks in documentation"
+}
+
+#[derive(Copy,Clone)]
+pub struct Doc;
+
+impl LintPass for Doc {
+    fn get_lints(&self) -> LintArray {
+        lint_array![DOC_MARKDOWN]
+    }
+}
+
+impl EarlyLintPass for Doc {
+    fn check_crate(&mut self, cx: &EarlyContext, krate: &ast::Crate) {
+        check_attrs(cx, &krate.attrs, krate.span);
+    }
+
+    fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) {
+        check_attrs(cx, &item.attrs, item.span);
+    }
+}
+
+/// 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<str>, Option<Span>) {
+    fn doc_and_span(attr: &ast::Attribute) -> Option<(&str, Span)> {
+        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));
+                }
+            }
+        }
+
+        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| s.0).collect::<String>().into(), None),
+    }
+}
+
+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);
+
+    let mut in_ticks = false;
+    for word in doc.split_whitespace() {
+        let ticks = word.bytes().filter(|&b| b == b'`').count();
+
+        if ticks == 2 { // likely to be “`foo`”
+            continue;
+        } else if ticks % 2 == 1 {
+            in_ticks = !in_ticks;
+            continue; // let’s assume no one will ever write something like “`foo`_bar”
+        }
+
+        if !in_ticks {
+            check_word(cx, word, span);
+        }
+    }
+}
+
+fn check_word(cx: &EarlyContext, word: &str, span: Span) {
+    /// Checks if a string a camel-case, ie. contains at least two uppercase letter (`Clippy` is
+    /// ok) and one lower-case letter (`NASA` is ok). Plural are also excluded (`IDs` is ok).
+    fn is_camel_case(s: &str) -> bool {
+        let s = if s.ends_with('s') {
+            &s[..s.len()-1]
+        } else {
+            s
+        };
+
+        s.chars().filter(|&c| c.is_uppercase()).take(2).count() > 1 &&
+        s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0
+    }
+
+    if 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/src/lib.rs b/src/lib.rs
index 75b5ec4cc23a359a93cd257e2d8bd12168abf1a2..0a51385f6f25fb7958e792477738a61e53444adb 100644 (file)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -54,6 +54,7 @@ fn main() {
 pub mod copies;
 pub mod cyclomatic_complexity;
 pub mod derive;
+pub mod doc;
 pub mod drop_ref;
 pub mod entry;
 pub mod enum_clike;
@@ -223,6 +224,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);
     reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names));
     reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
+    reg.register_early_lint_pass(box doc::Doc);
 
     reg.register_lint_group("clippy_pedantic", vec![
         array_indexing::INDEXING_SLICING,
@@ -265,6 +267,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
         derive::DERIVE_HASH_XOR_EQ,
         derive::EXPL_IMPL_CLONE_ON_COPY,
+        doc::DOC_MARKDOWN,
         drop_ref::DROP_REF,
         entry::MAP_ENTRY,
         enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,

diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs
new file mode 100755 (executable)
index 0000000..9eb949a
--- /dev/null
+++ b/tests/compile-fail/doc.rs
@@ -0,0 +1,26 @@
+//! This file tests for the DOC_MARKDOWN lint
+//~^ ERROR: you should put `DOC_MARKDOWN` between ticks
+
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(doc_markdown)]
+
+/// The foo_bar function does nothing.
+//~^ ERROR: you should put `foo_bar` between ticks
+fn foo_bar() {
+}
+
+/// That one tests multiline ticks.
+/// ```rust
+/// foo_bar FOO_BAR
+/// ```
+fn multiline_ticks() {
+}
+
+/// The `main` function is the entry point of the program. Here it only calls the `foo_bar` and
+/// `multiline_ticks` functions.
+fn main() {
+    foo_bar();
+    multiline_ticks();
+}