Auto merge of #4806 - FlorianRohm:issue/4623, r=flip1995

Issue/4623

Greetings.
This PR is intended to close #4623

Feedback very welcome, as this is out first contribution to the rust ecosystem :)

---
- [x] Followed [lint naming conventions][lint_naming]
  - allow tabs_in_doc_comments reads well
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `./util/dev update_lints`
- [x] Added lint documentation
- [x] Run `./util/dev fmt`

---

changelog: added lint "tabs_in_doc_comments"

CHANGELOG.md

@@ -1201,6 +1201,7 @@ Released 2018-09-13
 [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
 [`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
+[`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
 [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 333 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 334 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -274,6 +274,7 @@ pub mod slow_vector_initialization;
 pub mod strings;
 pub mod suspicious_trait_impl;
 pub mod swap;
+pub mod tabs_in_doc_comments;
 pub mod temporary_assignment;
 pub mod to_digit_is_some;
 pub mod trait_bounds;
@@ -716,6 +717,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
         &swap::ALMOST_SWAPPED,
         &swap::MANUAL_SWAP,
+        &tabs_in_doc_comments::TABS_IN_DOC_COMMENTS,
         &temporary_assignment::TEMPORARY_ASSIGNMENT,
         &to_digit_is_some::TO_DIGIT_IS_SOME,
         &trait_bounds::TYPE_REPETITION_IN_BOUNDS,
@@ -946,6 +948,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
     store.register_early_pass(|| box utils::internal_lints::ClippyLintsInternal);
     let enum_variant_name_threshold = conf.enum_variant_name_threshold;
     store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
+    store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
     store.register_late_pass(|| box unused_self::UnusedSelf);
     store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
     store.register_late_pass(|| box exit::Exit);
@@ -1243,6 +1246,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
         LintId::of(&swap::ALMOST_SWAPPED),
         LintId::of(&swap::MANUAL_SWAP),
+        LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
         LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
         LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
@@ -1370,6 +1374,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&strings::STRING_LIT_AS_BYTES),
+        LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
         LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(&try_err::TRY_ERR),
         LintId::of(&types::FN_TO_NUMERIC_CAST),

clippy_lints/src/tabs_in_doc_comments.rs

@@ -0,0 +1,219 @@
+use crate::utils::span_lint_and_sugg;
+use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
+use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_errors::Applicability;
+use std::convert::TryFrom;
+use syntax::ast;
+use syntax::source_map::{BytePos, Span};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks doc comments for usage of tab characters.
+    ///
+    /// **Why is this bad?** The rust style-guide promotes spaces instead of tabs for indentation.
+    /// To keep a consistent view on the source, also doc comments should not have tabs.
+    /// Also, explaining ascii-diagrams containing tabs can get displayed incorrectly when the
+    /// display settings of the author and reader differ.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// ///
+    /// /// Struct to hold two strings:
+    /// /// 	- first		one
+    /// /// 	- second	one
+    /// pub struct DoubleString {
+    ///    ///
+    ///    /// 	- First String:
+    ///    /// 		- needs to be inside here
+    ///    first_string: String,
+    ///    ///
+    ///    /// 	- Second String:
+    ///    /// 		- needs to be inside here
+    ///    second_string: String,
+    ///}
+    /// ```
+    ///
+    /// Will be converted to:
+     /// ```rust
+    /// ///
+    /// /// Struct to hold two strings:
+    /// ///     - first        one
+    /// ///     - second    one
+    /// pub struct DoubleString {
+    ///    ///
+    ///    ///     - First String:
+    ///    ///         - needs to be inside here
+    ///    first_string: String,
+    ///    ///
+    ///    ///     - Second String:
+    ///    ///         - needs to be inside here
+    ///    second_string: String,
+    ///}
+    /// ```
+    pub TABS_IN_DOC_COMMENTS,
+    style,
+    "using tabs in doc comments is not recommended"
+}
+
+declare_lint_pass!(TabsInDocComments => [TABS_IN_DOC_COMMENTS]);
+
+impl TabsInDocComments {
+    fn warn_if_tabs_in_doc(cx: &EarlyContext<'_>, attr: &ast::Attribute) {
+        if let ast::AttrKind::DocComment(comment) = attr.kind {
+            let comment = comment.as_str();
+
+            for (lo, hi) in get_chunks_of_tabs(&comment) {
+                let new_span = Span::new(
+                    attr.span.lo() + BytePos(lo),
+                    attr.span.lo() + BytePos(hi),
+                    attr.span.ctxt(),
+                );
+                span_lint_and_sugg(
+                    cx,
+                    TABS_IN_DOC_COMMENTS,
+                    new_span,
+                    "using tabs in doc comments is not recommended",
+                    "consider using four spaces per tab",
+                    "    ".repeat((hi - lo) as usize),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+}
+
+impl EarlyLintPass for TabsInDocComments {
+    fn check_attribute(&mut self, cx: &EarlyContext<'_>, attribute: &ast::Attribute) {
+        Self::warn_if_tabs_in_doc(cx, &attribute);
+    }
+}
+
+///
+/// scans the string for groups of tabs and returns the start(inclusive) and end positions
+/// (exclusive) of all groups
+/// e.g. "sd\tasd\t\taa" will be converted to [(2, 3), (6, 8)] as
+///       012 3456 7 89
+///         ^-^  ^---^
+fn get_chunks_of_tabs(the_str: &str) -> Vec<(u32, u32)> {
+    let line_length_way_to_long = "doc comment longer than 2^32 chars";
+    let mut spans: Vec<(u32, u32)> = vec![];
+    let mut current_start: u32 = 0;
+
+    // tracker to decide if the last group of tabs is not closed by a non-tab character
+    let mut is_active = false;
+
+    let chars_array: Vec<_> = the_str.chars().collect();
+
+    if chars_array == vec!['\t'] {
+        return vec![(0, 1)];
+    }
+
+    for (index, arr) in chars_array.windows(2).enumerate() {
+        let index = u32::try_from(index).expect(line_length_way_to_long);
+        match arr {
+            ['\t', '\t'] => {
+                // either string starts with double tab, then we have to set it active,
+                // otherwise is_active is true anyway
+                is_active = true;
+            },
+            [_, '\t'] => {
+                // as ['\t', '\t'] is excluded, this has to be a start of a tab group,
+                // set indices accordingly
+                is_active = true;
+                current_start = index + 1;
+            },
+            ['\t', _] => {
+                // this now has to be an end of the group, hence we have to push a new tuple
+                is_active = false;
+                spans.push((current_start, index + 1));
+            },
+            _ => {},
+        }
+    }
+
+    // only possible when tabs are at the end, insert last group
+    if is_active {
+        spans.push((
+            current_start,
+            u32::try_from(the_str.chars().count()).expect(line_length_way_to_long),
+        ));
+    }
+
+    spans
+}
+
+#[cfg(test)]
+mod tests_for_get_chunks_of_tabs {
+    use super::get_chunks_of_tabs;
+
+    #[test]
+    fn test_empty_string() {
+        let res = get_chunks_of_tabs("");
+
+        assert_eq!(res, vec![]);
+    }
+
+    #[test]
+    fn test_simple() {
+        let res = get_chunks_of_tabs("sd\t\t\taa");
+
+        assert_eq!(res, vec![(2, 5)]);
+    }
+
+    #[test]
+    fn test_only_t() {
+        let res = get_chunks_of_tabs("\t\t");
+
+        assert_eq!(res, vec![(0, 2)]);
+    }
+
+    #[test]
+    fn test_only_one_t() {
+        let res = get_chunks_of_tabs("\t");
+
+        assert_eq!(res, vec![(0, 1)]);
+    }
+
+    #[test]
+    fn test_double() {
+        let res = get_chunks_of_tabs("sd\tasd\t\taa");
+
+        assert_eq!(res, vec![(2, 3), (6, 8)]);
+    }
+
+    #[test]
+    fn test_start() {
+        let res = get_chunks_of_tabs("\t\taa");
+
+        assert_eq!(res, vec![(0, 2)]);
+    }
+
+    #[test]
+    fn test_end() {
+        let res = get_chunks_of_tabs("aa\t\t");
+
+        assert_eq!(res, vec![(2, 4)]);
+    }
+
+    #[test]
+    fn test_start_single() {
+        let res = get_chunks_of_tabs("\taa");
+
+        assert_eq!(res, vec![(0, 1)]);
+    }
+
+    #[test]
+    fn test_end_single() {
+        let res = get_chunks_of_tabs("aa\t");
+
+        assert_eq!(res, vec![(2, 3)]);
+    }
+
+    #[test]
+    fn test_no_tabs() {
+        let res = get_chunks_of_tabs("dsfs");
+
+        assert_eq!(res, vec![]);
+    }
+}

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 333] = [
+pub const ALL_LINTS: [Lint; 334] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1862,6 +1862,13 @@ pub const ALL_LINTS: [Lint; 333] = [
         deprecation: None,
         module: "formatting",
     },
+    Lint {
+        name: "tabs_in_doc_comments",
+        group: "style",
+        desc: "using tabs in doc comments is not recommended",
+        deprecation: None,
+        module: "tabs_in_doc_comments",
+    },
     Lint {
         name: "temporary_assignment",
         group: "complexity",

tests/ui/tabs_in_doc_comments.fixed

@@ -0,0 +1,22 @@
+// run-rustfix
+
+#![warn(clippy::tabs_in_doc_comments)]
+#[allow(dead_code)]
+
+///
+/// Struct to hold two strings:
+///     - first        one
+///     - second    one
+pub struct DoubleString {
+    ///
+    ///     - First String:
+    ///         - needs to be inside here
+    first_string: String,
+    ///
+    ///     - Second String:
+    ///         - needs to be inside here
+    second_string: String,
+}
+
+/// This is main
+fn main() {}

tests/ui/tabs_in_doc_comments.rs

@@ -0,0 +1,22 @@
+// run-rustfix
+
+#![warn(clippy::tabs_in_doc_comments)]
+#[allow(dead_code)]
+
+///
+/// Struct to hold two strings:
+/// 	- first		one
+/// 	- second	one
+pub struct DoubleString {
+    ///
+    /// 	- First String:
+    /// 		- needs to be inside here
+    first_string: String,
+    ///
+    /// 	- Second String:
+    /// 		- needs to be inside here
+    second_string: String,
+}
+
+/// This is main
+fn main() {}

tests/ui/tabs_in_doc_comments.stderr

@@ -0,0 +1,52 @@
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:12:9
+   |
+LL |     ///     - First String:
+   |         ^^^^ help: consider using four spaces per tab
+   |
+   = note: `-D clippy::tabs-in-doc-comments` implied by `-D warnings`
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:13:9
+   |
+LL |     ///         - needs to be inside here
+   |         ^^^^^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:16:9
+   |
+LL |     ///     - Second String:
+   |         ^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:17:9
+   |
+LL |     ///         - needs to be inside here
+   |         ^^^^^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:8:5
+   |
+LL | ///     - first        one
+   |     ^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:8:13
+   |
+LL | ///     - first        one
+   |                ^^^^^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:9:5
+   |
+LL | ///     - second    one
+   |     ^^^^ help: consider using four spaces per tab
+
+error: using tabs in doc comments is not recommended
+  --> $DIR/tabs_in_doc_comments.rs:9:14
+   |
+LL | ///     - second    one
+   |                 ^^^^ help: consider using four spaces per tab
+
+error: aborting due to 8 previous errors
+