Merge pull request #2590 from phansch/fix_another_false_positive

Fix false positive in empty_line_after_outer_attr

Author: oli-obk

clippy_lints/src/attrs.rs

@@ -7,7 +7,7 @@ use rustc::ty::{self, TyCtxt};
 use semver::Version;
 use syntax::ast::{Attribute, AttrStyle, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind};
 use syntax::codemap::Span;
-use utils::{in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then};
+use utils::{in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then, without_block_comments};
 
 /// **What it does:** Checks for items annotated with `#[inline(always)]`,
 /// unless the annotated function is empty or simply panics.
@@ -85,7 +85,11 @@ declare_clippy_lint! {
 /// If it was meant to be an outer attribute, then the following item
 /// should not be separated by empty lines.
 ///
-/// **Known problems:** None
+/// **Known problems:** Can cause false positives.
+///
+/// From the clippy side it's difficult to detect empty lines between an attributes and the
+/// following item because empty lines and comments are not part of the AST. The parsing
+/// currently works for basic cases but is not perfect.
 ///
 /// **Example:**
 /// ```rust
@@ -105,7 +109,7 @@ declare_clippy_lint! {
 /// ```
 declare_clippy_lint! {
     pub EMPTY_LINE_AFTER_OUTER_ATTR,
-    style,
+    nursery,
     "empty line after outer attribute"
 }
 
@@ -276,6 +280,8 @@ fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) {
 
             if let Some(snippet) = snippet_opt(cx, end_of_attr_to_item) {
                 let lines = snippet.split('\n').collect::<Vec<_>>();
+                let lines = without_block_comments(lines);
+
                 if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
                     span_lint(
                         cx,

clippy_lints/src/utils/mod.rs

@@ -1086,3 +1086,36 @@ pub fn clip(tcx: TyCtxt, u: u128, ity: ast::UintTy) -> u128 {
     let amt = 128 - bits;
     (u << amt) >> amt
 }
+
+/// Remove block comments from the given Vec of lines
+///
+/// # Examples
+///
+/// ```rust,ignore
+/// without_block_comments(vec!["/*", "foo", "*/"]);
+/// // => vec![]
+///
+/// without_block_comments(vec!["bar", "/*", "foo", "*/"]);
+/// // => vec!["bar"]
+/// ```
+pub fn without_block_comments(lines: Vec<&str>) -> Vec<&str> {
+    let mut without = vec![];
+
+    let mut nest_level = 0;
+
+    for line in lines.into_iter() {
+        if line.contains("/*") {
+            nest_level += 1;
+            continue;
+        } else if line.contains("*/") {
+            nest_level -= 1;
+            continue;
+        }
+
+        if nest_level == 0 {
+            without.push(line);
+        }
+    }
+
+    without
+}

tests/ui/empty_line_after_outer_attribute.rs

@@ -79,4 +79,16 @@ pub enum FooFighter {
     Bar4
 }
 
+// This should not produce a warning because the empty line is inside a block comment
+#[crate_type = "lib"]
+/*
+
+*/
+pub struct S;
+
+// This should not produce a warning
+#[crate_type = "lib"]
+/* test */
+pub struct T;
+
 fn main() { }

tests/without_block_comments.rs

@@ -0,0 +1,29 @@
+extern crate clippy_lints;
+use clippy_lints::utils::without_block_comments;
+
+#[test]
+fn test_lines_without_block_comments() {
+    let result = without_block_comments(vec!["/*", "", "*/"]);
+    println!("result: {:?}", result);
+    assert!(result.is_empty());
+
+    let result = without_block_comments(
+        vec!["", "/*", "", "*/", "#[crate_type = \"lib\"]", "/*", "", "*/", ""]
+    );
+    assert_eq!(result, vec!["", "#[crate_type = \"lib\"]", ""]);
+
+    let result = without_block_comments(vec!["/* rust", "", "*/"]);
+    assert!(result.is_empty());
+
+    let result = without_block_comments(vec!["/* one-line comment */"]);
+    assert!(result.is_empty());
+
+    let result = without_block_comments(vec!["/* nested", "/* multi-line",  "comment",  "*/", "test", "*/"]);
+    assert!(result.is_empty());
+
+    let result = without_block_comments(vec!["/* nested /* inline /* comment */ test */ */"]);
+    assert!(result.is_empty());
+
+    let result = without_block_comments(vec!["foo", "bar", "baz"]);
+    assert_eq!(result, vec!["foo", "bar", "baz"]);
+}