From 81f5969704b57215ac12a78459c8ccfbad9be654 Mon Sep 17 00:00:00 2001
From: Philipp Hansch <dev@phansch.net>
Date: Tue, 6 Feb 2018 22:14:23 +0100
Subject: [PATCH] Partly fix incorrect useless_attribute suggestion

This fixes an incorrect suggestion from the `useless_attribute` lint
when using `cfg_attr`.

Additionally, it will not show a suggestion anymore, if the attribute
begins on a previous line, because it is much harder to construct the
span of multi-line `cfg_attr` attributes as they don't appear in the AST.

To fix it completely, one would have to parse upwards into the file,
and find the beginning of the `cfg_attr` attribute.
---
 clippy_lints/src/attrs.rs         | 14 ++++++++------
 clippy_lints/src/utils/mod.rs     |  8 ++++++++
 tests/ui/useless_attribute.rs     |  3 +++
 tests/ui/useless_attribute.stderr |  8 +++++++-
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
index aefdd6527..50aaa66aa 100644
--- a/clippy_lints/src/attrs.rs
+++ b/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, 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};
 
 /// **What it does:** Checks for items annotated with `#[inline(always)]`,
 /// unless the annotated function is empty or simply panics.
@@ -156,17 +156,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
                                             }
                                         }
                                     }
-                                    if let Some(mut sugg) = snippet_opt(cx, attr.span) {
-                                        if sugg.len() > 1 {
+                                    let line_span = last_line_of_span(cx, attr.span);
+
+                                    if let Some(mut sugg) = snippet_opt(cx, line_span) {
+                                        if sugg.contains("#[") {
                                             span_lint_and_then(
                                                 cx,
                                                 USELESS_ATTRIBUTE,
-                                                attr.span,
+                                                line_span,
                                                 "useless lint attribute",
                                                 |db| {
-                                                    sugg.insert(1, '!');
+                                                    sugg = sugg.replacen("#[", "#![", 1);
                                                     db.span_suggestion(
-                                                        attr.span,
+                                                        line_span,
                                                         "if you just forgot a `!`, use",
                                                         sugg,
                                                     );
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 75aa235ed..e89163fb5 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -427,6 +427,14 @@ pub fn snippet_block<'a, 'b, T: LintContext<'b>>(cx: &T, span: Span, default: &'
     trim_multiline(snip, true)
 }
 
+/// Returns a new Span that covers the full last line of the given Span
+pub fn last_line_of_span<'a, T: LintContext<'a>>(cx: &T, span: Span) -> Span {
+    let file_map_and_line = cx.sess().codemap().lookup_line(span.lo()).unwrap();
+    let line_no = file_map_and_line.line;
+    let line_start = &file_map_and_line.fm.lines.clone().into_inner()[line_no];
+    Span::new(*line_start, span.hi(), span.ctxt())
+}
+
 /// Like `snippet_block`, but add braces if the expr is not an `ExprBlock`.
 /// Also takes an `Option<String>` which can be put inside the braces.
 pub fn expr_block<'a, 'b, T: LintContext<'b>>(
diff --git a/tests/ui/useless_attribute.rs b/tests/ui/useless_attribute.rs
index 4c2fb221a..217e886c8 100644
--- a/tests/ui/useless_attribute.rs
+++ b/tests/ui/useless_attribute.rs
@@ -3,6 +3,9 @@
 #![warn(useless_attribute)]
 
 #[allow(dead_code, unused_extern_crates)]
+#[cfg_attr(feature = "cargo-clippy", allow(dead_code, unused_extern_crates))]
+#[cfg_attr(feature = "cargo-clippy",
+           allow(dead_code, unused_extern_crates))]
 extern crate clippy_lints;
 
 // don't lint on unused_import for `use` items
diff --git a/tests/ui/useless_attribute.stderr b/tests/ui/useless_attribute.stderr
index 707a11d55..84b81e561 100644
--- a/tests/ui/useless_attribute.stderr
+++ b/tests/ui/useless_attribute.stderr
@@ -6,5 +6,11 @@ error: useless lint attribute
   |
   = note: `-D useless-attribute` implied by `-D warnings`
 
-error: aborting due to previous error
+error: useless lint attribute
+ --> $DIR/useless_attribute.rs:6:1
+  |
+6 | #[cfg_attr(feature = "cargo-clippy", allow(dead_code, unused_extern_crates))]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: if you just forgot a `!`, use: `#![cfg_attr(feature = "cargo-clippy", allow(dead_code, unused_extern_crates))`
+
+error: aborting due to 2 previous errors