Skip to content
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

Partly fix incorrect useless_attribute suggestion #2444

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions clippy_lints/src/attrs.rs
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
);
Expand Down
8 changes: 8 additions & 0 deletions clippy_lints/src/utils/mod.rs
Expand Up @@ -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>>(
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/useless_attribute.rs
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/useless_attribute.stderr
Expand Up @@ -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