From 42bf37f49f49829507be4f2dfd6c5db9b8234b66 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 19 Mar 2016 17:59:12 +0100 Subject: [PATCH] Add a lint for bad documentation formatting --- README.md | 1 + src/doc.rs | 112 ++++++++++++++++++++++++++++++++++++++ src/lib.rs | 3 + tests/compile-fail/doc.rs | 26 +++++++++ 4 files changed, 142 insertions(+) create mode 100644 src/doc.rs create mode 100755 tests/compile-fail/doc.rs diff --git a/README.md b/README.md index 95ba6b08539a..e4d18c28bfec 100644 --- 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 index 000000000000..610aa34ab2c2 --- /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, Option) { + 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::().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 75b5ec4cc23a..0a51385f6f25 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ pub mod collapsible_if; 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 index 000000000000..9eb949a3b68b --- /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(); +}