Skip to content

Fix false positive in empty_line_after_outer_attr #2590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 30, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -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,
33 changes: 33 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -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
}
12 changes: 12 additions & 0 deletions tests/ui/empty_line_after_outer_attribute.rs
Original file line number Diff line number Diff line change
@@ -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() { }
29 changes: 29 additions & 0 deletions tests/without_block_comments.rs
Original file line number Diff line number Diff line change
@@ -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"]);
}