From b1d1f095f12d6640aac2ab7e8ad29fc36de90b39 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Mon, 28 Mar 2016 18:00:24 +0200
Subject: [PATCH] Improve the DOC_MARKDOWN lint
`_` can be used for emphasize text. `::` is equality as bad outside
ticks.
---
README.md | 2 +-
src/doc.rs | 37 +++++++++++++++++++++++++++++--------
tests/compile-fail/doc.rs | 8 ++++++--
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/README.md b/README.md
index e4d18c28bfe..264bc6f010b 100644
--- 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 610aa34ab2c..a4a0448c102 100644
--- a/src/doc.rs
+++ b/src/doc.rs
@@ -4,22 +4,23 @@ 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.
+/// **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 collect_doc(attrs: &[ast::Attribute]) -> (Cow<str>, Option<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 check_word(cx: &EarlyContext, word: &str, span: Span) {
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 9eb949a3b68..35b5857d937 100755
--- 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.