diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index da7fff2ed..939fdf1fa 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -5,7 +5,7 @@ use rustc::lint::*; use rustc::hir::*; use rustc::ty::{self, TyCtxt}; use semver::Version; -use syntax::ast::{Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind}; +use syntax::ast::{Attribute, AttrStyle, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind}; use syntax::codemap::Span; use utils::{in_macro, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then}; @@ -78,12 +78,44 @@ declare_lint! { "use of `#[deprecated(since = \"x\")]` where x is not semver" } +/// **What it does:** Checks for empty lines after outer attributes +/// +/// **Why is this bad?** +/// Most likely the attribute was meant to be an inner attribute using a '!'. +/// If it was meant to be an outer attribute, then the following item +/// should not be separated by empty lines. +/// +/// **Known problems:** None +/// +/// **Example:** +/// ```rust +/// // Bad +/// #[inline(always)] +/// +/// fn not_quite_good_code(..) { ... } +/// +/// // Good (as inner attribute) +/// #![inline(always)] +/// +/// fn this_is_fine_too(..) { ... } +/// +/// // Good (as outer attribute) +/// #[inline(always)] +/// fn this_is_fine(..) { ... } +/// +/// ``` +declare_lint! { + pub EMPTY_LINE_AFTER_OUTER_ATTR, + Warn, + "empty line after outer attribute" +} + #[derive(Copy, Clone)] pub struct AttrPass; impl LintPass for AttrPass { fn get_lints(&self) -> LintArray { - lint_array!(INLINE_ALWAYS, DEPRECATED_SEMVER, USELESS_ATTRIBUTE) + lint_array!(INLINE_ALWAYS, DEPRECATED_SEMVER, USELESS_ATTRIBUTE, EMPTY_LINE_AFTER_OUTER_ATTR) } } @@ -171,7 +203,7 @@ fn is_relevant_item(tcx: TyCtxt, item: &Item) -> bool { if let ItemFn(_, _, _, _, _, eid) = item.node { is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir.body(eid).value) } else { - false + true } } @@ -230,6 +262,27 @@ fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) { } for attr in attrs { + if attr.style == AttrStyle::Outer { + if !is_present_in_source(cx, attr.span) { + return; + } + + let attr_to_item_span = Span::new(attr.span.lo(), span.lo(), span.ctxt()); + + if let Some(snippet) = snippet_opt(cx, attr_to_item_span) { + let lines = snippet.split('\n').collect::>(); + if lines.iter().filter(|l| l.trim().is_empty()).count() > 1 { + span_lint( + cx, + EMPTY_LINE_AFTER_OUTER_ATTR, + attr_to_item_span, + &format!("Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute?") + ); + + } + } + } + if let Some(ref values) = attr.meta_item_list() { if values.len() != 1 || attr.name().map_or(true, |n| n != "inline") { continue; @@ -270,3 +323,17 @@ fn is_word(nmi: &NestedMetaItem, expected: &str) -> bool { false } } + +// If the snippet is empty, it's an attribute that was inserted during macro +// expansion and we want to ignore those, because they could come from external +// sources that the user has no control over. +// For some reason these attributes don't have any expansion info on them, so +// we have to check it this way until there is a better way. +fn is_present_in_source(cx: &LateContext, span: Span) -> bool { + if let Some(snippet) = snippet_opt(cx, span) { + if snippet.is_empty() { + return false; + } + } + true +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5f2d34d9e..836d4531a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -440,6 +440,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { attrs::DEPRECATED_SEMVER, attrs::INLINE_ALWAYS, attrs::USELESS_ATTRIBUTE, + attrs::EMPTY_LINE_AFTER_OUTER_ATTR, bit_mask::BAD_BIT_MASK, bit_mask::INEFFECTIVE_BIT_MASK, bit_mask::VERBOSE_BIT_MASK, diff --git a/tests/ui/empty_line_after_outer_attribute.rs b/tests/ui/empty_line_after_outer_attribute.rs new file mode 100644 index 000000000..3d62a4913 --- /dev/null +++ b/tests/ui/empty_line_after_outer_attribute.rs @@ -0,0 +1,56 @@ + +#![warn(empty_line_after_outer_attr)] + +// This should produce a warning +#[crate_type = "lib"] + +/// some comment +fn with_one_newline_and_comment() { assert!(true) } + +// This should not produce a warning +#[crate_type = "lib"] +/// some comment +fn with_no_newline_and_comment() { assert!(true) } + + +// This should produce a warning +#[crate_type = "lib"] + +fn with_one_newline() { assert!(true) } + +// This should produce a warning, too +#[crate_type = "lib"] + + +fn with_two_newlines() { assert!(true) } + + +// This should produce a warning +#[crate_type = "lib"] + +enum Baz { + One, + Two +} + +// This should produce a warning +#[crate_type = "lib"] + +struct Foo { + one: isize, + two: isize +} + +// This should produce a warning +#[crate_type = "lib"] + +mod foo { +} + +// This should not produce a warning +#[allow(non_camel_case_types)] +#[allow(missing_docs)] +#[allow(missing_docs)] +fn three_attributes() { assert!(true) } + +fn main() { } diff --git a/tests/ui/empty_line_after_outer_attribute.stderr b/tests/ui/empty_line_after_outer_attribute.stderr new file mode 100644 index 000000000..7c9c7b8f3 --- /dev/null +++ b/tests/ui/empty_line_after_outer_attribute.stderr @@ -0,0 +1,54 @@ +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:5:1 + | +5 | / #[crate_type = "lib"] +6 | | +7 | | /// some comment +8 | | fn with_one_newline_and_comment() { assert!(true) } + | |_ + | + = note: `-D empty-line-after-outer-attr` implied by `-D warnings` + +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:17:1 + | +17 | / #[crate_type = "lib"] +18 | | +19 | | fn with_one_newline() { assert!(true) } + | |_ + +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:22:1 + | +22 | / #[crate_type = "lib"] +23 | | +24 | | +25 | | fn with_two_newlines() { assert!(true) } + | |_ + +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:29:1 + | +29 | / #[crate_type = "lib"] +30 | | +31 | | enum Baz { + | |_ + +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:37:1 + | +37 | / #[crate_type = "lib"] +38 | | +39 | | struct Foo { + | |_ + +error: Found an empty line after an outer attribute. Perhaps you forgot to add a '!' to make it an inner attribute? + --> $DIR/empty_line_after_outer_attribute.rs:45:1 + | +45 | / #[crate_type = "lib"] +46 | | +47 | | mod foo { + | |_ + +error: aborting due to 6 previous errors + diff --git a/tests/ui/inline_fn_without_body.rs b/tests/ui/inline_fn_without_body.rs index 82e073184..76e50e567 100644 --- a/tests/ui/inline_fn_without_body.rs +++ b/tests/ui/inline_fn_without_body.rs @@ -11,7 +11,6 @@ trait Foo { #[inline(always)]fn always_inline(); #[inline(never)] - fn never_inline(); #[inline] diff --git a/tests/ui/inline_fn_without_body.stderr b/tests/ui/inline_fn_without_body.stderr index fd26013d1..2b466b686 100644 --- a/tests/ui/inline_fn_without_body.stderr +++ b/tests/ui/inline_fn_without_body.stderr @@ -19,8 +19,7 @@ error: use of `#[inline]` on trait method `never_inline` which has no body | 13 | #[inline(never)] | _____-^^^^^^^^^^^^^^^ -14 | | -15 | | fn never_inline(); +14 | | fn never_inline(); | |____- help: remove error: aborting due to 3 previous errors